docs: add documentation for adding a new API fields with validation using DV tags only and testing

[aaron-prindle]

This updates api_changes.md with a prescriptive section for the CUJ: “add a new API field and validate it with declarative validation”. It documents the DV plumbing prerequisites, DV tag authoring, feature gate and strategy wiring, codegen checks, status subresource handling, current workarounds like +k8s:validation-gen-nolint, and copy-paste snippets + testing guidance for declarative_validation_test.go and strategy_test.go.

Which issue(s) this PR fixes:

Fixes #
N/A

[aaron-prindle]

/cc @yongruilin @lalitc375

[aaron-prindle]

/assign @thockin

[yongruilin]

Do we prefer handwritten if part of the validation on the same field can be DV?

[aaron-prindle]

Yes, my understanding from our last sync meeting on Friday is that API reviewers want new APIs fields validation to be all DV or no DV for the validations of a given new field.

[aaron-prindle]

Updated the text to be:

If DV can cover some of the new field's checks but cannot cover all of them,
prefer handwritten validation for that field rather than splitting one field's
checks between DV and handwritten validation.

[yongruilin]

It is straightforward for adding a "new API type". But a bit tricky when it comes to a new API field:
If this type already migrated some existing fields to DV. Do we still support? Once the strategy add the WithDeclarativeEnforcement(), non-prefixed tag would become authoritative and handwritten counterpart needs to be deleted; otherwise, the migrated fields need to add the k8s:beta(or alpha?) prefix to enable explicitly shadowing.

[aaron-prindle]

I clarified the scope of this section with an additional small paragraph around this

This guidance assumes the API type is plumbed for declarative
validation. If the type already has existing DV migrated/shadowed fields (+k8s:alpha/+k8s:beta) and still relies on fields w/ handwritten validation and/or shadowed DV, those older fields can continue to
exist as-is, but do not introduce that split for the new field covered by this
section.

[yongruilin]

The last case, gate off, old value changed on update, we should have a fobidden error right? I think we can extend this a bit about what should be the expected test result.

[aaron-prindle]

I updated the test guidance to make the expected result explicit. Now is a table:

Case	Expected result
gate on, field specified	Allowed if the value is valid.
gate on, field omitted	Allowed.
gate on, invalid value specified	Rejected with the field's normal validation error, such as NotSupported, Invalid, TooLong, or TooMany.
gate off, field specified	Rejected. With the standard ifDisabled(...)=+k8s:forbidden pattern, expect a forbidden error.
gate off, field omitted	Allowed.
gate off, old value unchanged on update	Allowed due to ratcheting.
gate off, old value changed on update	Rejected. With the standard ifDisabled(...)=+k8s:forbidden pattern, expect a forbidden error because the update is attempting to write a disabled field value.

[aaron-prindle]

/area api-validation

[k8s-ci-robot]

@aaron-prindle: The label(s) area/api-validation cannot be applied, because the repository doesn't have them.

Details

In response to this:

/area api-validation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[aaron-prindle]

Addressed the initial review comments there, PTAL the updated docs @yongruilin

[yongruilin]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aaron-prindle
Once this PR has been reviewed and has the lgtm label, please ask for approval from thockin. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• contributors/devel/sig-architecture/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[aaron-prindle]

@liggitt I've updated the PR here to reflect to updated guidance from the sig-arch meeting. Of note, we actually do check feature gate values for some validations in HW code, specifically when a new enum value is added to an enum and that new enum value is feature gated (as part of new gated functionality).

Example: https://github.com/kubernetes/kubernetes/blob/f5c7b422749303542baa17f1322d95250df9b0b5/pkg/apis/core/validation/validation.go#L7012-L7023

	var supportedTrafficDistribution []string
	if !utilfeature.DefaultFeatureGate.Enabled(features.PreferSameTrafficDistribution) {
		supportedTrafficDistribution = []string{
			v1.ServiceTrafficDistributionPreferClose,
		}
	} else {
		supportedTrafficDistribution = []string{
			v1.ServiceTrafficDistributionPreferClose,
			v1.ServiceTrafficDistributionPreferSameZone,
			v1.ServiceTrafficDistributionPreferSameNode,
		}
	}

I've updated the documentation here for this case now. PTAL when you get a chance, thanks!

[aaron-prindle]

/cc @liggitt (typo on initial message github name)

[yongruilin]

/lgtm

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[aaron-prindle]

Updated the feature gating + defaulting logic based on offline conversation. Now this does support the common practive of using +k8s:optional, snippet below:

  // +featureGate=Frobber2D
  // +optional
  // +k8s:optional
  // +default=8
  // +k8s:maximum=128
  Width *int32 `json:"width,omitempty" protobuf:"varint,3,opt,name=width"`

Declarative validation has some logic where +k8s:optional and +default get treated as +k8s:required (which we don't want for this case) BUT there is additional logic in DV whereif +k8s:optional, +default, and +featureGate are on a field (as it is in this case) then it is treated as +k8s:optional (which is correcT) so be sure all of those tags are on the defaulted & feature-gated field.

[aaron-prindle]

PTAL @yongruilin at the updated defaulting section

[yongruilin]

BUT there is additional logic in DV whereif +k8s:optional, +default, and +featureGate are on a field (as it is in this case)

Would you like to hold this? since it hasn't been implemented yet.

[aaron-prindle]

/hold

Good point, I will hold this awaiting the implementation for this support that we identified as a TODO for v1.37