Merge pull request #121343 from alexzielenski/apiserver/apiextensions/ratcheting-metrics

KEP-4008: CRDValidationRatcheting: Shared CorrelatedObject for Schema and CEL Validators

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) {
+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) {
-	return s.validate(ctx, fldPath, sts, obj, oldObj, ratchetingOptions{}, costBudget)
+	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) {
+	return s.validate(ctx, fldPath, sts, obj, oldObj, opt.ratchetingOptions, costBudget)
-	// 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)
 }
 
 // ratchetingOptions stores the current correlation object and the nearest

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")

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go

@@ -21,9 +21,6 @@ import (
 	"fmt"
 	"reflect"
 
-	"k8s.io/kube-openapi/pkg/validation/strfmt"
-	kubeopenapivalidate "k8s.io/kube-openapi/pkg/validation/validate"
-
 	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel"
 	schemaobjectmeta "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta"
@@ -74,7 +71,7 @@ func validate(ctx context.Context, pth *field.Path, s *structuralschema.Structur
 	isResourceRoot := s == rootSchema
 
 	if s.Default.Object != nil {
-		validator := kubeopenapivalidate.NewSchemaValidator(s.ToKubeOpenAPI(), nil, "", strfmt.Default)
+		validator := apiservervalidation.NewSchemaValidatorFromOpenAPI(s.ToKubeOpenAPI())
 
 		if insideMeta {
 			obj, _, err := f(runtime.DeepCopyJSONValue(s.Default.Object))

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting.go

@@ -54,14 +54,26 @@ func NewRatchetingSchemaValidator(schema *spec.Schema, rootSchema interface{}, r
 	}
 }
 
-func (r *RatchetingSchemaValidator) Validate(new interface{}) *validate.Result {
+func (r *RatchetingSchemaValidator) Validate(new interface{}, options ...ValidationOption) *validate.Result {
 	sv := validate.NewSchemaValidator(r.schema, r.root, r.path, r.knownFormats, r.options...)
 	return sv.Validate(new)
 }
 
-func (r *RatchetingSchemaValidator) ValidateUpdate(new, old interface{}) *validate.Result {
+func (r *RatchetingSchemaValidator) ValidateUpdate(new, old interface{}, options ...ValidationOption) *validate.Result {
+	opts := NewValidationOptions(options...)
+
+	if !opts.Ratcheting {
+		sv := validate.NewSchemaValidator(r.schema, r.root, r.path, r.knownFormats, r.options...)
+		return sv.Validate(new)
+	}
+
+	correlation := opts.CorrelatedObject
+	if correlation == nil {
+		correlation = common.NewCorrelatedObject(new, old, &celopenapi.Schema{Schema: r.schema})
+	}
+
 	return newRatchetingValueValidator(
-		common.NewCorrelatedObject(new, old, &celopenapi.Schema{Schema: r.schemaArgs.schema}),
+		correlation,
 		r.schemaArgs,
 	).Validate(new)
 }

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting_test.go

@@ -58,8 +58,8 @@ var largeIntSchema *spec.Schema = &spec.Schema{
 
 func TestScalarRatcheting(t *testing.T) {
 	validator := validation.NewRatchetingSchemaValidator(mediumIntSchema, nil, "", strfmt.Default)
-	require.True(t, validator.ValidateUpdate(1, 1).IsValid())
+	require.True(t, validator.ValidateUpdate(1, 1, validation.WithRatcheting(nil)).IsValid())
-	require.False(t, validator.ValidateUpdate(1, 2).IsValid())
+	require.False(t, validator.ValidateUpdate(1, 2, validation.WithRatcheting(nil)).IsValid())
 }
 
 var objectSchema *spec.Schema = &spec.Schema{
@@ -90,18 +90,18 @@ func TestObjectScalarFieldsRatcheting(t *testing.T) {
 		"small": 500,
 	}, map[string]interface{}{
 		"small": 500,
-	}).IsValid())
+	}, validation.WithRatcheting(nil)).IsValid())
 	assert.True(t, validator.ValidateUpdate(map[string]interface{}{
 		"small": 501,
 	}, map[string]interface{}{
 		"small":  501,
 		"medium": 500,
-	}).IsValid())
+	}, validation.WithRatcheting(nil)).IsValid())
 	assert.False(t, validator.ValidateUpdate(map[string]interface{}{
 		"small": 500,
 	}, map[string]interface{}{
 		"small": 501,
-	}).IsValid())
+	}, validation.WithRatcheting(nil)).IsValid())
 }
 
 // Shows schemas with object fields which themselves are ratcheted can be ratcheted
@@ -113,7 +113,7 @@ func TestObjectObjectFieldsRatcheting(t *testing.T) {
 		}}, map[string]interface{}{
 		"nested": map[string]interface{}{
 			"small": 500,
-		}}).IsValid())
+		}}, validation.WithRatcheting(nil)).IsValid())
 	assert.True(t, validator.ValidateUpdate(map[string]interface{}{
 		"nested": map[string]interface{}{
 			"small": 501,
@@ -121,14 +121,14 @@ func TestObjectObjectFieldsRatcheting(t *testing.T) {
 		"nested": map[string]interface{}{
 			"small":  501,
 			"medium": 500,
-		}}).IsValid())
+		}}, validation.WithRatcheting(nil)).IsValid())
 	assert.False(t, validator.ValidateUpdate(map[string]interface{}{
 		"nested": map[string]interface{}{
 			"small": 500,
 		}}, map[string]interface{}{
 		"nested": map[string]interface{}{
 			"small": 501,
-		}}).IsValid())
+		}}, validation.WithRatcheting(nil)).IsValid())
 }
 
 func ptr[T any](v T) *T {

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go

@@ -24,6 +24,7 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apiextensions-apiserver/pkg/features"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/apiserver/pkg/cel/common"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	openapierrors "k8s.io/kube-openapi/pkg/validation/errors"
 	"k8s.io/kube-openapi/pkg/validation/spec"
@@ -33,22 +34,60 @@ import (
 
 type SchemaValidator interface {
 	SchemaCreateValidator
-	ValidateUpdate(new, old interface{}) *validate.Result
+	ValidateUpdate(new, old interface{}, options ...ValidationOption) *validate.Result
 }
 
 type SchemaCreateValidator interface {
-	Validate(value interface{}) *validate.Result
+	Validate(value interface{}, options ...ValidationOption) *validate.Result
+}
+
+type ValidationOptions struct {
+	// Whether errors from unchanged portions of the schema should be ratcheted
+	// This field is ignored for Validate
+	Ratcheting bool
+
+	// Correlation between old and new arguments.
+	// If set, this is expected to be the correlation between the `new` and
+	// `old` arguments to ValidateUpdate, and values for `new` and `old` will
+	// be taken from the correlation.
+	//
+	// This field is ignored for Validate
+	//
+	// Used for ratcheting, but left as a separate field since it may be used
+	// for other purposes in the future.
+	CorrelatedObject *common.CorrelatedObject
+}
+
+type ValidationOption func(*ValidationOptions)
+
+func NewValidationOptions(opts ...ValidationOption) ValidationOptions {
+	options := ValidationOptions{}
+	for _, opt := range opts {
+		opt(&options)
+	}
+	return options
+}
+
+func WithRatcheting(correlation *common.CorrelatedObject) ValidationOption {
+	return func(options *ValidationOptions) {
+		options.Ratcheting = true
+		options.CorrelatedObject = correlation
+	}
 }
 
 // basicSchemaValidator wraps a kube-openapi SchemaCreateValidator to
 // support ValidateUpdate. It implements ValidateUpdate by simply validating
-// the new value via kube-openapi, ignoring the old value.
+// the new value via kube-openapi, ignoring the old value
 type basicSchemaValidator struct {
 	*validate.SchemaValidator
 }
 
-func (s basicSchemaValidator) ValidateUpdate(new, old interface{}) *validate.Result {
+func (s basicSchemaValidator) Validate(new interface{}, options ...ValidationOption) *validate.Result {
-	return s.Validate(new)
+	return s.SchemaValidator.Validate(new)
+}
+
+func (s basicSchemaValidator) ValidateUpdate(new, old interface{}, options ...ValidationOption) *validate.Result {
+	return s.Validate(new, options...)
 }
 
 // NewSchemaValidator creates an openapi schema validator for the given CRD validation.
@@ -67,11 +106,15 @@ func NewSchemaValidator(customResourceValidation *apiextensions.JSONSchemaProps)
 			return nil, nil, err
 		}
 	}
+	return NewSchemaValidatorFromOpenAPI(openapiSchema), openapiSchema, nil
+}
 
+func NewSchemaValidatorFromOpenAPI(openapiSchema *spec.Schema) SchemaValidator {
 	if utilfeature.DefaultFeatureGate.Enabled(features.CRDValidationRatcheting) {
-		return NewRatchetingSchemaValidator(openapiSchema, nil, "", strfmt.Default), openapiSchema, nil
+		return NewRatchetingSchemaValidator(openapiSchema, nil, "", strfmt.Default)
 	}
-	return basicSchemaValidator{validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default)}, openapiSchema, nil
+	return basicSchemaValidator{validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default)}
+
 }
 
 // ValidateCustomResourceUpdate validates the transition of Custom Resource from
@@ -80,7 +123,7 @@ func NewSchemaValidator(customResourceValidation *apiextensions.JSONSchemaProps)
 //
 // If feature `CRDValidationRatcheting` is disabled, this behaves identically to
 // ValidateCustomResource(customResource).
-func ValidateCustomResourceUpdate(fldPath *field.Path, customResource, old interface{}, validator SchemaValidator) field.ErrorList {
+func ValidateCustomResourceUpdate(fldPath *field.Path, customResource, old interface{}, validator SchemaValidator, options ...ValidationOption) field.ErrorList {
 	// Additional feature gate check for sanity
 	if !utilfeature.DefaultFeatureGate.Enabled(features.CRDValidationRatcheting) {
 		return ValidateCustomResource(nil, customResource, validator)
@@ -88,7 +131,7 @@ func ValidateCustomResourceUpdate(fldPath *field.Path, customResource, old inter
 		return nil
 	}
 
-	result := validator.ValidateUpdate(customResource, old)
+	result := validator.ValidateUpdate(customResource, old, options...)
 	if result.IsValid() {
 		return nil
 	}
@@ -98,12 +141,12 @@ func ValidateCustomResourceUpdate(fldPath *field.Path, customResource, old inter
 
 // ValidateCustomResource validates the Custom Resource against the schema in the CustomResourceDefinition.
 // CustomResource is a JSON data structure.
-func ValidateCustomResource(fldPath *field.Path, customResource interface{}, validator SchemaCreateValidator) field.ErrorList {
+func ValidateCustomResource(fldPath *field.Path, customResource interface{}, validator SchemaCreateValidator, options ...ValidationOption) field.ErrorList {
 	if validator == nil {
 		return nil
 	}
 
-	result := validator.Validate(customResource)
+	result := validator.Validate(customResource, options...)
 	if result.IsValid() {
 		return nil
 	}

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel"
+	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model"
 	structurallisttype "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype"
 	schemaobjectmeta "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
@@ -38,6 +39,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	celconfig "k8s.io/apiserver/pkg/apis/cel"
+	"k8s.io/apiserver/pkg/cel/common"
 	"k8s.io/apiserver/pkg/features"
 	apiserverstorage "k8s.io/apiserver/pkg/storage"
 	"k8s.io/apiserver/pkg/storage/names"
@@ -243,8 +245,16 @@ func (a customResourceStrategy) ValidateUpdate(ctx context.Context, obj, old run
 		return field.ErrorList{field.Invalid(field.NewPath(""), old, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", old))}
 	}
 
+	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
-	errs = append(errs, a.validator.ValidateUpdate(ctx, uNew, uOld, a.scale)...)
+	errs = append(errs, a.validator.ValidateUpdate(ctx, uNew, uOld, a.scale, options...)...)
 
 	// Checks the embedded objects. We don't make a difference between update and create for those.
 	errs = append(errs, schemaobjectmeta.Validate(nil, uNew.Object, a.structuralSchema, false)...)
@@ -258,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...)
 		}
 	}

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go

@@ -58,7 +58,7 @@ func (a customResourceValidator) Validate(ctx context.Context, obj *unstructured
 	return allErrs
 }
 
-func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old *unstructured.Unstructured, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
+func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old *unstructured.Unstructured, scale *apiextensions.CustomResourceSubresourceScale, options ...apiextensionsvalidation.ValidationOption) field.ErrorList {
 	if errs := a.ValidateTypeMeta(ctx, obj); len(errs) > 0 {
 		return errs
 	}
@@ -66,7 +66,7 @@ func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old *u
 	var allErrs field.ErrorList
 
 	allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(obj, old, field.NewPath("metadata"))...)
-	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, obj.UnstructuredContent(), old.UnstructuredContent(), a.schemaValidator)...)
+	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, obj.UnstructuredContent(), old.UnstructuredContent(), a.schemaValidator, options...)...)
 	allErrs = append(allErrs, a.ValidateScaleSpec(ctx, obj, scale)...)
 	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, obj, scale)...)