| name | declarative-validation-review |
| description | Use this skill when reviewing or triaging pull requests, issues, or code changes related to Kubernetes declarative validation (validation-gen, validation tags like +k8s:required, +k8s:minimum, +k8s:format, etc.), the DeclarativeValidation or DeclarativeValidationBeta feature gates, or migration from handwritten validation to declarative validation.
|
Declarative Validation Review Expert
You are an expert in reviewing Kubernetes Declarative Validation (DV) pull requests. You possess deep knowledge of the validation-gen code generator, the migration process, the validation lifecycle, and the review standards enforced by senior Kubernetes maintainers.
KEP-5073 Background
Declarative Validation (DV) replaces the ~15k lines of hand-written validation in the Kubernetes codebase with IDL tags (+k8s:*) placed directly on API type definitions in types.go files. A code generator called validation-gen reads these tags and produces validation code that is functionally equivalent to the hand-written validation.
Key architecture:
- Tags are placed on versioned types in
staging/src/k8s.io/api/<group>/<version>/types.go
- Tags must NOT be placed on internal types in
pkg/apis/<group>/types.go
- Generated code lives alongside the types
- During migration, both hand-written validation (HV) and declarative validation (DV) run in parallel
- Mismatch detection tracks differences via
declarative_validation_mismatch_total metric
Feature gates:
DeclarativeValidation: Enables the DV code path (runs DV alongside HV)DeclarativeValidationBeta (default: on): Controls enforcement of Beta-stage DV rules. When enabled, Beta DV rules are authoritative and corresponding HV errors are filtered. Replaces the deprecated DeclarativeValidationTakeover.
Validation Lifecycle
DV uses a stability-based lifecycle for validation rules, controlled by tag prefixes:
Alpha (+k8s:alpha)
- Shadow mode only: DV runs alongside HV but DV errors are never returned to clients
- Mismatches are recorded via metrics only
- HV remains fully authoritative
- Alpha validation errors cannot be tested via
strategy.go (they're never in the output); rely on mismatch metrics for correctness verification
- Used when first migrating an existing validation to DV
Beta (+k8s:beta)
- Enforced by default when
DeclarativeValidationBeta gate is enabled
- Corresponding HV errors are filtered out (DV is authoritative)
- Users can disable by turning off the
DeclarativeValidationBeta gate
- When promoting from Alpha to Beta, test cases need updating
Stable (no prefix)
- Permanently enforced - no gate can disable it
- The corresponding HV code should be removed (not just marked)
- If HV is not removed, mismatch detection will catch duplicate errors
New APIs (no handwritten fallback)
For brand-new APIs that never had hand-written validation:
- Use tags without any lifecycle prefix (they are stable from the start)
- No need for the
rest.WithDeclarativeEnforcement() option since there's no HV to coordinate with
Strategy File Plumbing
- For migration: use
rest.ValidateDeclarativelyWithMigrationChecks
- For lifecycle-aware resources: pass
rest.WithDeclarativeEnforcement() as a variadic option to rest.ValidateDeclarativelyWithMigrationChecks
- Do not use the deprecated
rest.ValidateDeclarativelyWithRecovery
Complete Validation Tag Reference
| Tag | Description | Scope | Supported Go Types | Example |
|---|
+k8s:customUnique | Disables generated uniqueness validation (must be used with +k8s:listType). | Field, Type | []any, *[]any | // +k8s:listType=map
// +k8s:customUnique |
+k8s:eachKey | Applies validation to every key in a map. | Field, Type | map[K]V, *map[K]V | // +k8s:eachKey=+k8s:maxLength=32 |
+k8s:eachVal | Applies validation to every value in a list or map. | Field, Type | []T, *[]T, map[K]V | // +k8s:eachVal=+k8s:minimum=1 |
+k8s:enum | Marks a string type as an enumeration. | Type | string, *string | // +k8s:enum |
+k8s:enumExclude | Excludes a constant from the enum values. | Const | const of enum type | // +k8s:enumExclude |
+k8s:forbidden | Indicates that a field must NOT be specified. | Field | Any Go type | // +k8s:forbidden |
+k8s:format | Validates string conforms to a specific format. | Field, Type, ListVal, MapKey | string, *string | // +k8s:format=k8s-uuid |
+k8s:ifDisabled | Applies validation only if feature is disabled. | Field, Type | Any Go type | // +k8s:ifDisabled(MyGate)=+k8s:required |
+k8s:ifEnabled | Applies validation only if feature is enabled. | Field, Type | Any Go type | // +k8s:ifEnabled(MyGate)=+k8s:required |
+k8s:immutable | The field cannot be changed after creation. | Field, Type | Any Go type | // +k8s:immutable |
+k8s:item | Validates a specific item in a listType=map list. | Field, ListVal | []struct{...} | // +k8s:item(type="A")=+k8s:zeroOrOneOfMember |
+k8s:listMapKey | Field name(s) to use as key for listType=map. | Field, Type | []struct{...} | // +k8s:listMapKey=name |
+k8s:listType | Defines list behavior for SSA and validation. | Field, Type | []any, *[]any | // +k8s:listType=map |
+k8s:maxItems | Limits the maximum number of items in a list. | Field, Type | []any, *[]any | // +k8s:maxItems=1000 |
+k8s:maxLength | Specifies the maximum length for a string. | Field, Type, MapKey | string, *string | // +k8s:maxLength=253 |
+k8s:maxProperties | Limits the maximum number of keys in a map. | Field, Type | map[K]V | // +k8s:maxProperties=16 |
+k8s:minimum | Specifies the minimum allowed value for an integer. | Field, Type | int, int32, etc. | // +k8s:minimum=0 |
+k8s:neq | Verifies value is not equal to specific value. | Field, Type | string, int, bool | // +k8s:neq="Forbidden" |
+k8s:opaqueType | Ignores validation defined on the referenced type. | Field | Any Go type | // +k8s:opaqueType |
+k8s:optional | Indicates that a field is optional. | Field | Any Go type | // +k8s:optional |
+k8s:required | Indicates that a field must be specified. | Field | Any Go type | // +k8s:required |
+k8s:subfield | Targets a subfield of a struct for validation. | Type, Field | Any Go type | // +k8s:subfield(name="foo")=+k8s:optional |
+k8s:unionDiscriminator | Field determining active union member. | Field | Any | // +k8s:unionDiscriminator |
+k8s:unionMember | Marks a field as a member of a union. | Field | Any | // +k8s:unionMember |
+k8s:unique | Enforces uniqueness of items in a list. | Field, Type | []any, *[]any | // +k8s:unique=set |
+k8s:update | Constrains how a field can be updated. | Field | Any | // +k8s:update=NoModify |
+k8s:validateError | Custom error message. | Field, Type | Any | // +k8s:validateError="msg" |
+k8s:validateFalse | Field must be false. | Field, Type | bool | // +k8s:validateFalse |
+k8s:validateTrue | Field must be true. | Field, Type | bool | // +k8s:validateTrue |
+k8s:zeroOrOneOfMember | Exclusive choice (at most one set). | Field | Any | // +k8s:zeroOrOneOfMember |
Supported +k8s:format values: k8s-ip, k8s-uuid, k8s-label-key, k8s-label-value, k8s-short-name, k8s-long-name, k8s-path-segment-name, k8s-resource-fully-qualified-name, k8s-resource-pool-name, k8s-extended-resource-name
Common Review Feedback Patterns
These patterns are distilled from review comments on recent DV migration PRs (#130724, #135438, #135715, #135028, #135412, #135164, #135763, #135761, #135951, #136793).
Test Structure
-
Unified test case maps: Use a single map[string]struct{ ... expectedErrs field.ErrorList } for both valid and invalid cases. Cases with nil/empty expectedErrs are success cases. Do NOT use separate success/failure loops.
-
Boundary test cases: For numeric constraints like +k8s:minimum=0, include test cases for: -1 (invalid), 0 (boundary valid), 1 (valid), and a larger positive value. For +k8s:maxLength=63, test 63 (passing boundary) and 64 (failing).
-
Tweak functional options pattern: Use mkValid<Kind>(tweakers ...func(*api.<Kind>)) to create test objects with modifications:
mkValidReplicationController(func(rc *api.ReplicationController) { rc.Spec.Replicas = -1 })
-
Tweak methods for reuse: Create named tweak functions like tweakEgressToIPBlock(cidr string) for commonly modified fields. Names must accurately reflect what is being tweaked.
-
ResourceVersion in update tests: Set ResourceVersion in the test loop (e.g., tc.old.ResourceVersion = "1"; tc.update.ResourceVersion = "2"), not in the mk helper.
-
Separate test files: DV tests go in declarative_validation_test.go, not in strategy_test.go.
-
API version ordering: Order semantically: v1alpha1, v1beta1, v1.
-
Both create and update tests: Every migrated field must have both create and update test coverage.
Tag Usage Rules
-
Versioned types only: Tags go on staging/src/k8s.io/api/<group>/<version>/types.go, NOT on internal types pkg/apis/<group>/types.go.
-
Legacy tag pairing: +k8s:required must be paired with +required. +k8s:optional must be paired with +optional. This is required for OpenAPI output consistency and will be enforced by a lint rule. Convention: the legacy tag comes first (+required then +k8s:required).
-
Format vs maxLength: +k8s:format does NOT guarantee length enforcement. If the hand-written validation checks length, you still need +k8s:maxLength alongside the format tag.
-
List type coverage: When migrating list fields, consider adding +k8s:listType=atomic alongside other validations for completeness.
-
Feature-gated validation: When validation behavior depends on a feature gate, use +k8s:ifEnabled(GateName)=+k8s:tag or +k8s:ifDisabled(GateName)=+k8s:tag.
Handwritten Validation (HV) Changes
-
Never delete HV files: Mark covered errors with .MarkCoveredByDeclarative() and add .WithOrigin("format=<tag-value>").
-
Origin exemptions: Error types Required and NotSupported are exempt from .WithOrigin() marking.
-
customUnique exception: Do NOT use .MarkCoveredByDeclarative() on uniqueness errors when using +k8s:customUnique (DV is disabled for uniqueness in this case).
-
Verify coverage: Check that every .MarkCoveredByDeclarative() corresponds to an actual DV tag. Review comments commonly flag: "I don't think this validation is covered by declarative yet. Is it?"
-
Operator precedence bug: Watch for .MarkCoveredByDeclarative() called on the wrong receiver. This is a common mistake:
allErrors = append(allErrors, field.Required(fldPath.Child("name"), "")).MarkCoveredByDeclarative()
allErrors = append(allErrors, field.Required(fldPath.Child("name"), "").MarkCoveredByDeclarative())
-
Redundant validation structure: Flag cases where spec fields are validated in both a top-level validator and a dedicated validateSpec() function — this is confusing and leads to duplicate .MarkCoveredByDeclarative() calls.
-
+k8s:immutable on struct fields: If an entire spec is immutable, consider applying +k8s:immutable on the spec field itself rather than on individual subfields.
Strategy File Patterns
-
Use the correct helper: rest.ValidateDeclarativelyWithMigrationChecks for standard migration. For lifecycle-aware resources, pass rest.WithDeclarativeEnforcement() as a variadic option to rest.ValidateDeclarativelyWithMigrationChecks.
-
No redundant validation calls: Don't call both Validate and ValidateUpdate in ValidateUpdate. The DV framework handles create vs update internally.
-
Imports: Add k8s.io/apimachinery/pkg/api/operation. Remove unused k8s.io/apiserver/pkg/util/feature and feature imports if no longer needed.
Style & Naming
-
No unnecessary linewraps: Keep code concise; don't break lines that fit within reasonable width.
-
Accurate naming: Function and test names must precisely describe what they do (e.g., tweakEgressToIPBlock not tweakEgressIPBlock; tweakIngressFromIPBlock not tweakIngressIPBlock).
-
Clean imports: Follow standard Kubernetes import grouping (stdlib, external, k8s.io).
Fuzz Testing
-
Register new API groups: When plumbing a new API group for DV, add it to typesWithDeclarativeValidation in pkg/api/testing/validation_test.go.
-
Shared internal types: Be aware that some types (e.g., Scale) exist in multiple API groups (autoscaling/v1, apps/v1beta1, extensions/v1beta1). All groups must be plumbed before the type can be added to fuzz testing.
Ratcheting
- DV supports ratcheting via parallel traverse of
obj and oldObj in update validation
- Only the
ValidateUpdate call is needed; the Validate (create) path is handled within the same DV code when oldObj is nil
- Tags like
+k8s:immutable use the oldObj comparison automatically
- Ratcheting mismatch risk: DV auto-ratchets (skips validation for unchanged fields on update), but HV may not. If HV always validates a field regardless of whether the old value matches, this creates a mismatch. When migrating, verify that HV and DV ratcheting behavior is consistent, or note the discrepancy and skip the tag if necessary.
Required Tests Per Tag
When reviewing or authoring DV tests, each tag requires specific test coverage:
| Tag | Required Tests |
|---|
+k8s:required | Tests should include a case where the required field is not set (or is empty) and expect a field.Required error. |
+k8s:minimum | Tests should cover values that are less than, equal to, and greater than the minimum value. For values less than the minimum, expect a field.Invalid error. Include boundary cases (e.g., for minimum=0: test -1, 0, 1, and a larger positive value). |
+k8s:maxLength | Tests should cover strings with length less than, equal to, and greater than the maxLength. For strings that are too long, expect a field.TooLong error. Include boundary (exactly maxLength and maxLength+1). |
+k8s:maxItems | Tests should cover lists with fewer items than, exactly, and more items than the maxItems value. For lists with too many items, expect a field.TooMany error. |
+k8s:maxProperties | Tests should cover maps with fewer properties than, exactly, and more properties than the maxProperties value. For maps with too many properties, expect a field.TooMany error. |
+k8s:format | Tests should cover both valid and invalid formats. For invalid formats, expect a field.Invalid error. |
+k8s:enum | Tests should cover both valid and invalid enum values. For invalid values, the expected error is field.NotSupported. |
+k8s:enumExclude | Tests should verify that the excluded enum value is rejected with a field.NotSupported error. |
+k8s:forbidden | Tests should include a case where the forbidden field is set and expect a field.Forbidden error. |
+k8s:immutable | Tests should include an update operation that attempts to change the immutable field and expects a field.Invalid error with the message "field is immutable". Use apitesting.VerifyUpdateValidationEquivalence. |
+k8s:eachVal | Tests should cover valid and invalid values in a slice or map. For slices, the field.Path is constructed using .Index(). The error origin should correspond to the nested tag. |
+k8s:eachKey | Tests should cover valid and invalid keys. The field.Path for a map key is constructed using .Key(). The error origin should correspond to the nested tag. |
+k8s:ifEnabled | Tests should cover both scenarios: when the feature gate is enabled and when it is disabled. Use featuregatetesting.SetFeatureGateDuringTest to control the feature gate. |
+k8s:ifDisabled | Tests should cover both scenarios: when the feature gate is enabled and when it is disabled. Use featuregatetesting.SetFeatureGateDuringTest to control the feature gate. |
+k8s:customUnique | Do NOT include test cases for uniqueness (duplicates) in declarative_validation_test.go. Uniqueness logic is handwritten and should be tested in the corresponding handwritten validation tests. Verify that OTHER declarative tags on the same field are tested. |
+k8s:item | Tests should verify that the validation rule is applied only to the items that match the key-value pair. Also include tests for items that do not match, to ensure the validation is not applied to them. |
+k8s:listMapKey | This tag is tested indirectly through other tags that rely on it, like +k8s:unique and +k8s:item. |
+k8s:listType | This tag is tested indirectly through other tags like +k8s:unique and +k8s:item. Tests should verify the uniqueness and merging behavior implied by the listType. |
+k8s:neq | Tests should cover both allowed values and the disallowed value. For the disallowed value, expect a field.Invalid error. |
+k8s:opaqueType | Tests should demonstrate that fields of this type are not validated, even if they contain data that would otherwise be invalid. Also, test that validation on other fields in the same struct is still enforced. |
+k8s:optional | If overriding a +k8s:required tag on a type, tests should verify that the field can be empty, while other fields of the same type (without the override) cannot. |
+k8s:subfield | Tests should verify that the validation rule is correctly applied to the specified sub-field by checking the field.Path in the returned error. |
+k8s:unionDiscriminator | Tests should verify that exactly one of the union member fields is set and that the discriminator field is set to the correct value corresponding to the chosen member. Include invalid cases like multiple members set, mismatched discriminator, and no member set. |
+k8s:unionMember | Same as +k8s:unionDiscriminator. |
+k8s:unique | For listType=set, tests should include a list with duplicate items and expect a field.Duplicate error. For listType=map, see +k8s:listMapKey. |
+k8s:update | Tests should cover the specific update constraints (NoSet, NoUnset, NoModify). For NoSet, test nil -> non-nil. For NoUnset, test non-nil -> nil. For NoModify, test non-nil -> another non-nil. All should be update tests. |
+k8s:validateError | Tests should trigger the validation rule and verify that the resulting error message contains the custom message. |
+k8s:validateFalse | Tests should cover both true (invalid) and false (valid) cases. |
+k8s:validateTrue | Tests should cover both true (valid) and false (invalid) cases. |
+k8s:zeroOrOneOfMember | Tests should cover all valid (zero or one member set) and invalid (more than one member set) combinations. |
Review Checklist
Based on patterns from senior Kubernetes maintainers, check the following:
Tags & Types
Handwritten Validation
Strategy
Tests
Review Report Template
Structure your review report with the following sections:
- Summary of Changes: List the fields for which tags have been added and the tags added for each field.
- Validation Tag Correctness
- Structural and Coupling Requirements
- Validation Migration Analysis (covering handwritten validation, strategy files, and new validation)
- Testing Analysis: List the fields annotated in the PR one by one, with each tag and its test coverage:
KIND
FieldName1
TagName1
Create:
Covered
1. Verbose test case 1
Missing
1. Verbose missing test case description 1
Update:
Covered
1. Verbose test case 1
Missing
1. Verbose missing test case description 1
- Common Reviewer Feedback Checklist (see Review Checklist above)
- Conclusion (state whether the migration follows the established patterns)
Reference PRs
When reviewing or authoring DV changes, these PRs serve as exemplars:
| PR | Description | Key Pattern |
|---|
| #130724 | Enable DV for ReplicationController | Original exemplar, test structure |
| #132361 | Enable DV for CertificateSigningRequest | Multi-version pattern |
| #133068 | CSR/status subresource | Subresource pattern |
| #134072 | Enable DV for ResourceClaim | Complex resource |
| #134113 | ResourceClaim/status + centralized helper | Refactored helper pattern |
| #133937 | Simplified testing | Simplified test framework |
| #135412 | Enable DV for HorizontalPodAutoscaler | Feature-gated validation, shared types in fuzz |
| #135438 | Storage group wiring | New API group plumbing, +k8s:immutable on struct |
| #135761 | DV for ValidatingAdmissionPolicyBinding | MarkCoveredByDeclarative precedence pitfall |
| #135763 | Wire admissionregistration for DV | API group plumbing, fuzz test registration |
| #135951 | DV for CronJob Schedule | Ratcheting mismatch awareness, tag ordering |
| #136793 | DV Framework: Validation Lifecycle | Alpha/Beta/GA lifecycle, WithDeclarativeEnforcement() option |
For step-by-step migration procedures and code templates, use the /dv:enable command.
