feat(validation): Enhance validation tests with normalization rules support

This commit is contained in:
yongruilin 2025-10-03 19:33:46 +00:00
parent 7bbc7228ac
commit ae8ea8994e
3 changed files with 89 additions and 17 deletions

View file

@ -156,6 +156,28 @@ func convertToInternal(t *testing.T, scheme *runtime.Scheme, obj runtime.Object)
return scheme.ConvertToVersion(obj, schema.GroupVersion{Group: gvk.Group, Version: runtime.APIVersionInternal})
}
type ValidationTestConfig func(*validationOption)
// validationOptions encapsulates optional parameters for validation equivalence tests.
type validationOption struct {
// SubResources are the subresources to validate.
SubResources []string
// NormalizationRules are the rules to apply to field paths before comparison.
NormalizationRules []field.NormalizationRule
}
func WithSubResources(subResources ...string) ValidationTestConfig {
return func(o *validationOption) {
o.SubResources = subResources
}
}
func WithNormalizationRules(rules ...field.NormalizationRule) ValidationTestConfig {
return func(o *validationOption) {
o.NormalizationRules = rules
}
}
// VerifyValidationEquivalence provides a helper for testing the migration from
// hand-written imperative validation to declarative validation. It ensures that
// the validation logic remains consistent before and after the feature is enabled.
@ -169,12 +191,16 @@ func convertToInternal(t *testing.T, scheme *runtime.Scheme, obj runtime.Object)
// 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.
func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime.Object, validateFn ValidateFunc, expectedErrs field.ErrorList, subResources ...string) {
func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime.Object, validateFn ValidateFunc, expectedErrs field.ErrorList, testConfigs ...ValidationTestConfig) {
t.Helper()
opts := &validationOption{}
for _, testcfg := range testConfigs {
testcfg(opts)
}
verifyValidationEquivalence(t, expectedErrs, func() field.ErrorList {
return validateFn(ctx, obj)
})
VerifyVersionedValidationEquivalence(t, obj, nil, subResources...)
}, opts)
VerifyVersionedValidationEquivalence(t, obj, nil, opts.SubResources...)
}
// VerifyUpdateValidationEquivalence provides a helper for testing the migration from
@ -190,22 +216,31 @@ func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime.
// 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.
func VerifyUpdateValidationEquivalence(t *testing.T, ctx context.Context, obj, old runtime.Object, validateUpdateFn ValidateUpdateFunc, expectedErrs field.ErrorList, subResources ...string) {
func VerifyUpdateValidationEquivalence(t *testing.T, ctx context.Context, obj, old runtime.Object, validateUpdateFn ValidateUpdateFunc, expectedErrs field.ErrorList, testConfigs ...ValidationTestConfig) {
t.Helper()
opts := &validationOption{}
for _, testcfg := range testConfigs {
testcfg(opts)
}
verifyValidationEquivalence(t, expectedErrs, func() field.ErrorList {
return validateUpdateFn(ctx, obj, old)
})
VerifyVersionedValidationEquivalence(t, obj, old, subResources...)
}, opts)
VerifyVersionedValidationEquivalence(t, obj, old, opts.SubResources...)
}
// 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) {
func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, runValidations func() field.ErrorList, opt *validationOption) {
t.Helper()
var declarativeTakeoverErrs field.ErrorList
var imperativeErrs field.ErrorList
// The errOutputMatcher is used to verify the output matches the expected errors in test cases.
errOutputMatcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin()
errOutputMatcher := field.ErrorMatcher{}.ByType().ByOrigin()
if len(opt.NormalizationRules) > 0 {
errOutputMatcher = errOutputMatcher.ByFieldNormalized(opt.NormalizationRules)
} else {
errOutputMatcher = errOutputMatcher.ByField()
}
// We only need to test both gate enabled and disabled together, because
// 1) the DeclarativeValidationTakeover won't take effect if DeclarativeValidation is disabled.
@ -241,7 +276,12 @@ func verifyValidationEquivalence(t *testing.T, expectedErrs field.ErrorList, run
// The equivalenceMatcher is used to verify the output errors from hand-written imperative validation
// are equivalent to the output errors when DeclarativeValidationTakeover is enabled.
equivalenceMatcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin()
equivalenceMatcher := field.ErrorMatcher{}.ByType().ByOrigin()
if len(opt.NormalizationRules) > 0 {
equivalenceMatcher = equivalenceMatcher.ByFieldNormalized(opt.NormalizationRules)
} else {
equivalenceMatcher = equivalenceMatcher.ByField()
}
// The imperative validation may produce duplicate errors, which is not supported by the ErrorMatcher.
// TODO: remove this once ErrorMatcher has been extended to handle this form of deduplication.

View file

@ -68,12 +68,20 @@ func WithSubresourceMapper(subresourceMapper GroupVersionKindProvider) Validatio
}
}
// WithNormalizationRules sets the normalization rules for validation.
func WithNormalizationRules(rules []field.NormalizationRule) ValidationConfig {
return func(config *validationConfigOption) {
config.normalizationRules = rules
}
}
type validationConfigOption struct {
opType operation.Type
options []string
takeover bool
subresourceGVKMapper GroupVersionKindProvider
validationIdentifier string
normalizationRules []field.NormalizationRule
}
// validateDeclaratively validates obj and oldObj against declarative
@ -146,9 +154,9 @@ func parseSubresourcePath(subresourcePath string) ([]string, error) {
// compareDeclarativeErrorsAndEmitMismatches checks for mismatches between imperative and declarative validation
// and logs + emits metrics when inconsistencies are found
func compareDeclarativeErrorsAndEmitMismatches(ctx context.Context, imperativeErrs, declarativeErrs field.ErrorList, takeover bool, validationIdentifier string) {
func compareDeclarativeErrorsAndEmitMismatches(ctx context.Context, imperativeErrs, declarativeErrs field.ErrorList, takeover bool, validationIdentifier string, normalizationRules []field.NormalizationRule) {
logger := klog.FromContext(ctx)
mismatchDetails := gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs, takeover)
mismatchDetails := gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs, takeover, normalizationRules)
for _, detail := range mismatchDetails {
// Log information about the mismatch using contextual logger
logger.Error(nil, detail)
@ -160,7 +168,7 @@ func compareDeclarativeErrorsAndEmitMismatches(ctx context.Context, imperativeEr
// gatherDeclarativeValidationMismatches compares imperative and declarative validation errors
// and returns detailed information about any mismatches found. Errors are compared via type, field, and origin
func gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs field.ErrorList, takeover bool) []string {
func gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs field.ErrorList, takeover bool, normalizationRules []field.NormalizationRule) []string {
var mismatchDetails []string
// short circuit here to minimize allocs for usual case of 0 validation errors
if len(imperativeErrs) == 0 && len(declarativeErrs) == 0 {
@ -171,7 +179,12 @@ func gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs field
if takeover {
recommendation = "Consider disabling the DeclarativeValidationTakeover feature gate to keep data persisted in etcd consistent with prior versions of Kubernetes."
}
fuzzyMatcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin().RequireOriginWhenInvalid()
fuzzyMatcher := field.ErrorMatcher{}.ByType().ByOrigin().RequireOriginWhenInvalid()
if len(normalizationRules) > 0 {
fuzzyMatcher = fuzzyMatcher.ByFieldNormalized(normalizationRules)
} else {
fuzzyMatcher = fuzzyMatcher.ByField()
}
exactMatcher := field.ErrorMatcher{}.Exactly()
// Dedupe imperative errors of exact error matches as they are
@ -353,8 +366,7 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti
// Call the panic-safe wrapper with the real validation function.
declarativeErrs := panicSafeValidateFunc(validateDeclaratively, cfg.takeover, cfg.validationIdentifier)(ctx, scheme, obj, oldObj, cfg)
compareDeclarativeErrorsAndEmitMismatches(ctx, errs, declarativeErrs, takeover, validationIdentifier)
compareDeclarativeErrorsAndEmitMismatches(ctx, errs, declarativeErrs, takeover, validationIdentifier, cfg.normalizationRules)
if takeover {
errs = append(errs.RemoveCoveredByDeclarative(), declarativeErrs...)
}

View file

@ -213,6 +213,7 @@ func TestGatherDeclarativeValidationMismatches(t *testing.T) {
errB := field.Invalid(minReadySecondsPath, -1, "covered error B").WithOrigin("minimum")
coveredErrB := field.Invalid(minReadySecondsPath, -1, "covered error B").WithOrigin("minimum")
errBWithDiffDetail := field.Invalid(minReadySecondsPath, -1, "covered error B - different detail").WithOrigin("minimum")
errBWithDiffPath := field.Invalid(field.NewPath("spec").Child("fakeminReadySeconds"), -1, "covered error B").WithOrigin("minimum")
coveredErrB.CoveredByDeclarative = true
errC := field.Invalid(replicasPath, nil, "covered error C").WithOrigin("minimum")
coveredErrC := field.Invalid(replicasPath, nil, "covered error C").WithOrigin("minimum")
@ -227,6 +228,7 @@ func TestGatherDeclarativeValidationMismatches(t *testing.T) {
takeover bool
expectMismatches bool
expectDetailsContaining []string
normalizedRules []field.NormalizationRule
}{
{
name: "Declarative and imperative return 0 errors - no mismatch",
@ -358,11 +360,29 @@ func TestGatherDeclarativeValidationMismatches(t *testing.T) {
expectMismatches: false,
expectDetailsContaining: []string{},
},
{
name: "Field normalization, errors don't match - mismatch",
imperativeErrors: field.ErrorList{
coveredErrB,
},
declarativeErrors: field.ErrorList{
errBWithDiffPath,
},
normalizedRules: []field.NormalizationRule{
{
Regexp: regexp.MustCompile(`spec.fakeminReadySeconds`),
Replacement: "spec.minReadySeconds",
},
},
takeover: false,
expectMismatches: false,
expectDetailsContaining: []string{},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
details := gatherDeclarativeValidationMismatches(tc.imperativeErrors, tc.declarativeErrors, tc.takeover)
details := gatherDeclarativeValidationMismatches(tc.imperativeErrors, tc.declarativeErrors, tc.takeover, tc.normalizedRules)
// Check if mismatches were found if expected
if tc.expectMismatches && len(details) == 0 {
t.Errorf("Expected mismatches but got none")
@ -429,7 +449,7 @@ func TestCompareDeclarativeErrorsAndEmitMismatches(t *testing.T) {
defer klog.LogToStderr(true)
ctx := context.Background()
compareDeclarativeErrorsAndEmitMismatches(ctx, tc.imperativeErrs, tc.declarativeErrs, tc.takeover, "test_validationIdentifier")
compareDeclarativeErrorsAndEmitMismatches(ctx, tc.imperativeErrs, tc.declarativeErrs, tc.takeover, "test_validationIdentifier", nil)
klog.Flush()
logOutput := buf.String()