From 93b901d17705e43916fd7f0e236e6fc0f680db93 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Thu, 12 Feb 2026 20:28:50 +0000 Subject: [PATCH] Enhance validation testing: add support for all declarative rules enforcement --- pkg/api/testing/validation.go | 81 ++++++++++++++----- .../apiserver/pkg/registry/rest/validate.go | 30 ++++++- 2 files changed, 87 insertions(+), 24 deletions(-) diff --git a/pkg/api/testing/validation.go b/pkg/api/testing/validation.go index 2144e2316ee..67844e9c442 100644 --- a/pkg/api/testing/validation.go +++ b/pkg/api/testing/validation.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/version" "k8s.io/apiserver/pkg/features" + "k8s.io/apiserver/pkg/registry/rest" utilfeature "k8s.io/apiserver/pkg/util/feature" validationmetrics "k8s.io/apiserver/pkg/validation" featuregatetesting "k8s.io/component-base/featuregate/testing" @@ -254,12 +255,17 @@ func WithFuzzer(fuzzer *randfill.Filler) ValidationTestConfig { // hand-written imperative validation to declarative validation. It ensures that // the validation logic remains consistent before and after the feature is enabled. // -// The function operates by running the provided validation function under two scenarios: -// 1. With DeclarativeValidation and DeclarativeValidationTakeover feature gates disabled, +// The function operates by running the provided validation function under four scenarios: +// 1. With DeclarativeValidation and DeclarativeValidationBeta feature gates enabled, +// using the new declarative validation rules (Beta stage). +// 2. With DeclarativeValidation enabled and DeclarativeValidationBeta disabled, +// using the new declarative validation rules (Standard stage). +// 3. With DeclarativeValidation and DeclarativeValidationTakeover feature gates disabled, // simulating the legacy hand-written validation. -// 2. With both feature gates enabled, using the new declarative validation rules. +// 4. With all declarative rules enforced (including Alpha), ensuring that the full set of +// declarative validations is correctly implemented (testing only). // -// It then asserts that the validation errors produced in both scenarios are equivalent, +// It then asserts that the validation errors produced in all scenarios are equivalent, // guaranteeing a safe migration. It also checks the errors against an expected set. // It compares errors by field, origin and type; all three should match to be called equivalent. // It also make sure all versions of the given API returns equivalent errors. @@ -269,9 +275,10 @@ func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime. for _, testcfg := range testConfigs { testcfg(opts) } - verifyValidationEquivalence(t, expectedErrs, func() field.ErrorList { - return validateFn(ctx, obj) - }, opts) + + verifyValidationEquivalence(t, expectedErrs, func(c context.Context) field.ErrorList { + return validateFn(c, obj) + }, ctx, opts) VerifyVersionedValidationEquivalence(t, obj, nil, testConfigs...) } @@ -279,12 +286,17 @@ func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime. // hand-written imperative validation to declarative validation for update operations. // It ensures that the validation logic remains consistent before and after the feature is enabled. // -// The function operates by running the provided validation function under two scenarios: -// 1. With DeclarativeValidation and DeclarativeValidationTakeover feature gates disabled, +// The function operates by running the provided validation function under four scenarios: +// 1. With DeclarativeValidation and DeclarativeValidationBeta feature gates enabled, +// using the new declarative validation rules (Beta stage). +// 2. With DeclarativeValidation enabled and DeclarativeValidationBeta disabled, +// using the new declarative validation rules (Standard stage). +// 3. With DeclarativeValidation and DeclarativeValidationTakeover feature gates disabled, // simulating the legacy hand-written validation. -// 2. With both feature gates enabled, using the new declarative validation rules. +// 4. With all declarative rules enforced (including Alpha), ensuring that the full set of +// declarative validations is correctly implemented (testing only). // -// It then asserts that the validation errors produced in both scenarios are equivalent, +// It then asserts that the validation errors produced in all scenarios are equivalent, // guaranteeing a safe migration. It also checks the errors against an expected set. // It compares errors by field, origin and type; all three should match to be called equivalent. // It also make sure all versions of the given API returns equivalent errors. @@ -294,14 +306,15 @@ func VerifyUpdateValidationEquivalence(t *testing.T, ctx context.Context, obj, o for _, testcfg := range testConfigs { testcfg(opts) } - verifyValidationEquivalence(t, expectedErrs, func() field.ErrorList { - return validateUpdateFn(ctx, obj, old) - }, opts) + + verifyValidationEquivalence(t, expectedErrs, func(c context.Context) field.ErrorList { + return validateUpdateFn(c, obj, old) + }, ctx, opts) VerifyVersionedValidationEquivalence(t, obj, old, testConfigs...) } // verifyValidationEquivalence is a generic helper that verifies validation equivalence with and without declarative validation. -func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, runValidations func() field.ErrorList, opt *validationOption) { +func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, runValidations func(context.Context) field.ErrorList, ctx context.Context, opt *validationOption) { t.Helper() var declarativeBetaEnabledErrs field.ErrorList var declarativeBetaDisabledErrs field.ErrorList @@ -312,10 +325,7 @@ func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, run defer legacyregistry.Reset() // The errOutputMatcher is used to verify the output matches the expected errors in test cases. - // TODO: Use ByValidationStabilityLevel once we want to explicitly verify the lifecycle stage of declarative - // validations (e.g. during promotion from Alpha to Beta), ensuring they are generated with the intended - // stability levels. - errOutputMatcher := field.ErrorMatcher{}.ByType().ByOrigin().ByFieldNormalized(opt.NormalizationRules).ByDeclarativeNative() + errOutputMatcher := field.ErrorMatcher{}.ByType().ByOrigin().ByFieldNormalized(opt.NormalizationRules) // 1. Declarative Validation with Beta Gate Enabled t.Run("with declarative validation (Beta enabled)", func(t *testing.T) { @@ -324,7 +334,7 @@ func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, run features.DeclarativeValidation: true, features.DeclarativeValidationBeta: true, }) - declarativeBetaEnabledErrs = runValidations() + declarativeBetaEnabledErrs = runValidations(ctx) if len(expectedErrs) > 0 { errOutputMatcher.Test(t, expectedErrs, declarativeBetaEnabledErrs) @@ -343,7 +353,7 @@ func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, run features.DeclarativeValidation: true, features.DeclarativeValidationBeta: false, }) - declarativeBetaDisabledErrs = runValidations() + declarativeBetaDisabledErrs = runValidations(ctx) if len(expectedErrs) > 0 { errOutputMatcher.Test(t, expectedErrs, declarativeBetaDisabledErrs) @@ -365,7 +375,7 @@ func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, run featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{ features.DeclarativeValidation: false, }) - imperativeErrs = runValidations() + imperativeErrs = runValidations(ctx) if len(expectedErrs) > 0 { errOutputMatcher.Test(t, expectedErrs, imperativeErrs) @@ -374,6 +384,33 @@ func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, run } }) + // 4. Declarative Validation with All Rules Enforced (Testing Only) + // This sub-test ensures that all declarative validation rules (including those marked as Alpha) + // are correctly implemented and match the expected errors. It uses a special context + // to force enforcement of all declarative rules and filter out their handwritten counterparts. + t.Run("with declarative validation (All Rules Enforced)", func(t *testing.T) { + validationmetrics.ResetValidationMetricsInstance() + // We don't strictly need to set feature gates here as the context override should force enforcement, + // but setting them ensures a consistent environment. + featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{ + features.DeclarativeValidation: true, + features.DeclarativeValidationBeta: true, + }) + testCtx := rest.WithAllDeclarativeEnforcedForTest(ctx) + allDeclarativeErrs := runValidations(testCtx) + + // The matcher here is more specific to ensure that errors from Alpha rules are included and matched correctly. + errOutputMatcherByStability := field.ErrorMatcher{}.ByType().ByOrigin().ByFieldNormalized(opt.NormalizationRules).ByValidationStabilityLevel() + if len(expectedErrs) > 0 { + errOutputMatcherByStability.Test(t, expectedErrs, allDeclarativeErrs) + } else if len(allDeclarativeErrs) != 0 { + t.Errorf("expected no errors, but got: %v", allDeclarativeErrs) + } + + // Ensure no mismatches were logged/metrics incremented + testutil.AssertVectorCount(t, "apiserver_validation_declarative_validation_mismatch_total", nil, 0) + }) + if t.Failed() { // There is no point in moving forward, if any of above tests failed for any reason. Running follow up tests will return noise. t.SkipNow() diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go index 0eadc6a00f9..df263adf29b 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go @@ -87,6 +87,19 @@ func WithDeclarativeEnforcement() ValidationConfig { } } +type allDeclarativeEnforcedKeyType struct{} + +var allDeclarativeEnforcedKey = allDeclarativeEnforcedKeyType{} + +// WithAllDeclarativeEnforcedForTest returns a copy of parent context with allDeclarativeEnforcedKey set to true. +// This is used for testing to expose all declarative validation errors and filter all handwritten validation errors +// that are covered by declarative validation, regardless of the feature gate or maturity level. +// +// NOTE: This function is intended for testing purposes only and should not be used in production code. +func WithAllDeclarativeEnforcedForTest(ctx context.Context) context.Context { + return context.WithValue(ctx, allDeclarativeEnforcedKey, true) +} + type validationConfigOption struct { opType operation.Type options []string @@ -371,9 +384,14 @@ func metricIdentifier(ctx context.Context, scheme *runtime.Scheme, obj runtime.O // // Mismatches between HV and DV are logged if the DeclarativeValidation gate is enabled. // Mismatch checking is limited to Alpha and Beta stages when explicit enforcement is active. +// +// For testing purposes, WithAllDeclarativeEnforcedForTest can be used to enforce all declarative validations +// regardless of feature gates and filter all covered handwritten validations. func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runtime.Scheme, obj, oldObj runtime.Object, errs field.ErrorList, opType operation.Type, configOpts ...ValidationConfig) field.ErrorList { declarativeValidationEnabled := utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidation) betaEnabled := utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidationBeta) + // allDeclarativeEnforced indicates that we should check all declarative errors for testing purposes. + allDeclarativeEnforced := ctx.Value(allDeclarativeEnforcedKey) == true validationIdentifier, err := metricIdentifier(ctx, scheme, obj, opType) if err != nil { @@ -391,7 +409,7 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti } // Short-circuit if neither DeclarativeValidation is enabled nor the object is explicitly configured for declarative enforcement. - if !declarativeValidationEnabled && !cfg.declarativeEnforcement { + if !declarativeValidationEnabled && !cfg.declarativeEnforcement && !allDeclarativeEnforced { return errs } @@ -418,7 +436,7 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti compareDeclarativeErrorsAndEmitMismatches(ctx, errs, mismatchCandidateErrs, cfg.declarativeEnforcement && betaEnabled, validationIdentifier, cfg.normalizationRules) } - if !cfg.declarativeEnforcement { + if !cfg.declarativeEnforcement && !allDeclarativeEnforced { // If enforcement is not enabled, we shadow declarative errors with hand-written ones, so we return early here. return errs } @@ -431,6 +449,10 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti return false } + if allDeclarativeEnforced { + return true + } + // Explicit Strategy if fe.IsBeta() { // Beta validations are enforced only if the Beta feature gate is enabled. @@ -443,6 +465,10 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti // Append Enforced DV errors for _, dvErr := range declarativeErrs { + if allDeclarativeEnforced { + errs = append(errs, dvErr) + continue + } switch { case dvErr.Type == field.ErrorTypeInternal: errs = append(errs, dvErr)