From 19fa5ef5eb070f7401f958c8b28c173071b86d96 Mon Sep 17 00:00:00 2001 From: Lalit Chauhan Date: Tue, 6 Jan 2026 21:23:52 +0000 Subject: [PATCH 1/3] apimachinery/field: Add MarkDeclarativeNative support to Error and ErrorList --- .../util/validation/field/error_matcher.go | 16 ++++ .../validation/field/error_matcher_test.go | 50 +++++++++++ .../pkg/util/validation/field/errors.go | 89 ++++++++++++++++--- 3 files changed, 144 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher.go index f0264e50c7d..0ec6c7ef2da 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher.go @@ -44,6 +44,7 @@ type ErrorMatcher struct { matchOrigin bool matchDetail func(want, got string) bool requireOriginWhenInvalid bool + matchDeclarativeNative bool // normalizationRules holds the pre-compiled regex patterns for path normalization. normalizationRules []NormalizationRule } @@ -86,6 +87,10 @@ func (m ErrorMatcher) Matches(want, got *Error) bool { if m.matchDetail != nil && !m.matchDetail(want.Detail, got.Detail) { return false } + if m.matchDeclarativeNative && want.DeclarativeNative != got.DeclarativeNative { + return false + } + return true } @@ -148,6 +153,10 @@ func (m ErrorMatcher) Render(e *Error) string { comma() buf.WriteString(fmt.Sprintf("Detail=%q", e.Detail)) } + if m.matchDeclarativeNative { + comma() + buf.WriteString(fmt.Sprintf("DeclarativeNative=%t", e.DeclarativeNative)) + } return "{" + buf.String() + "}" } @@ -224,6 +233,13 @@ func (m ErrorMatcher) RequireOriginWhenInvalid() ErrorMatcher { return m } +// ByDeclarativeNative returns a derived ErrorMatcher which also matches by the DeclarativeNative +// value of field errors. +func (m ErrorMatcher) ByDeclarativeNative() ErrorMatcher { + m.matchDeclarativeNative = true + return m +} + // ByDetailExact returns a derived ErrorMatcher which also matches errors by // the exact detail string. func (m ErrorMatcher) ByDetailExact() ErrorMatcher { diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher_test.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher_test.go index 45a477ce2a2..02e07fb8be5 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/error_matcher_test.go @@ -303,6 +303,26 @@ func TestErrorMatcher_Matches(t *testing.T) { wantedErr: baseErr, actualErr: &Error{Type: ErrorTypeRequired, Field: "field", BadValue: "value", Detail: "detail", Origin: "origin"}, matches: false, + }, { + name: "ByDeclarativeNative: match", + matcher: ErrorMatcher{}.ByDeclarativeNative(), + wantedErr: func() *Error { + e := baseErr() + e.DeclarativeNative = true + return e + }, + actualErr: &Error{DeclarativeNative: true}, + matches: true, + }, { + name: "ByDeclarativeNative: no match", + matcher: ErrorMatcher{}.ByDeclarativeNative(), + wantedErr: func() *Error { + e := baseErr() + e.DeclarativeNative = true + return e + }, + actualErr: &Error{DeclarativeNative: false}, + matches: false, }, { name: "RequireOriginWhenInvalid: match", matcher: ErrorMatcher{}.ByOrigin().RequireOriginWhenInvalid(), @@ -406,6 +426,17 @@ func TestErrorMatcher_Test(t *testing.T) { want: ErrorList{Invalid(NewPath("f").Index(0).Child("x", "a"), nil, "")}, got: ErrorList{Invalid(NewPath("f").Index(1).Child("a"), "v", "d")}, expectedErrors: []string{"expected an error matching:", "unmatched error:"}, + }, { + name: "declarative native: match", + matcher: ErrorMatcher{}.ByDeclarativeNative(), + want: ErrorList{{DeclarativeNative: true}}, + got: ErrorList{{DeclarativeNative: true}}, + }, { + name: "declarative native: no match", + matcher: ErrorMatcher{}.ByDeclarativeNative(), + want: ErrorList{{DeclarativeNative: true}}, + got: ErrorList{{DeclarativeNative: false}}, + expectedErrors: []string{"expected an error matching:", "unmatched error:"}, }, { name: "with origin: single match", matcher: ErrorMatcher{}.ByField().ByOrigin(), @@ -533,6 +564,25 @@ func TestErrorMatcher_Render(t *testing.T) { expected: `{Field="f[0].x.a"}`, }, { + name: "with covered by declarative", + matcher: ErrorMatcher{}.ByDeclarativeNative(), + err: func() *Error { + e := Invalid(NewPath("field"), "value", "detail") + e.DeclarativeNative = true + return e + }(), + expected: `{DeclarativeNative=true}`, + }, + { + name: "all fields with covered by declarative", + matcher: ErrorMatcher{}.ByType().ByField().ByValue().ByOrigin().ByDetailExact().ByDeclarativeNative(), + err: func() *Error { + e := Invalid(NewPath("field"), "value", "detail").WithOrigin("origin") + e.DeclarativeNative = true + return e + }(), + expected: `{Type="Invalid value", Field="field", Value="value", Origin="origin", Detail="detail", DeclarativeNative=true}`, + }, { name: "requireOriginWhenInvalid with origin", matcher: ErrorMatcher{}.ByOrigin().RequireOriginWhenInvalid(), err: Invalid(NewPath("field"), "value", "detail").WithOrigin("origin"), diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go index 950d8386823..4b83eed3104 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go @@ -42,7 +42,7 @@ type Error struct { // The value should be either: // - A simple camelCase identifier (e.g., "maximum", "maxItems") // - A structured format using "format=" for validation errors related to specific formats - // (e.g., "format=dns-label", "format=qualified-name") + // (e.g. "format=k8s-short-name") // // If the Origin corresponds to an existing declarative validation tag or JSON Schema keyword, // use that same name for consistency. @@ -55,6 +55,10 @@ type Error struct { // validation. This field is to identify errors from imperative validation // that should also be caught by declarative validation. CoveredByDeclarative bool + + // DeclarativeNative is true when this error originates from a declarative-native validation. + // This field is used to distinguish errors that are exclusively declarative and lack an imperative counterpart. + DeclarativeNative bool } var _ error = &Error{} @@ -201,32 +205,56 @@ func (t ErrorType) String() string { // TypeInvalid returns a *Error indicating "type is invalid" func TypeInvalid(field *Path, value interface{}, detail string) *Error { - return &Error{ErrorTypeTypeInvalid, field.String(), value, detail, "", false} + return &Error{ + Type: ErrorTypeTypeInvalid, + Field: field.String(), + BadValue: value, + Detail: detail, + } } // NotFound returns a *Error indicating "value not found". This is // used to report failure to find a requested value (e.g. looking up an ID). func NotFound(field *Path, value interface{}) *Error { - return &Error{ErrorTypeNotFound, field.String(), value, "", "", false} + return &Error{ + Type: ErrorTypeNotFound, + Field: field.String(), + BadValue: value, + } } // Required returns a *Error indicating "value required". This is used // to report required values that are not provided (e.g. empty strings, null // values, or empty arrays). func Required(field *Path, detail string) *Error { - return &Error{ErrorTypeRequired, field.String(), "", detail, "", false} + return &Error{ + Type: ErrorTypeRequired, + Field: field.String(), + Detail: detail, + BadValue: "", + } } // Duplicate returns a *Error indicating "duplicate value". This is // used to report collisions of values that must be unique (e.g. names or IDs). func Duplicate(field *Path, value interface{}) *Error { - return &Error{ErrorTypeDuplicate, field.String(), value, "", "", false} + return &Error{ + Type: ErrorTypeDuplicate, + Field: field.String(), + BadValue: value, + } } // Invalid returns a *Error indicating "invalid value". This is used // to report malformed values (e.g. failed regex match, too long, out of bounds). func Invalid(field *Path, value interface{}, detail string) *Error { - return &Error{ErrorTypeInvalid, field.String(), value, detail, "", false} + return &Error{ + Type: ErrorTypeInvalid, + Field: field.String(), + BadValue: value, + Detail: detail, + } + } // NotSupported returns a *Error indicating "unsupported value". @@ -241,7 +269,12 @@ func NotSupported[T ~string](field *Path, value interface{}, validValues []T) *E } detail = "supported values: " + strings.Join(quotedValues, ", ") } - return &Error{ErrorTypeNotSupported, field.String(), value, detail, "", false} + return &Error{ + Type: ErrorTypeNotSupported, + Field: field.String(), + BadValue: value, + Detail: detail, + } } // Forbidden returns a *Error indicating "forbidden". This is used to @@ -249,7 +282,12 @@ func NotSupported[T ~string](field *Path, value interface{}, validValues []T) *E // some conditions, but which are not permitted by current conditions (e.g. // security policy). func Forbidden(field *Path, detail string) *Error { - return &Error{ErrorTypeForbidden, field.String(), "", detail, "", false} + return &Error{ + Type: ErrorTypeForbidden, + Field: field.String(), + Detail: detail, + BadValue: "", + } } // TooLong returns a *Error indicating "too long". This is used to report that @@ -267,7 +305,12 @@ func TooLong(field *Path, _ interface{}, maxLength int) *Error { } else { msg = "value is too long" } - return &Error{ErrorTypeTooLong, field.String(), "", msg, "", false} + return &Error{ + Type: ErrorTypeTooLong, + Field: field.String(), + BadValue: "", + Detail: msg, + } } // TooLongMaxLength returns a *Error indicating "too long". @@ -299,14 +342,24 @@ func TooMany(field *Path, actualQuantity, maxQuantity int) *Error { actual = omitValue } - return &Error{ErrorTypeTooMany, field.String(), actual, msg, "", false} + return &Error{ + Type: ErrorTypeTooMany, + Field: field.String(), + BadValue: actual, + Detail: msg, + } } // InternalError returns a *Error indicating "internal error". This is used // to signal that an error was found that was not directly related to user // input. The err argument must be non-nil. func InternalError(field *Path, err error) *Error { - return &Error{ErrorTypeInternal, field.String(), nil, err.Error(), "", false} + return &Error{ + Type: ErrorTypeInternal, + Field: field.String(), + BadValue: err, + Detail: err.Error(), + } } // ErrorList holds a set of Errors. It is plausible that we might one day have @@ -397,6 +450,20 @@ func (list ErrorList) ExtractCoveredByDeclarative() ErrorList { return newList } +// MarkDeclarativeNative marks the error as originating from a declarative-native validation. +func (e *Error) MarkDeclarativeNative() *Error { + e.DeclarativeNative = true + return e +} + +// MarkDeclarativeNative marks all errors in the list as originating from declarative-native validations. +func (list ErrorList) MarkDeclarativeNative() ErrorList { + for _, err := range list { + err.DeclarativeNative = true + } + return list +} + // RemoveCoveredByDeclarative returns a new ErrorList containing only the errors that should not be covered by declarative validation. func (list ErrorList) RemoveCoveredByDeclarative() ErrorList { newList := ErrorList{} From 265394c0a5ff9795d151226d77c35b8615ad3d39 Mon Sep 17 00:00:00 2001 From: Lalit Chauhan Date: Thu, 8 Jan 2026 21:58:26 +0000 Subject: [PATCH 2/3] Add utilities to allow strategy.go files to enable DV naitve validations --- .../apiserver/pkg/registry/rest/validate.go | 89 +++++++++---- .../pkg/registry/rest/validate_test.go | 123 ++++++++++++++++++ 2 files changed, 185 insertions(+), 27 deletions(-) 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 65ffdd4f16d..41d2ab1287e 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go @@ -76,13 +76,24 @@ func WithNormalizationRules(rules []field.NormalizationRule) ValidationConfig { } } +// WithDeclarativeNative marks the validation configuration to indicate that it includes +// declarative validations that are defined *only* declaratively, lacking corresponding imperative validation. +// When set, declarative validation is always executed regardless of feature gates. Errors marked as +// declarative-native are separated from the full set and returned alongside imperative errors. +func WithDeclarativeNative() ValidationConfig { + return func(config *validationConfigOption) { + config.containsDeclarativeNative = true + } +} + type validationConfigOption struct { - opType operation.Type - options []string - takeover bool - subresourceGVKMapper GroupVersionKindProvider - validationIdentifier string - normalizationRules []field.NormalizationRule + opType operation.Type + options []string + takeover bool + subresourceGVKMapper GroupVersionKindProvider + validationIdentifier string + normalizationRules []field.NormalizationRule + containsDeclarativeNative bool } // validateDeclaratively validates obj and oldObj against declarative @@ -262,8 +273,8 @@ func gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs field } // createDeclarativeValidationPanicHandler returns a function with panic recovery logic -// that will increment the panic metric and either log or append errors based on the takeover parameter. -func createDeclarativeValidationPanicHandler(ctx context.Context, errs *field.ErrorList, takeover bool, validationIdentifier string) func() { +// that will increment the panic metric and either log or append errors based on the shouldFail parameter. +func createDeclarativeValidationPanicHandler(ctx context.Context, errs *field.ErrorList, shouldFail bool, validationIdentifier string) func() { logger := klog.FromContext(ctx) return func() { if r := recover(); r != nil { @@ -271,11 +282,11 @@ func createDeclarativeValidationPanicHandler(ctx context.Context, errs *field.Er validationmetrics.Metrics.IncDeclarativeValidationPanicMetric(validationIdentifier) const errorFmt = "panic during declarative validation: %v" - if takeover { - // If takeover is enabled, output as a validation error as authoritative validator panicked and validation should error + if shouldFail { + // If shouldFail is enabled, output as a validation error as authoritative validator panicked and validation should error *errs = append(*errs, field.InternalError(nil, fmt.Errorf(errorFmt, r))) } else { - // if takeover not enabled, log the panic as an error message + // if shouldFail not enabled, log the panic as an error message logger.Error(nil, fmt.Sprintf(errorFmt, r)) } } @@ -285,13 +296,13 @@ func createDeclarativeValidationPanicHandler(ctx context.Context, errs *field.Er // panicSafeValidateFunc wraps an validation function with panic recovery logic. // The returned function will execute the wrapped function and handle any panics by // incrementing the panic metric, and logging an error message -// if takeover=false, and adding a validation error if takeover=true. +// if shouldFail=false, and adding a validation error if shouldFail=true. func panicSafeValidateFunc( validateUpdateFunc func(ctx context.Context, scheme *runtime.Scheme, obj, oldObj runtime.Object, o *validationConfigOption) field.ErrorList, - takeover bool, validationIdentifier string, + shouldFail bool, validationIdentifier string, ) func(ctx context.Context, scheme *runtime.Scheme, obj, oldObj runtime.Object, o *validationConfigOption) field.ErrorList { return func(ctx context.Context, scheme *runtime.Scheme, obj, oldObj runtime.Object, o *validationConfigOption) (errs field.ErrorList) { - defer createDeclarativeValidationPanicHandler(ctx, &errs, takeover, validationIdentifier)() + defer createDeclarativeValidationPanicHandler(ctx, &errs, shouldFail, validationIdentifier)() return validateUpdateFunc(ctx, scheme, obj, oldObj, o) } @@ -334,14 +345,13 @@ func metricIdentifier(ctx context.Context, scheme *runtime.Scheme, obj runtime.O return identifier, errs } -// ValidateDeclarativelyWithMigrationChecks is a helper function that encapsulates the logic for running declarative validation. -// It checks if the DeclarativeValidation feature gate is enabled, generates a validation identifier, -// runs declarative validation, compares the results with imperative validation, and merges the errors if takeover is enabled. +// ValidateDeclarativelyWithMigrationChecks encapsulates the logic for running declarative validation. +// It proceeds if either the DeclarativeValidation feature gate is enabled or `containsDeclarativeNative` is set. +// The function generates a validation identifier, runs declarative validation, +// and conditionally compares results with imperative validation and merges errors based on the feature gate and `takeover` flag. +// Declarative-native errors are always appended to the result. func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runtime.Scheme, obj, oldObj runtime.Object, errs field.ErrorList, opType operation.Type, configOpts ...ValidationConfig) field.ErrorList { - if !utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidation) { - return errs - } - + declarativeValidationEnabled := utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidation) takeover := utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidationTakeover) validationIdentifier, err := metricIdentifier(ctx, scheme, obj, opType) @@ -360,14 +370,39 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti opt(cfg) } - // 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, cfg.normalizationRules) - if takeover { - errs = append(errs.RemoveCoveredByDeclarative(), declarativeErrs...) + if !declarativeValidationEnabled && !cfg.containsDeclarativeNative { + return errs } + // Call the panic-safe wrapper with the real validation function. + declarativeErrs := panicSafeValidateFunc(validateDeclaratively, cfg.takeover || cfg.containsDeclarativeNative, cfg.validationIdentifier)(ctx, scheme, obj, oldObj, cfg) + + mirroredDVErrors := field.ErrorList{} + dvNativeErrors := field.ErrorList{} + if cfg.containsDeclarativeNative { + for _, err := range declarativeErrs { + if err.DeclarativeNative { + dvNativeErrors = append(dvNativeErrors, err) + // Internal Error is very unlikely to happen, if that happen it should be fail the validations + // for both of types of validations. + } else if err.Type == field.ErrorTypeInternal { + dvNativeErrors = append(dvNativeErrors, err) + mirroredDVErrors = append(mirroredDVErrors, err) + } else { + mirroredDVErrors = append(mirroredDVErrors, err) + } + } + } else { + mirroredDVErrors = declarativeErrs + } + + if declarativeValidationEnabled { + compareDeclarativeErrorsAndEmitMismatches(ctx, errs, mirroredDVErrors, takeover, validationIdentifier, cfg.normalizationRules) + if takeover { + errs = append(errs.RemoveCoveredByDeclarative(), mirroredDVErrors...) + } + } + errs = append(errs, dvNativeErrors...) return errs } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go index b1218e01918..d8d6304fa5e 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go @@ -34,7 +34,10 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/validation" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-base/metrics/legacyregistry" "k8s.io/component-base/metrics/testutil" "k8s.io/klog/v2" @@ -823,3 +826,123 @@ func TestMetricIdentifier(t *testing.T) { }) } } + +func TestValidateDeclarativelyWithMigrationChecks(t *testing.T) { + // Create errors for testing + errImperative := field.ErrorList{ + field.Invalid(field.NewPath("spec", "restartPolicy"), "imp", "imperative error"), + field.Forbidden(field.NewPath("spec", "restartPolicy"), "imperative error covered by declarative").MarkCoveredByDeclarative(), + } + errMirroredDeclarative := field.Invalid(field.NewPath("spec", "restartPolicy"), "dec", "declarative error") + errDeclarativeNative := field.Invalid(field.NewPath("spec", "replicas"), "decOnly", "declarative native error").MarkDeclarativeNative() + + testCases := []struct { + name string + dvFeatureEnabled bool + takeoverEnabled bool + containsDeclarativeNative bool + imperativeErrors field.ErrorList + declarativeErrors field.ErrorList + expectedErrors field.ErrorList + shouldPanic bool + }{ + { + name: "Feature Disabled, Not DeclarativeNative -> Skips declarative", + imperativeErrors: errImperative, + declarativeErrors: field.ErrorList{errMirroredDeclarative}, + expectedErrors: errImperative, + }, + { + name: "Feature Disabled, contains DeclarativeNative -> Returns DeclarativeNative errors, ignores regular declarative errors", + containsDeclarativeNative: true, + imperativeErrors: errImperative, + declarativeErrors: field.ErrorList{errMirroredDeclarative, errDeclarativeNative}, + expectedErrors: append(errImperative, errDeclarativeNative), + }, + { + name: "Feature Enabled, Not DeclarativeNative -> Returns imperative (no takeover)", + dvFeatureEnabled: true, + imperativeErrors: errImperative, + declarativeErrors: field.ErrorList{errMirroredDeclarative}, + expectedErrors: errImperative, + }, + { + name: "Feature Enabled, contains DeclarativeNative -> Returns imperative + DeclarativeNative", + dvFeatureEnabled: true, + containsDeclarativeNative: true, + imperativeErrors: errImperative, + declarativeErrors: field.ErrorList{errMirroredDeclarative, errDeclarativeNative}, + expectedErrors: append(errImperative, errDeclarativeNative), + }, + { + name: "Feature Enabled, takeover enabled, contains DeclarativeNative -> Returns non mirrored imperative + declarative + DeclarativeNative", + dvFeatureEnabled: true, + containsDeclarativeNative: true, + takeoverEnabled: true, + imperativeErrors: errImperative, + declarativeErrors: field.ErrorList{errMirroredDeclarative, errDeclarativeNative}, + expectedErrors: field.ErrorList{errImperative[0], errMirroredDeclarative, errDeclarativeNative}, + }, + { + name: "Feature Disabled, contains DeclarativeNative, Panics -> Returns InternalError", + containsDeclarativeNative: true, + imperativeErrors: errImperative, + shouldPanic: true, + expectedErrors: append(errImperative, field.InternalError(nil, fmt.Errorf("panic during declarative validation: test panic"))), + }, + { + name: "Feature Enabled, takeover enabled, contains DeclarativeNative, InternalError -> Returns non mirrored imperative + InternalError", + dvFeatureEnabled: true, + containsDeclarativeNative: true, + takeoverEnabled: true, + imperativeErrors: errImperative, + declarativeErrors: field.ErrorList{field.InternalError(nil, fmt.Errorf("internal error"))}, + expectedErrors: field.ErrorList{errImperative[0], field.InternalError(nil, fmt.Errorf("internal error")), field.InternalError(nil, fmt.Errorf("internal error"))}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Set feature gate + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DeclarativeValidation, tc.dvFeatureEnabled) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DeclarativeValidationTakeover, tc.takeoverEnabled) + + // Setup scheme for this run + localScheme := runtime.NewScheme() + localScheme.AddKnownTypes(schema.GroupVersion{Group: "", Version: "v1"}, &v1.Pod{}) + localScheme.AddValidationFunc(&v1.Pod{}, func(ctx context.Context, op operation.Operation, object, oldObject interface{}) field.ErrorList { + if tc.shouldPanic { + panic("test panic") + } + return tc.declarativeErrors + }) + + // Setup context + ctx := genericapirequest.WithRequestInfo(context.Background(), &genericapirequest.RequestInfo{ + APIGroup: "", + APIVersion: "v1", + Resource: "pods", + }) + + obj := &v1.Pod{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Pod"}, + ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, + } + + // Copy imperative errors because they might be modified/appended to + inputErrs := make(field.ErrorList, len(tc.imperativeErrors)) + copy(inputErrs, tc.imperativeErrors) + + opts := []ValidationConfig{} + if tc.containsDeclarativeNative { + opts = append(opts, WithDeclarativeNative()) + } + + gotErrs := ValidateDeclarativelyWithMigrationChecks(ctx, localScheme, obj, nil, inputErrs, operation.Create, opts...) + + if !equalErrorLists(gotErrs, tc.expectedErrors) { + t.Errorf("Expected errors: %v, got: %v", tc.expectedErrors, gotErrs) + } + }) + } +} From 42b8ceee213b76bba19f591e22b9dd2b5be6febf Mon Sep 17 00:00:00 2001 From: Lalit Chauhan Date: Mon, 12 Jan 2026 22:09:50 +0000 Subject: [PATCH 3/3] Address feedbacks --- .../apiserver/pkg/registry/rest/validate.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) 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 41d2ab1287e..72d4bce7ad7 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go @@ -345,11 +345,9 @@ func metricIdentifier(ctx context.Context, scheme *runtime.Scheme, obj runtime.O return identifier, errs } -// ValidateDeclarativelyWithMigrationChecks encapsulates the logic for running declarative validation. +// ValidateDeclarativelyWithMigrationChecks runs declarative validation, and conditionally compares results +// with imperative validation and merges errors based on the feature gate and `takeover` flag. // It proceeds if either the DeclarativeValidation feature gate is enabled or `containsDeclarativeNative` is set. -// The function generates a validation identifier, runs declarative validation, -// and conditionally compares results with imperative validation and merges errors based on the feature gate and `takeover` flag. -// Declarative-native errors are always appended to the result. 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) takeover := utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidationTakeover) @@ -370,7 +368,8 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti opt(cfg) } - if !declarativeValidationEnabled && !cfg.containsDeclarativeNative { + // Short-circuit if neither DeclarativeValidation is enabled nor the object contains declarative native validation. + if !(declarativeValidationEnabled || cfg.containsDeclarativeNative) { return errs } @@ -379,13 +378,16 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti mirroredDVErrors := field.ErrorList{} dvNativeErrors := field.ErrorList{} + + // When declarative native validation is present, we need to separate declarative native errors + // from mirrored declarative errors. This is to avoid comparing declarative native errors (which + // have no imperative equivalent) with handwritten imperative errors. if cfg.containsDeclarativeNative { for _, err := range declarativeErrs { if err.DeclarativeNative { dvNativeErrors = append(dvNativeErrors, err) - // Internal Error is very unlikely to happen, if that happen it should be fail the validations - // for both of types of validations. } else if err.Type == field.ErrorTypeInternal { + // Internal errors should fail both types of validation. dvNativeErrors = append(dvNativeErrors, err) mirroredDVErrors = append(mirroredDVErrors, err) } else {