Update declarative validation docs for current validation-gen workflow#9059
Update declarative validation docs for current validation-gen workflow#9059yongruilin wants to merge 1 commit into
Conversation
Refresh the API changes and conventions guides to match how declarative validation works today: plumbing is already wired for in-tree groups, so the workflow is add tags, regenerate, and test. Document runtime gating via +k8s:ifEnabled/+k8s:ifDisabled with DeclarativeValidationConfig, and rollout/migration staging via +k8s:alpha/+k8s:beta. Remove the outdated strategy.go wiring guidance and stale example PR links.
|
/assign @jpbetz |
|
/sig api-machinery |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lalitc375, yongruilin The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cc @dims |
|
/assign @liggitt |
| supports, and existing hand-written validation should be migrated to declarative | ||
| tags where possible. For the supported tags and how to apply them, see the |
There was a problem hiding this comment.
let's drop the recommendation to migrate hand-written validation from here for now
| for the catalog of supported tags. | ||
|
|
||
| Declarative validation is already wired for the in-tree API groups, so for an | ||
| existing resource you do not need to touch `doc.go` or `strategy.go` — add tags, |
There was a problem hiding this comment.
| existing resource you do not need to touch `doc.go` or `strategy.go` — add tags, | |
| existing resource you do not need to touch `doc.go` or `strategy.go`. You only need to add tags to the versioned `types.go` files, |
|
|
||
| Then regenerate with `hack/update-codegen.sh validation` (or `make update`), | ||
| which writes a `zz_generated.validations.go` file in each tagged package. Finally, | ||
| add Go unit tests just as you would for hand-written validation, asserting which |
There was a problem hiding this comment.
These need to be added in a special location for the coverage detection to work, right? can we point at docs for how to do that? (can be done in a follow-up if you want)
| func (autoscalerStrategy) DeclarativeValidationConfig(ctx context.Context, obj, oldObj runtime.Object) rest.DeclarativeValidationConfig { | ||
| var options []string | ||
| enabled := utilfeature.DefaultFeatureGate.Enabled(features.HPAScaleToZero) | ||
| if !enabled && oldObj != nil { |
There was a problem hiding this comment.
do we need to do the oldObj detection for ratcheting for DV tags? I thought we got ratcheting for free for all DV tags already
| enabled = old.Spec.MinReplicas != nil && *old.Spec.MinReplicas == 0 | ||
| } | ||
| if enabled { | ||
| options = append(options, "HPAScaleToZero") |
There was a problem hiding this comment.
@jpbetz I keep forgetting we have to manually inject string feature validation for ifEnabled/ifDisabled to work, and unknown options are silently treated as false ... let's make sure kubernetes/kubernetes#139197 gets prioritized. I would not be surprised if we already have ifEnabled/ifDisabled rules that aren't doing what they think they are because of this
|
|
||
| #### Staging a rule with `+k8s:alpha` and `+k8s:beta` | ||
|
|
||
| A tag may carry a lifecycle prefix that controls whether it is enforced or only |
There was a problem hiding this comment.
it's probably worth explaining that this is only expected to be used when migrating from hand-written validation to declarative validation. For new APIs / new fields, I would not expect this to be used at all
Refresh the API changes and conventions guides to match how declarative
validation works today.
just: add tags, regenerate, and test.
+k8s:ifEnabled/+k8s:ifDisabledand thestrategy's
DeclarativeValidationConfig.+k8s:alpha/+k8s:beta.strategy.gowiring guidance and stale example PR links.Which issue(s) this PR fixes:
Fixes #