diff --git a/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go b/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go index b400b8cf71d..2989810b533 100644 --- a/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go +++ b/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go @@ -121,7 +121,6 @@ func (priorityLevelConfigurationStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (priorityLevelConfigurationStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { newPL := obj.(*flowcontrol.PriorityLevelConfiguration) - _ = old.(*flowcontrol.PriorityLevelConfiguration) // 1.28 server is not aware of the roundtrip annotation, and will // default any 0 value persisted (for the NominalConcurrencyShares diff --git a/pkg/registry/scheduling/podgroup/strategy.go b/pkg/registry/scheduling/podgroup/strategy.go index fb83a876e10..754320c7158 100644 --- a/pkg/registry/scheduling/podgroup/strategy.go +++ b/pkg/registry/scheduling/podgroup/strategy.go @@ -75,17 +75,6 @@ func (*podGroupStrategy) Validate(ctx context.Context, obj runtime.Object) field return validation.ValidatePodGroup(podGroup) } -// DeclarativeValidationConfig implements rest.DeclarativeValidationConfigurer to supply declarative -// validation options to the generic BeforeCreate/BeforeUpdate code path. -// -// TODO: Behavior drift introduced when wiring declarative validation -// universally: the status substrategy embeds *podGroupStrategy and now -// inherits this method, so status updates pick up the full feature-gated -// options list and rest.WithDeclarativeEnforcement(). Previously the -// pre-migration inline call on the status subresource only passed -// WithDeclarativeEnforcement() (no options list). The change is additive but -// should be reviewed; if the status subresource should not see these -// options, override ValidationConfig on podGroupStatusStrategy. func (*podGroupStrategy) DeclarativeValidationConfig(ctx context.Context, obj, oldObj runtime.Object) rest.DeclarativeValidationConfig { opts := []string{} if utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareWorkloadScheduling) { @@ -97,7 +86,7 @@ func (*podGroupStrategy) DeclarativeValidationConfig(ctx context.Context, obj, o if utilfeature.DefaultFeatureGate.Enabled(features.WorkloadAwarePreemption) { opts = append(opts, string(features.WorkloadAwarePreemption)) } - return rest.DeclarativeValidationConfig{DeclarativeEnforcement: true, Options: opts} + return rest.DeclarativeValidationConfig{Options: opts} } func (*podGroupStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { 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 c8b537e510d..948386eafee 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go @@ -63,6 +63,7 @@ type DeclarativeValidation struct { func (d DeclarativeValidation) ValidateDeclaratively(ctx context.Context, obj, oldObj runtime.Object, validationErrs field.ErrorList, opType operation.Type, config DeclarativeValidationConfig) field.ErrorList { if d.Scheme == nil { + validationErrs = append(validationErrs, field.InternalError(nil, fmt.Errorf("cannot validate declaratively without a scheme"))) return validationErrs } return ValidateDeclarativelyWithMigrationChecks(ctx, d.Scheme, obj, oldObj, validationErrs, opType, config) @@ -337,11 +338,9 @@ func createDeclarativeValidationPanicHandler(ctx context.Context, errs *field.Er // if shouldFail=false, and adding a validation error if shouldFail=true. func panicSafeValidateFunc( validateFunc func(ctx context.Context, scheme *runtime.Scheme, obj, oldObj runtime.Object, o *ValidationConfigOption) field.ErrorList, - 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, shouldFail, validationIdentifier)() - + defer createDeclarativeValidationPanicHandler(ctx, &errs, o.DeclarativeEnforcement, o.ValidationIdentifier)() return validateFunc(ctx, scheme, obj, oldObj, o) } } @@ -422,7 +421,7 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti // Call the panic-safe wrapper with the real validation function. // We should fail if validation is enforced. - declarativeErrs := panicSafeValidateFunc(validateDeclaratively, cfg.DeclarativeEnforcement, cfg.ValidationIdentifier)(ctx, scheme, obj, oldObj, cfg) + declarativeErrs := panicSafeValidateFunc(validateDeclaratively)(ctx, scheme, obj, oldObj, cfg) if declarativeValidationEnabled { // Log mismatches. 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 c38c2b790f1..1ea2386fe4a 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 @@ -176,7 +176,7 @@ func TestValidateDeclaratively(t *testing.T) { cfg.OpType = operation.Update } // takeover is not used here, passing false for shouldFail - results := panicSafeValidateFunc(validateDeclaratively, false, cfg.ValidationIdentifier)(ctx, scheme, tc.object, tc.oldObject, cfg) + results := panicSafeValidateFunc(validateDeclaratively)(ctx, scheme, tc.object, tc.oldObject, cfg) matcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin() matcher.Test(t, tc.expected, results) }) @@ -503,8 +503,8 @@ func TestWithRecover(t *testing.T) { defer klog.LogToStderr(true) // Pass the enforcement flag to panicSafeValidateFunc - wrapped := panicSafeValidateFunc(tc.validateFn, tc.enforcementEnabled, "test_validationIdentifier") - gotErrs := wrapped(ctx, scheme, obj, nil, &ValidationConfigOption{OpType: operation.Create, DeclarativeValidationConfig: DeclarativeValidationConfig{Options: options}}) + wrapped := panicSafeValidateFunc(tc.validateFn) + gotErrs := wrapped(ctx, scheme, obj, nil, &ValidationConfigOption{ValidationIdentifier: "test_validationIdentifier", OpType: operation.Create, DeclarativeValidationConfig: DeclarativeValidationConfig{Options: options, DeclarativeEnforcement: tc.enforcementEnabled}}) klog.Flush() logOutput := buf.String() @@ -597,8 +597,8 @@ func TestWithRecoverUpdate(t *testing.T) { defer klog.LogToStderr(true) // Pass the enforcement flag to panicSafeValidateUpdateFunc - wrapped := panicSafeValidateFunc(tc.validateFn, tc.enforcementEnabled, "test_validationIdentifier") - gotErrs := wrapped(ctx, scheme, obj, oldObj, &ValidationConfigOption{OpType: operation.Update, DeclarativeValidationConfig: DeclarativeValidationConfig{Options: options}}) + wrapped := panicSafeValidateFunc(tc.validateFn) + gotErrs := wrapped(ctx, scheme, obj, oldObj, &ValidationConfigOption{ValidationIdentifier: "test_validationIdentifier", OpType: operation.Update, DeclarativeValidationConfig: DeclarativeValidationConfig{Options: options, DeclarativeEnforcement: tc.enforcementEnabled}}) klog.Flush() logOutput := buf.String()