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/apiserver/schema/defaulting/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go index cd7c6e07558..02945b14f4c 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go +++ b/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)) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting.go index 0609d8eb66e..3cc653e7466 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting.go +++ b/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) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting_test.go index 078bb548cdc..c136617218c 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting_test.go +++ b/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.False(t, validator.ValidateUpdate(1, 2).IsValid()) + require.True(t, validator.ValidateUpdate(1, 1, validation.WithRatcheting(nil)).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 { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go index e0042356ac0..7304018fb34 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go +++ b/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 { - return s.Validate(new) +func (s basicSchemaValidator) Validate(new interface{}, options ...ValidationOption) *validate.Result { + 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 } 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 403535de5f3..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 @@ -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...) } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go index a1a15108d6a..adcb5c22024 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go +++ b/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)...)