Merge pull request #110549 from benluddy/skip-unused-oldself-activation-work

Only provide an oldSelf binding when referenced by a CEL rule.

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

@@ -51,6 +51,10 @@ type Validator struct {
 	// isResourceRoot is true if this validator node is for the root of a resource. Either the root of the
 	// custom resource being validated, or the root of an XEmbeddedResource object.
 	isResourceRoot bool
+
+	// celActivationFactory produces an Activation, which resolves identifiers (e.g. self and
+	// oldSelf) to CEL values.
+	celActivationFactory func(sts *schema.Structural, obj, oldObj interface{}) interpreter.Activation
 }
 
 // NewValidator returns compiles all the CEL programs defined in x-kubernetes-validations extensions
@@ -82,6 +86,14 @@ func validator(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) *
 		additionalPropertiesValidator = validator(s.AdditionalProperties.Structural, s.AdditionalProperties.Structural.XEmbeddedResource, perCallLimit)
 	}
 	if len(compiledRules) > 0 || err != nil || itemsValidator != nil || additionalPropertiesValidator != nil || len(propertiesValidators) > 0 {
+		var activationFactory func(*schema.Structural, interface{}, interface{}) interpreter.Activation = validationActivationWithoutOldSelf
+		for _, rule := range compiledRules {
+			if rule.TransitionRule {
+				activationFactory = validationActivationWithOldSelf
+				break
+			}
+		}
+
 		return &Validator{
 			compiledRules:        compiledRules,
 			compilationErr:       err,
@@ -89,6 +101,7 @@ func validator(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) *
 			Items:                itemsValidator,
 			AdditionalProperties: additionalPropertiesValidator,
 			Properties:           propertiesValidators,
+			celActivationFactory: activationFactory,
 		}
 	}
 
@@ -159,7 +172,7 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
 	if s.isResourceRoot {
 		sts = model.WithTypeAndObjectMeta(sts)
 	}
-	var activation interpreter.Activation = NewValidationActivation(obj, oldObj, sts)
+	activation := s.celActivationFactory(sts, obj, oldObj)
 	for i, compiled := range s.compiledRules {
 		rule := sts.XValidations[i]
 		if compiled.Error != nil {
@@ -231,17 +244,23 @@ type validationActivation struct {
 	hasOldSelf    bool
 }
 
-func NewValidationActivation(obj, oldObj interface{}, structural *schema.Structural) *validationActivation {
+func validationActivationWithOldSelf(sts *schema.Structural, obj, oldObj interface{}) interpreter.Activation {
 	va := &validationActivation{
-		self: UnstructuredToVal(obj, structural),
+		self: UnstructuredToVal(obj, sts),
 	}
 	if oldObj != nil {
-		va.oldSelf = UnstructuredToVal(oldObj, structural) // +k8s:verify-mutation:reason=clone
-		va.hasOldSelf = true                               // +k8s:verify-mutation:reason=clone
+		va.oldSelf = UnstructuredToVal(oldObj, sts) // +k8s:verify-mutation:reason=clone
+		va.hasOldSelf = true                        // +k8s:verify-mutation:reason=clone
 	}
 	return va
 }
 
+func validationActivationWithoutOldSelf(sts *schema.Structural, obj, _ interface{}) interpreter.Activation {
+	return &validationActivation{
+		self: UnstructuredToVal(obj, sts),
+	}
+}
+
 func (a *validationActivation) ResolveName(name string) (interface{}, bool) {
 	switch name {
 	case ScopedVarName:

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

@@ -27,6 +27,7 @@ import (
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/kube-openapi/pkg/validation/strfmt"
 )
 
 // TestValidationExpressions tests CEL integration with custom resource values and OpenAPIv3.
@@ -2017,6 +2018,57 @@ func BenchmarkCELValidationWithCancelledContext(b *testing.B) {
 	}
 }
 
+// BenchmarkCELValidationWithAndWithoutOldSelfReference measures the additional cost of evaluating
+// validation rules that reference "oldSelf".
+func BenchmarkCELValidationWithAndWithoutOldSelfReference(b *testing.B) {
+	for _, rule := range []string{
+		"self.getMonth() >= 0",
+		"oldSelf.getMonth() >= 0",
+	} {
+		b.Run(rule, func(b *testing.B) {
+			obj := map[string]interface{}{
+				"datetime": time.Time{}.Format(strfmt.ISO8601LocalTime),
+			}
+			s := &schema.Structural{
+				Generic: schema.Generic{
+					Type: "object",
+				},
+				Properties: map[string]schema.Structural{
+					"datetime": {
+						Generic: schema.Generic{
+							Type: "string",
+						},
+						ValueValidation: &schema.ValueValidation{
+							Format: "date-time",
+						},
+						Extensions: schema.Extensions{
+							XValidations: []apiextensions.ValidationRule{
+								{Rule: rule},
+							},
+						},
+					},
+				},
+			}
+			validator := NewValidator(s, PerCallLimit)
+			if validator == nil {
+				b.Fatal("expected non nil validator")
+			}
+
+			ctx := context.TODO()
+			root := field.NewPath("root")
+
+			b.ReportAllocs()
+			b.ResetTimer()
+			for i := 0; i < b.N; i++ {
+				errs, _ := validator.Validate(ctx, root, s, obj, obj, RuntimeCELCostBudget)
+				for _, err := range errs {
+					b.Errorf("unexpected error: %v", err)
+				}
+			}
+		})
+	}
+}
+
 func primitiveType(typ, format string) schema.Structural {
 	result := schema.Structural{
 		Generic: schema.Generic{