From 5a5ed81e1f2b8650c570e7907534cf527c8d1c41 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 12 Mar 2025 15:07:40 -0400 Subject: [PATCH] ReplicationController: Enable declarative validation After declarative validation is enabled in the ReplicationController strategy in this way, the generated declarative validation code in pkg/apis/core/v1/zz.generated.validations.go will be run when the strategy validates ReplicationController. Co-authored-by: Tim Hockin Co-authored-by: Aaron Prindle Co-authored-by: Yongrui Lin Co-authored-by: David Eads --- .../core/replicationcontroller/strategy.go | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/pkg/registry/core/replicationcontroller/strategy.go b/pkg/registry/core/replicationcontroller/strategy.go index cdb32dd5f28..734b13d094e 100644 --- a/pkg/registry/core/replicationcontroller/strategy.go +++ b/pkg/registry/core/replicationcontroller/strategy.go @@ -37,11 +37,13 @@ import ( "k8s.io/apiserver/pkg/registry/rest" apistorage "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/pod" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper" corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" + "k8s.io/kubernetes/pkg/features" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -123,7 +125,27 @@ func (rcStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) func (rcStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { controller := obj.(*api.ReplicationController) opts := pod.GetValidationOptionsFromPodTemplate(controller.Spec.Template, nil) - return corevalidation.ValidateReplicationController(controller, opts) + + // Run imperative validation + allErrs := corevalidation.ValidateReplicationController(controller, opts) + + // If DeclarativeValidation feature gate is enabled, also run declarative validation + if utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidation) { + // Determine if takeover is enabled + takeover := utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidationTakeover) + + // Run declarative validation with panic recovery + declarativeErrs := rest.ValidateDeclarativelyWithRecovery(ctx, nil, legacyscheme.Scheme, controller, takeover) + + // Compare imperative and declarative errors and log + emit metric if there's a mismatch + rest.CompareDeclarativeErrorsAndEmitMismatches(ctx, allErrs, declarativeErrs, takeover) + + // Only apply declarative errors if takeover is enabled + if takeover { + allErrs = append(allErrs.RemoveCoveredByDeclarative(), declarativeErrs...) + } + } + return allErrs } // WarningsOnCreate returns warnings for the creation of the given object. @@ -153,6 +175,8 @@ func (rcStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) f newRc := obj.(*api.ReplicationController) opts := pod.GetValidationOptionsFromPodTemplate(newRc.Spec.Template, oldRc.Spec.Template) + // FIXME: Calling both validator functions here results in redundant calls to ValidateReplicationControllerSpec. + // This should be fixed to avoid the redundant calls, but carefully. validationErrorList := corevalidation.ValidateReplicationController(newRc, opts) updateErrorList := corevalidation.ValidateReplicationControllerUpdate(newRc, oldRc, opts) errs := append(validationErrorList, updateErrorList...) @@ -174,6 +198,23 @@ func (rcStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) f } } + // If DeclarativeValidation feature gate is enabled, also run declarative validation + if utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidation) { + // Determine if takeover is enabled + takeover := utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidationTakeover) + + // Run declarative update validation with panic recovery + declarativeErrs := rest.ValidateUpdateDeclarativelyWithRecovery(ctx, nil, legacyscheme.Scheme, newRc, oldRc, takeover) + + // Compare imperative and declarative errors and emit metric if there's a mismatch + rest.CompareDeclarativeErrorsAndEmitMismatches(ctx, errs, declarativeErrs, takeover) + + // Only apply declarative errors if takeover is enabled + if takeover { + errs = append(errs.RemoveCoveredByDeclarative(), declarativeErrs...) + } + } + return errs }