Skip to content

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

Open
aaron-prindle wants to merge 2 commits intokubernetes:masterfrom
aaron-prindle:dv-adding-a-new-field-docs-v2
Open

docs: add documentation for adding a new API fields with validation using DV tags only and testing#8906
aaron-prindle wants to merge 2 commits intokubernetes:masterfrom
aaron-prindle:dv-adding-a-new-field-docs-v2

Conversation

@aaron-prindle
Copy link
Contributor

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

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 17, 2026
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. area/developer-guide Issues or PRs related to the developer guide sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. labels Mar 17, 2026
@aaron-prindle aaron-prindle force-pushed the dv-adding-a-new-field-docs-v2 branch from d11a786 to bf0efac Compare March 17, 2026 17:41
@aaron-prindle
Copy link
Contributor Author

/cc @yongruilin @lalitc375

@aaron-prindle
Copy link
Contributor Author

/assign @thockin

Comment on lines +1357 to +1358
rule, prefer handwritten validation for that field rather than mixing DV and
handwritten validation for the same field.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@aaron-prindle aaron-prindle Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


While the goal is to express as much validation declaratively as possible, some complex or validation rules might still require manual implementation in `validation.go`.

#### Adding a new API field with DV-native validation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@aaron-prindle aaron-prindle Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

* gate off, field specified
* gate off, field omitted
* gate off, old value unchanged on update
* gate off, old value changed on update
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@k8s-ci-robot
Copy link
Contributor

[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:

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

@aaron-prindle aaron-prindle force-pushed the dv-adding-a-new-field-docs-v2 branch from ec793b5 to 962fe20 Compare March 18, 2026 05:00
@aaron-prindle aaron-prindle force-pushed the dv-adding-a-new-field-docs-v2 branch from 962fe20 to 78e1c1a Compare March 18, 2026 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/developer-guide Issues or PRs related to the developer guide cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants