From e7a9a148489b868c4f41c01818e51e6540d4674b Mon Sep 17 00:00:00 2001 From: Alexander Zielenski Date: Tue, 24 Oct 2023 11:22:59 -0700 Subject: [PATCH] replace CEL ValidateWithRatcheting with variadic options so we can now supply a shared CorrelatedObject --- .../pkg/apiserver/schema/cel/validation.go | 27 ++++++++++++------- .../apiserver/schema/cel/validation_test.go | 4 ++- .../pkg/registry/customresource/strategy.go | 7 +++-- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go index 128c5b05418..14e48c4c76f 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go @@ -144,20 +144,29 @@ func validator(s *schema.Structural, isResourceRoot bool, declType *cel.DeclType return nil } +type options struct { + ratchetingOptions +} + +type Option func(*options) + +func WithRatcheting(correlation *common.CorrelatedObject) Option { + return func(o *options) { + o.currentCorrelation = correlation + } +} + // Validate validates all x-kubernetes-validations rules in Validator against obj and returns any errors. // If the validation rules exceed the costBudget, subsequent evaluations will be skipped, the list of errs returned will not be empty, and a negative remainingBudget will be returned. // Most callers can ignore the returned remainingBudget value unless another validate call is going to be made // context is passed for supporting context cancellation during cel validation -func (s *Validator) Validate(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) { - return s.validate(ctx, fldPath, sts, obj, oldObj, ratchetingOptions{}, costBudget) -} +func (s *Validator) Validate(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj interface{}, costBudget int64, opts ...Option) (errs field.ErrorList, remainingBudget int64) { + opt := options{} + for _, o := range opts { + o(&opt) + } -func (s *Validator) ValidateWithRatcheting(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) { - // May be a worthwhile optimization to share the correlated object instance - // with the OpenAPI schema validator to avoid doing DeepEqual twice - return s.validate(ctx, fldPath, sts, obj, oldObj, ratchetingOptions{ - currentCorrelation: common.NewCorrelatedObject(obj, oldObj, &model.Structural{Structural: sts}), - }, costBudget) + return s.validate(ctx, fldPath, sts, obj, oldObj, opt.ratchetingOptions, costBudget) } // ratchetingOptions stores the current correlation object and the nearest diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go index 2b662534a53..8d67ac15021 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go @@ -39,6 +39,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/yaml" celconfig "k8s.io/apiserver/pkg/apis/cel" + "k8s.io/apiserver/pkg/cel/common" "k8s.io/apiserver/pkg/warning" ) @@ -3564,13 +3565,14 @@ func TestRatcheting(t *testing.T) { if budget == 0 { budget = celconfig.RuntimeCELCostBudget } - errs, _ := validator.ValidateWithRatcheting( + errs, _ := validator.Validate( ctx, field.NewPath("root"), c.schema, c.newObj, c.oldObj, budget, + WithRatcheting(common.NewCorrelatedObject(c.newObj, c.oldObj, &model.Structural{Structural: c.schema})), ) require.Len(t, errs, len(c.errors), "must have expected number of errors") diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go index 254fe6754b2..3852125c23d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go @@ -246,9 +246,11 @@ func (a customResourceStrategy) ValidateUpdate(ctx context.Context, obj, old run } var options []validation.ValidationOption + var celOptions []cel.Option if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CRDValidationRatcheting) { correlatedObject := common.NewCorrelatedObject(uNew.Object, uOld.Object, &model.Structural{Structural: a.structuralSchema}) options = append(options, validation.WithRatcheting(correlatedObject)) + celOptions = append(celOptions, cel.WithRatcheting(correlatedObject)) } var errs field.ErrorList @@ -266,11 +268,8 @@ func (a customResourceStrategy) ValidateUpdate(ctx context.Context, obj, old run if celValidator := a.celValidator; celValidator != nil { if has, err := hasBlockingErr(errs); has { errs = append(errs, err) - } else if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CRDValidationRatcheting) { - err, _ := celValidator.ValidateWithRatcheting(ctx, nil, a.structuralSchema, uNew.Object, uOld.Object, celconfig.RuntimeCELCostBudget) - errs = append(errs, err...) } else { - err, _ := celValidator.Validate(ctx, nil, a.structuralSchema, uNew.Object, uOld.Object, celconfig.RuntimeCELCostBudget) + err, _ := celValidator.Validate(ctx, nil, a.structuralSchema, uNew.Object, uOld.Object, celconfig.RuntimeCELCostBudget, celOptions...) errs = append(errs, err...) } }