Merge pull request #121016 from alexzielenski/apiserver/apiextensions/ratcheting-cel

CRDValidationRatcheting: Ratchet errors from CEL expressions if `old` DeepEqual `new`

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

@@ -38,8 +38,10 @@ import (
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/cel"
+	"k8s.io/apiserver/pkg/cel/common"
 	"k8s.io/apiserver/pkg/cel/environment"
 	"k8s.io/apiserver/pkg/cel/metrics"
+	"k8s.io/apiserver/pkg/warning"
 
 	celconfig "k8s.io/apiserver/pkg/apis/cel"
 )
@@ -147,6 +149,71 @@ func validator(s *schema.Structural, isResourceRoot bool, declType *cel.DeclType
 // 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) 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)
+}
+
+// ratchetingOptions stores the current correlation object and the nearest
+// parent which was correlatable. The parent is stored so that we can check at
+// the point an error is thrown whether it should be ratcheted using simple
+// logic
+// Key and Index should be used as normally to traverse to the next node.
+type ratchetingOptions struct {
+	// Current correlation object. If nil, then this node is from an uncorrelatable
+	// part of the schema
+	currentCorrelation *common.CorrelatedObject
+
+	// If currentCorrelation is nil, this is the nearest parent to this node
+	// which was correlatable. If the parent is deepequal to its old value,
+	// then errors thrown by this node are ratcheted
+	nearestParentCorrelation *common.CorrelatedObject
+}
+
+// shouldRatchetError returns true if the errors raised by the current node
+// should be ratcheted.
+//
+// Errors for the current node should be ratcheted if one of the following is true:
+//  1. The current node is correlatable, and it is equal to its old value
+//  2. The current node has a correlatable ancestor, and the ancestor is equal
+//     to its old value.
+func (r ratchetingOptions) shouldRatchetError() bool {
+	if r.currentCorrelation != nil {
+		return r.currentCorrelation.CachedDeepEqual()
+	}
+
+	return r.nearestParentCorrelation.CachedDeepEqual()
+}
+
+// Finds the next node following the field in the tree and returns options using
+// that node. If none could be found, then retains a reference to the last
+// correlatable ancestor for ratcheting purposes
+func (r ratchetingOptions) key(field string) ratchetingOptions {
+	if r.currentCorrelation == nil {
+		return r
+	}
+
+	return ratchetingOptions{currentCorrelation: r.currentCorrelation.Key(field), nearestParentCorrelation: r.currentCorrelation}
+}
+
+// Finds the next node following the index in the tree and returns options using
+// that node. If none could be found, then retains a reference to the last
+// correlatable ancestor for ratcheting purposes
+func (r ratchetingOptions) index(idx int) ratchetingOptions {
+	if r.currentCorrelation == nil {
+		return r
+	}
+
+	return ratchetingOptions{currentCorrelation: r.currentCorrelation.Index(idx), nearestParentCorrelation: r.currentCorrelation}
+}
+
+func (s *Validator) validate(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj interface{}, correlation ratchetingOptions, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
 	t := time.Now()
 	defer func() {
 		metrics.Metrics.ObserveEvaluation(time.Since(t))
@@ -156,28 +223,31 @@ func (s *Validator) Validate(ctx context.Context, fldPath *field.Path, sts *sche
 		return nil, remainingBudget
 	}
 
-	errs, remainingBudget = s.validateExpressions(ctx, fldPath, sts, obj, oldObj, remainingBudget)
+	errs, remainingBudget = s.validateExpressions(ctx, fldPath, sts, obj, oldObj, correlation, remainingBudget)
+
 	if remainingBudget < 0 {
 		return errs, remainingBudget
 	}
+
 	switch obj := obj.(type) {
 	case []interface{}:
 		oldArray, _ := oldObj.([]interface{})
 		var arrayErrs field.ErrorList
-		arrayErrs, remainingBudget = s.validateArray(ctx, fldPath, sts, obj, oldArray, remainingBudget)
+		arrayErrs, remainingBudget = s.validateArray(ctx, fldPath, sts, obj, oldArray, correlation, remainingBudget)
 		errs = append(errs, arrayErrs...)
 		return errs, remainingBudget
 	case map[string]interface{}:
 		oldMap, _ := oldObj.(map[string]interface{})
 		var mapErrs field.ErrorList
-		mapErrs, remainingBudget = s.validateMap(ctx, fldPath, sts, obj, oldMap, remainingBudget)
+		mapErrs, remainingBudget = s.validateMap(ctx, fldPath, sts, obj, oldMap, correlation, remainingBudget)
 		errs = append(errs, mapErrs...)
 		return errs, remainingBudget
 	}
+
 	return errs, remainingBudget
 }
 
-func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
+func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj interface{}, correlation ratchetingOptions, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
 	// guard against oldObj being a non-nil interface with a nil value
 	if oldObj != nil {
 		v := reflect.ValueOf(oldObj)
@@ -263,26 +333,35 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
 			if len(compiled.NormalizedRuleFieldPath) > 0 {
 				fldPath = fldPath.Child(compiled.NormalizedRuleFieldPath)
 			}
+
+			addErr := func(e *field.Error) {
+				if !compiled.TransitionRule && correlation.shouldRatchetError() {
+					warning.AddWarning(ctx, "", e.Error())
+				} else {
+					errs = append(errs, e)
+				}
+			}
+
 			if compiled.MessageExpression != nil {
 				messageExpression, newRemainingBudget, msgErr := evalMessageExpression(ctx, compiled.MessageExpression, rule.MessageExpression, activation, remainingBudget)
 				if msgErr != nil {
 					if msgErr.Type == cel.ErrorTypeInternal {
-						errs = append(errs, field.InternalError(fldPath, msgErr))
+						addErr(field.InternalError(fldPath, msgErr))
 						return errs, -1
 					} else if msgErr.Type == cel.ErrorTypeInvalid {
-						errs = append(errs, field.Invalid(fldPath, sts.Type, msgErr.Error()))
+						addErr(field.Invalid(fldPath, sts.Type, msgErr.Error()))
 						return errs, -1
 					} else {
 						klog.V(2).ErrorS(msgErr, "messageExpression evaluation failed")
-						errs = append(errs, fieldErrorForReason(fldPath, sts.Type, ruleMessageOrDefault(rule), rule.Reason))
+						addErr(fieldErrorForReason(fldPath, sts.Type, ruleMessageOrDefault(rule), rule.Reason))
 						remainingBudget = newRemainingBudget
 					}
 				} else {
-					errs = append(errs, fieldErrorForReason(fldPath, sts.Type, messageExpression, rule.Reason))
+					addErr(fieldErrorForReason(fldPath, sts.Type, messageExpression, rule.Reason))
 					remainingBudget = newRemainingBudget
 				}
 			} else {
-				errs = append(errs, fieldErrorForReason(fldPath, sts.Type, ruleMessageOrDefault(rule), rule.Reason))
+				addErr(fieldErrorForReason(fldPath, sts.Type, ruleMessageOrDefault(rule), rule.Reason))
 			}
 		}
 	}
@@ -566,7 +645,7 @@ func (a *validationActivation) Parent() interpreter.Activation {
 	return nil
 }
 
-func (s *Validator) validateMap(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj map[string]interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
+func (s *Validator) validateMap(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj map[string]interface{}, correlation ratchetingOptions, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
 	remainingBudget = costBudget
 	if remainingBudget < 0 {
 		return errs, remainingBudget
@@ -585,7 +664,7 @@ func (s *Validator) validateMap(ctx context.Context, fldPath *field.Path, sts *s
 			}
 
 			var err field.ErrorList
-			err, remainingBudget = s.AdditionalProperties.Validate(ctx, fldPath.Key(k), sts.AdditionalProperties.Structural, v, oldV, remainingBudget)
+			err, remainingBudget = s.AdditionalProperties.validate(ctx, fldPath.Key(k), sts.AdditionalProperties.Structural, v, oldV, correlation.key(k), remainingBudget)
 			errs = append(errs, err...)
 			if remainingBudget < 0 {
 				return errs, remainingBudget
@@ -603,7 +682,7 @@ func (s *Validator) validateMap(ctx context.Context, fldPath *field.Path, sts *s
 				}
 
 				var err field.ErrorList
-				err, remainingBudget = sub.Validate(ctx, fldPath.Child(k), &stsProp, v, oldV, remainingBudget)
+				err, remainingBudget = sub.validate(ctx, fldPath.Child(k), &stsProp, v, oldV, correlation.key(k), remainingBudget)
 				errs = append(errs, err...)
 				if remainingBudget < 0 {
 					return errs, remainingBudget
@@ -615,7 +694,7 @@ func (s *Validator) validateMap(ctx context.Context, fldPath *field.Path, sts *s
 	return errs, remainingBudget
 }
 
-func (s *Validator) validateArray(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj []interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
+func (s *Validator) validateArray(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj []interface{}, correlation ratchetingOptions, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
 	remainingBudget = costBudget
 	if remainingBudget < 0 {
 		return errs, remainingBudget
@@ -627,7 +706,7 @@ func (s *Validator) validateArray(ctx context.Context, fldPath *field.Path, sts
 		correlatableOldItems := makeMapList(sts, oldObj)
 		for i := range obj {
 			var err field.ErrorList
-			err, remainingBudget = s.Items.Validate(ctx, fldPath.Index(i), sts.Items, obj[i], correlatableOldItems.Get(obj[i]), remainingBudget)
+			err, remainingBudget = s.Items.validate(ctx, fldPath.Index(i), sts.Items, obj[i], correlatableOldItems.Get(obj[i]), correlation.index(i), remainingBudget)
 			errs = append(errs, err...)
 			if remainingBudget < 0 {
 				return errs, remainingBudget

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go

@@ -17,22 +17,29 @@ limitations under the License.
 package cel
 
 import (
+	"bytes"
 	"context"
 	"flag"
 	"fmt"
 	"math"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"k8s.io/klog/v2"
 	"k8s.io/kube-openapi/pkg/validation/strfmt"
 
+	apiextensionsinternal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/apimachinery/pkg/util/yaml"
 	celconfig "k8s.io/apiserver/pkg/apis/cel"
+	"k8s.io/apiserver/pkg/warning"
 )
 
 // TestValidationExpressions tests CEL integration with custom resource values and OpenAPIv3.
@@ -3014,6 +3021,590 @@ func TestValidateFieldPath(t *testing.T) {
 	}
 }
 
+// FixTabsOrDie counts the number of tab characters preceding the first
+// line in the given yaml object. It removes that many tabs from every
+// line. It panics (it's a test funvion) if some line has fewer tabs
+// than the first line.
+//
+// The purpose of this is to make it easier to read tests.
+func FixTabsOrDie(in string) string {
+	lines := bytes.Split([]byte(in), []byte{'\n'})
+	if len(lines[0]) == 0 && len(lines) > 1 {
+		lines = lines[1:]
+	}
+	// Create prefix made of tabs that we want to remove.
+	var prefix []byte
+	for _, c := range lines[0] {
+		if c != '\t' {
+			break
+		}
+		prefix = append(prefix, byte('\t'))
+	}
+	// Remove prefix from all tabs, fail otherwise.
+	for i := range lines {
+		line := lines[i]
+		// It's OK for the last line to be blank (trailing \n)
+		if i == len(lines)-1 && len(line) <= len(prefix) && bytes.TrimSpace(line) == nil {
+			lines[i] = []byte{}
+			break
+		}
+		if !bytes.HasPrefix(line, prefix) {
+			minRange := i - 5
+			maxRange := i + 5
+			if minRange < 0 {
+				minRange = 0
+			}
+			if maxRange > len(lines) {
+				maxRange = len(lines)
+			}
+			panic(fmt.Errorf("line %d doesn't start with expected number (%d) of tabs (%v-%v):\n%v", i, len(prefix), minRange, maxRange, string(bytes.Join(lines[minRange:maxRange], []byte{'\n'}))))
+		}
+		lines[i] = line[len(prefix):]
+	}
+
+	joined := string(bytes.Join(lines, []byte{'\n'}))
+
+	// Convert rest of tabs to spaces since yaml doesnt like yabs
+	// (assuming 2 space alignment)
+	return strings.ReplaceAll(joined, "\t", "  ")
+}
+
+// Creates a *spec.Schema Schema by decoding the given YAML. Panics on error
+func mustSchema(source string) *schema.Structural {
+	source = FixTabsOrDie(source)
+	d := yaml.NewYAMLOrJSONDecoder(strings.NewReader(source), 4096)
+	props := &apiextensions.JSONSchemaProps{}
+	if err := d.Decode(props); err != nil {
+		panic(err)
+	}
+	convertedProps := &apiextensionsinternal.JSONSchemaProps{}
+	if err := apiextensions.Convert_v1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(props, convertedProps, nil); err != nil {
+		panic(err)
+	}
+
+	res, err := schema.NewStructural(convertedProps)
+	if err != nil {
+		panic(err)
+	}
+	return res
+}
+
+// Creates an *unstructured by decoding the given YAML. Panics on error
+func mustUnstructured(source string) interface{} {
+	source = FixTabsOrDie(source)
+	d := yaml.NewYAMLOrJSONDecoder(strings.NewReader(source), 4096)
+	var res interface{}
+	if err := d.Decode(&res); err != nil {
+		panic(err)
+	}
+	return res
+}
+
+type warningRecorder struct {
+	mu       sync.Mutex
+	warnings []string
+}
+
+// AddWarning adds a warning to recorder.
+func (r *warningRecorder) AddWarning(agent, text string) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	r.warnings = append(r.warnings, text)
+}
+
+func (r *warningRecorder) Warnings() []string {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+
+	warnings := make([]string, len(r.warnings))
+	copy(warnings, r.warnings)
+	return warnings
+}
+
+func TestRatcheting(t *testing.T) {
+	cases := []struct {
+		name   string
+		schema *schema.Structural
+		oldObj interface{}
+		newObj interface{}
+
+		// Errors that should occur when evaluating this operation with
+		// ratcheting feature enabled
+		errors []string
+
+		// Errors that should occur when evaluating this operation with
+		// ratcheting feature disabled
+		// These errors should be raised as warnings when ratcheting is enabled
+		warnings []string
+
+		runtimeCostBudget int64
+	}{
+		{
+			name: "normal CEL expression",
+			schema: mustSchema(`
+				type: object
+				properties:
+					foo:
+						type: string
+						x-kubernetes-validations:
+						- rule: self == "bar"
+						  message: "gotta be baz"
+				`),
+			oldObj: mustUnstructured(`
+				foo: baz
+			`),
+			newObj: mustUnstructured(`
+				foo: baz
+			`),
+			warnings: []string{
+				`root.foo: Invalid value: "string": gotta be baz`,
+			},
+		},
+		{
+			name: "normal CEL expression thats a descendent of an atomic array whose parent is totally unchanged",
+			schema: mustSchema(`
+				type: array
+				x-kubernetes-list-type: atomic
+				items:
+					type: object
+					properties:
+						bar:
+							type: string
+							x-kubernetes-validations:
+							- rule: self == "baz"
+							  message: "gotta be baz"
+				`),
+			// CEL error comes from uncorrelatable portion of the schema,
+			// but it should be ratcheted anyway because it is the descendent
+			// of an unchanged correlatable node
+			oldObj: mustUnstructured(`
+				- bar: bar
+			`),
+			newObj: mustUnstructured(`
+				- bar: bar
+			`),
+			warnings: []string{
+				`root[0].bar: Invalid value: "string": gotta be baz`,
+			},
+		},
+		{
+			name: "normal CEL expression thats a descendent of a set whose parent is totally unchanged",
+			schema: mustSchema(`
+				type: array
+				x-kubernetes-list-type: set
+				items:
+					type: number
+					x-kubernetes-validations:
+					- rule: int(self) % 2 == 1
+					  message: "gotta be odd"
+				`),
+			// CEL error comes from uncorrelatable portion of the schema,
+			// but it should be ratcheted anyway because it is the descendent
+			// of an unchanged correlatable node
+			oldObj: mustUnstructured(`
+				- 1
+				- 2
+			`),
+			newObj: mustUnstructured(`
+				- 1
+				- 2
+			`),
+			warnings: []string{
+				`root[1]: Invalid value: "number": gotta be odd`,
+			},
+		},
+		{
+			name: "normal CEL expression thats a descendent of a set and one of its siblings has changed",
+			schema: mustSchema(`
+				type: object
+				properties:
+					stringField:
+						type: string
+					setArray:
+						type: array
+						x-kubernetes-list-type: set
+						items:
+							type: number
+							x-kubernetes-validations:
+							- rule: int(self) % 2 == 1
+							  message: "gotta be odd"
+				`),
+			oldObj: mustUnstructured(`
+				stringField: foo
+				setArray:
+				- 1
+				- 3
+				- 2
+			`),
+			newObj: mustUnstructured(`
+				stringField: changed but ratcheted
+				setArray:
+				- 1
+				- 3
+				- 2
+			`),
+			warnings: []string{
+				`root.setArray[2]: Invalid value: "number": gotta be odd`,
+			},
+		},
+		{
+			name: "descendent of a map list whose parent is unchanged",
+			schema: mustSchema(`
+				type: array
+				x-kubernetes-list-type: map
+				x-kubernetes-list-map-keys: ["key"]
+				items:
+					type: object
+					properties:
+						key:
+							type: string
+						value:
+							type: string
+							x-kubernetes-validations:
+							- rule: self == "baz"
+							  message: "gotta be baz"
+			`),
+			oldObj: mustUnstructured(`
+				- key: foo
+				  value: notbaz
+				- key: bar
+				  value: notbaz
+			`),
+			newObj: mustUnstructured(`
+				- key: foo
+				  value: notbaz
+				- key: bar
+				  value: notbaz
+				- key: baz
+				  value: baz
+			`),
+			warnings: []string{
+				`root[0].value: Invalid value: "string": gotta be baz`,
+				`root[1].value: Invalid value: "string": gotta be baz`,
+			},
+		},
+		{
+			name: "descendent of a map list whose siblings have changed",
+			schema: mustSchema(`
+				type: array
+				x-kubernetes-list-type: map
+				x-kubernetes-list-map-keys: ["key"]
+				items:
+					type: object
+					properties:
+						key:
+							type: string
+						value:
+							type: string
+							x-kubernetes-validations:
+							- rule: self == "baz"
+							  message: "gotta be baz"
+			`),
+			oldObj: mustUnstructured(`
+				- key: foo
+				  value: notbaz
+				- key: bar
+				  value: notbaz
+			`),
+			newObj: mustUnstructured(`
+				- key: foo
+				  value: baz
+				- key: bar
+				  value: notbaz
+			`),
+			warnings: []string{
+				`root[1].value: Invalid value: "string": gotta be baz`,
+			},
+		},
+		{
+			name: "descendent of a map whose parent is totally unchanged",
+			schema: mustSchema(`
+				type: object
+				properties:
+					stringField:
+						type: string
+					mapField:
+						type: object
+						properties:
+							foo:
+								type: string
+								x-kubernetes-validations:
+								- rule: self == "baz"
+								  message: "gotta be baz"
+							mapField:
+								type: object
+								properties:
+									bar:
+										type: string
+										x-kubernetes-validations:
+										- rule: self == "baz"
+										  message: "gotta be nested baz"
+				`),
+			oldObj: mustUnstructured(`
+				stringField: foo
+				mapField:
+					foo: notbaz
+					mapField:
+						bar: notbaz
+			`),
+			newObj: mustUnstructured(`
+				stringField: foo
+				mapField:
+					foo: notbaz
+					mapField:
+						bar: notbaz
+			`),
+			warnings: []string{
+				`root.mapField.foo: Invalid value: "string": gotta be baz`,
+				`root.mapField.mapField.bar: Invalid value: "string": gotta be nested baz`,
+			},
+		},
+		{
+			name: "descendent of a map whose siblings have changed",
+			schema: mustSchema(`
+				type: object
+				properties:
+					stringField:
+						type: string
+					mapField:
+						type: object
+						properties:
+							foo:
+								type: string
+								x-kubernetes-validations:
+								- rule: self == "baz"
+								  message: "gotta be baz"
+							mapField:
+								type: object
+								properties:
+									bar:
+										type: string
+										x-kubernetes-validations:
+										- rule: self == "baz"
+										  message: "gotta be baz"
+									otherBar:
+										type: string
+										x-kubernetes-validations:
+										- rule: self == "otherBaz"
+										  message: "gotta be otherBaz"
+				`),
+			oldObj: mustUnstructured(`
+				stringField: foo
+				mapField:
+					foo: baz
+					mapField:
+						bar: notbaz
+						otherBar: nototherBaz
+			`),
+			newObj: mustUnstructured(`
+				stringField: foo
+				mapField:
+					foo: notbaz
+					mapField:
+						bar: notbaz
+						otherBar: otherBaz
+			`),
+			errors: []string{
+				// Didn't get ratcheted because we changed its value from baz to notbaz
+				`root.mapField.foo: Invalid value: "string": gotta be baz`,
+			},
+			warnings: []string{
+				// Ratcheted because its value remained the same, even though it is invalid
+				`root.mapField.mapField.bar: Invalid value: "string": gotta be baz`,
+			},
+		},
+		{
+			name: "normal CEL expression thats a descendent of an atomic array whose siblings has changed",
+			schema: mustSchema(`
+				type: object
+				properties:
+					stringField:
+						type: string
+					atomicArray:
+						type: array
+						x-kubernetes-list-type: atomic
+						items:
+							type: object
+							properties:
+								bar:
+									type: string
+									x-kubernetes-validations:
+									- rule: self == "baz"
+									  message: "gotta be baz"
+				`),
+			oldObj: mustUnstructured(`
+				stringField: foo
+				atomicArray:
+				- bar: bar
+			`),
+			newObj: mustUnstructured(`
+				stringField: changed but ratcheted
+				atomicArray:
+				- bar: bar
+			`),
+			warnings: []string{
+				`root.atomicArray[0].bar: Invalid value: "string": gotta be baz`,
+			},
+		},
+		{
+			name: "we can't ratchet a normal CEL expression from an uncorrelatable part of the schema whose parent nodes has changed",
+			schema: mustSchema(`
+				type: array
+				x-kubernetes-list-type: atomic
+				items:
+					type: object
+					properties:
+						bar:
+							type: string
+							x-kubernetes-validations:
+							- rule: self == "baz"
+							  message: "gotta be baz"
+			`),
+			// CEL error comes from uncorrelatable portion of the schema,
+			// but it should be ratcheted anyway because it is the descendent
+			// or an unchanged correlatable node
+			oldObj: mustUnstructured(`
+				- bar: bar
+			`),
+			newObj: mustUnstructured(`
+				- bar: bar
+				- bar: baz
+			`),
+			errors: []string{
+				`root[0].bar: Invalid value: "string": gotta be baz`,
+			},
+		},
+		{
+			name: "transition rules never ratchet for correlatable schemas",
+			schema: mustSchema(`
+				type: object
+				properties:
+					foo:
+						type: string
+						x-kubernetes-validations:
+						- rule: oldSelf != "bar" && self == "baz"
+						  message: gotta be baz
+			`),
+			oldObj: mustUnstructured(`
+				foo: bar
+			`),
+			newObj: mustUnstructured(`
+				foo: bar
+			`),
+			errors: []string{
+				`root.foo: Invalid value: "string": gotta be baz`,
+			},
+		},
+		{
+			name: "changing field path does not change ratcheting logic",
+			schema: mustSchema(`
+				type: object
+				x-kubernetes-validations:
+				- rule: self.foo == "baz"
+				  message: gotta be baz
+				  fieldPath: ".foo"
+				properties:
+					bar:
+						type: string
+					foo:
+						type: string
+			`),
+			oldObj: mustUnstructured(`
+				foo: bar
+			`),
+			// Fieldpath is on unchanged field `foo`, but since rule is on the
+			// changed parent object we still get an error
+			newObj: mustUnstructured(`
+				foo: bar
+				bar: invalid
+			`),
+			errors: []string{
+				`root.foo: Invalid value: "object": gotta be baz`,
+			},
+		},
+		{
+			name: "cost budget errors are not ratcheted",
+			schema: mustSchema(`
+				type: string
+				minLength: 5
+				x-kubernetes-validations:
+				- rule: self == "baz"
+				  message: gotta be baz
+			`),
+			oldObj:            "unchanged",
+			newObj:            "unchanged",
+			runtimeCostBudget: 1,
+			errors: []string{
+				`validation failed due to running out of cost budget, no further validation rules will be run`,
+			},
+		},
+		{
+			name: "compile errors are not ratcheted",
+			schema: mustSchema(`
+				type: string
+				x-kubernetes-validations:
+				- rule: asdausidyhASDNJm
+				  message: gotta be baz
+			`),
+			oldObj: "unchanged",
+			newObj: "unchanged",
+			errors: []string{
+				`rule compile error: compilation failed: ERROR: <input>:1:1: undeclared reference to 'asdausidyhASDNJm'`,
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			validator := NewValidator(c.schema, false, celconfig.PerCallLimit)
+			require.NotNil(t, validator)
+			recorder := &warningRecorder{}
+			ctx := warning.WithWarningRecorder(context.TODO(), recorder)
+			budget := c.runtimeCostBudget
+			if budget == 0 {
+				budget = celconfig.RuntimeCELCostBudget
+			}
+			errs, _ := validator.ValidateWithRatcheting(
+				ctx,
+				field.NewPath("root"),
+				c.schema,
+				c.newObj,
+				c.oldObj,
+				budget,
+			)
+
+			require.Len(t, errs, len(c.errors), "must have expected number of errors")
+			require.Len(t, recorder.Warnings(), len(c.warnings), "must have expected number of warnings")
+
+			// Check that the expected errors were raised
+			for _, expectedErr := range c.errors {
+				found := false
+				for _, err := range errs {
+					if strings.Contains(err.Error(), expectedErr) {
+						found = true
+						break
+					}
+				}
+
+				assert.True(t, found, "expected error %q not found", expectedErr)
+			}
+
+			// Check that the ratcheting disabled errors were raised as warnings
+			for _, expectedWarning := range c.warnings {
+				found := false
+				for _, warning := range recorder.Warnings() {
+					if warning == expectedWarning {
+						found = true
+						break
+					}
+				}
+				assert.True(t, found, "expected warning %q not found", expectedWarning)
+			}
+
+		})
+	}
+}
+
 func genString(n int, c rune) string {
 	b := strings.Builder{}
 	for i := 0; i < n; i++ {

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

@@ -26,6 +26,7 @@ import (
 	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"
+	apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -257,6 +258,9 @@ 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)
 			errs = append(errs, err...)

staging/src/k8s.io/apiextensions-apiserver/test/integration/ratcheting_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package integration_test
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -36,6 +37,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/apimachinery/pkg/util/wait"
+	utilyaml "k8s.io/apimachinery/pkg/util/yaml"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/client-go/dynamic"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
@@ -95,11 +97,50 @@ var fakeRESTMapper map[schema.GroupVersionResource]string = map[schema.GroupVers
 	myCRDV1Beta1: "MyCoolCRD",
 }
 
+// FixTabsOrDie counts the number of tab characters preceding the first
+// line in the given yaml object. It removes that many tabs from every
+// line. It panics (it's a test function) if some line has fewer tabs
+// than the first line.
+//
+// The purpose of this is to make it easier to read tests.
+func FixTabsOrDie(in string) string {
+	lines := bytes.Split([]byte(in), []byte{'\n'})
+	if len(lines[0]) == 0 && len(lines) > 1 {
+		lines = lines[1:]
+	}
+	// Create prefix made of tabs that we want to remove.
+	var prefix []byte
+	for _, c := range lines[0] {
+		if c != '\t' {
+			break
+		}
+		prefix = append(prefix, byte('\t'))
+	}
+	// Remove prefix from all tabs, fail otherwise.
+	for i := range lines {
+		line := lines[i]
+		// It's OK for the last line to be blank (trailing \n)
+		if i == len(lines)-1 && len(line) <= len(prefix) && bytes.TrimSpace(line) == nil {
+			lines[i] = []byte{}
+			break
+		}
+		if !bytes.HasPrefix(line, prefix) {
+			panic(fmt.Errorf("line %d doesn't start with expected number (%d) of tabs: %v", i, len(prefix), string(line)))
+		}
+		lines[i] = line[len(prefix):]
+	}
+	joined := string(bytes.Join(lines, []byte{'\n'}))
+
+	// Convert rest of tabs to spaces since yaml doesnt like yabs
+	// (assuming 2 space alignment)
+	return strings.ReplaceAll(joined, "\t", "  ")
+}
+
 type applyPatchOperation struct {
 	description string
 	gvr         schema.GroupVersionResource
 	name        string
-	patch       map[string]interface{}
+	patch       interface{}
 }
 
 func (a applyPatchOperation) Do(ctx *ratchetingTestContext) error {
@@ -109,23 +150,31 @@ func (a applyPatchOperation) Do(ctx *ratchetingTestContext) error {
 		return fmt.Errorf("no mapping found for Gvr %v, add entry to fakeRESTMapper", a.gvr)
 	}
 
-	a.patch["kind"] = kind
+	patch := &unstructured.Unstructured{}
-	a.patch["apiVersion"] = a.gvr.GroupVersion().String()
+	if obj, ok := a.patch.(map[string]interface{}); ok {
-
+		patch.Object = obj
-	if meta, ok := a.patch["metadata"]; ok {
+	} else if str, ok := a.patch.(string); ok {
-		mObj := meta.(map[string]interface{})
+		str = FixTabsOrDie(str)
-		mObj["name"] = a.name
+		if err := utilyaml.NewYAMLOrJSONDecoder(strings.NewReader(str), len(str)).Decode(&patch.Object); err != nil {
-		mObj["namespace"] = "default"
+			return err
+		}
 	} else {
-		a.patch["metadata"] = map[string]interface{}{
+		return fmt.Errorf("invalid patch type: %T", a.patch)
-			"name":      a.name,
-			"namespace": "default",
-		}
 	}
 
-	_, err := ctx.DynamicClient.Resource(a.gvr).Namespace("default").Apply(context.TODO(), a.name, &unstructured.Unstructured{
+	patch.SetKind(kind)
-		Object: a.patch,
+	patch.SetAPIVersion(a.gvr.GroupVersion().String())
-	}, metav1.ApplyOptions{
+	patch.SetName(a.name)
+	patch.SetNamespace("default")
+
+	_, err := ctx.DynamicClient.
+		Resource(a.gvr).
+		Namespace(patch.GetNamespace()).
+		Apply(
+			context.TODO(),
+			patch.GetName(),
+			patch,
+			metav1.ApplyOptions{
 				FieldManager: "manager",
 			})
 
@@ -1295,13 +1344,54 @@ func TestRatchetingFunctionality(t *testing.T) {
 		},
 		{
 			Name: "CEL_transition_rules_should_not_ratchet",
+			Operations: []ratchetingTestOperation{
+				updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{
+					Type:                   "object",
+					XPreserveUnknownFields: ptr(true),
+				}},
+				applyPatchOperation{
+					"create instance with strings that do not start with k8s",
+					myCRDV1Beta1, myCRDInstanceName,
+					`
+						myStringField: myStringValue
+						myOtherField: myOtherField
+					`,
+				},
+				updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{
+					Type:                   "object",
+					XPreserveUnknownFields: ptr(true),
+					Properties: map[string]apiextensionsv1.JSONSchemaProps{
+						"myStringField": {
+							Type: "string",
+							XValidations: apiextensionsv1.ValidationRules{
+								{
+									Rule: "oldSelf != 'myStringValue' || self == 'validstring'",
+								},
+							},
+						},
+					},
+				}},
+				expectError{applyPatchOperation{
+					"try to change one field to valid value, but unchanged field fails to be ratcheted by transition rule",
+					myCRDV1Beta1, myCRDInstanceName,
+					`
+						myOtherField: myNewOtherField
+						myStringField: myStringValue
+					`,
+				}},
+				applyPatchOperation{
+					"change both fields to valid values",
+					myCRDV1Beta1, myCRDInstanceName,
+					`
+						myStringField: validstring
+						myOtherField: myNewOtherField
+					`,
+				},
+			},
 		},
 		// Future Functionality, disabled tests
 		{
 			Name: "CEL Add Change Rule",
-			// Planned future test. CEL Rules are not yet ratcheted in alpha
-			// implementation of CRD Validation Ratcheting
-			Disabled: true,
 			Operations: []ratchetingTestOperation{
 				updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{
 					Type: "object",