Adding a deprecating feature gate to fix cost for VAP and webhook mat…

…chConditions.
kubernetes · May 6, 2024 · 81f1682 · 81f1682
1 parent f97ac22
commit 81f1682
Show file tree

Hide file tree

Showing 18 changed files with 1,037 additions and 34 deletions.
diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go
@@ -1250,17 +1250,17 @@ func validateFieldRef(fieldRef string, fldPath *field.Path) field.ErrorList {
 
 // statelessCELCompiler does not support variable composition (and thus is stateless). It should be used when
 // variable composition is not allowed, for example, when validating MatchConditions.
-var statelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
+var statelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSetWithoutStrictCost(environment.DefaultCompatibilityVersion()))
 
 func createCompiler(allowComposition bool) plugincel.Compiler {
 	if !allowComposition {
 		return statelessCELCompiler
 	}
-	compiler, err := plugincel.NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
+	compiler, err := plugincel.NewCompositedCompiler(environment.MustBaseEnvSetWithoutStrictCost(environment.DefaultCompatibilityVersion()))
 	if err != nil {
 		// should never happen, but cannot panic either.
 		utilruntime.HandleError(err)
-		return plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
+		return plugincel.NewCompiler(environment.MustBaseEnvSetWithoutStrictCost(environment.DefaultCompatibilityVersion()))
 	}
 	return compiler
 }

diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go
@@ -3413,7 +3413,7 @@ func TestValidateValidatingAdmissionPolicyUpdate(t *testing.T) {
 		// TODO: CustomAuditAnnotations: string valueExpression with {oldObject} is allowed
 	}
 	// Include the test library, which includes the test() function in the storage environment during test
-	base := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())
+	base := environment.MustBaseEnvSetWithoutStrictCost(environment.DefaultCompatibilityVersion())
 	extended, err := base.Extend(environment.VersionedOptions{
 		IntroducedVersion: version.MustParseGeneric("1.999"),
 		EnvOptions:        []cel.EnvOption{library.Test()},
@@ -3423,7 +3423,7 @@ func TestValidateValidatingAdmissionPolicyUpdate(t *testing.T) {
 	}
 	statelessCELCompiler = plugincel.NewCompiler(extended)
 	defer func() {
-		statelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
+		statelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSetWithoutStrictCost(environment.DefaultCompatibilityVersion()))
 	}()
 
 	for _, test := range tests {

diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go
@@ -1270,6 +1270,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	genericfeatures.StorageVersionHash: {Default: true, PreRelease: featuregate.Beta},
 
+	genericfeatures.StrictCostEnforcement: {Default: false, PreRelease: featuregate.Alpha}, // deprecating in 1.32
+
 	genericfeatures.StructuredAuthenticationConfiguration: {Default: true, PreRelease: featuregate.Beta},
 
 	genericfeatures.StructuredAuthorizationConfiguration: {Default: true, PreRelease: featuregate.Beta},

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
@@ -61,6 +61,7 @@ func TestValidationExpressions(t *testing.T) {
 		costBudget    int64
 		isRoot        bool
 		expectSkipped bool
+		expectedCost  int64
 	}{
 		// tests where val1 and val2 are equal but val3 is different
 		// equality, comparisons and type specific functions
@@ -2006,14 +2007,48 @@ func TestValidationExpressions(t *testing.T) {
 				`quantity(self.val1).isInteger()`,
 			},
 		},
+		{name: "cost for extended lib calculated correctly: isSorted",
+			obj:    objs("20", "200M"),
+			schema: schemas(stringType, stringType),
+			valid: []string{
+				"[1,2,3,4].isSorted()",
+			},
+			expectedCost: 4,
+		},
+		{name: "cost for extended lib calculated correctly: url",
+			obj:    objs("20", "200M"),
+			schema: schemas(stringType, stringType),
+			valid: []string{
+				"url('https:://kubernetes.io/').getHostname() != 'test'",
+			},
+			expectedCost: 4,
+		},
+		{name: "cost for extended lib calculated correctly: split",
+			obj:    objs("20", "200M"),
+			schema: schemas(stringType, stringType),
+			valid: []string{
+				"size('abc 123 def 123'.split(' ')) > 0",
+			},
+			expectedCost: 5,
+		},
+		{name: "cost for extended lib calculated correctly: join",
+			obj:    objs("20", "200M"),
+			schema: schemas(stringType, stringType),
+			valid: []string{
+				"size(['aa', 'bb', 'cc', 'd', 'e', 'f', 'g', 'h', 'i', 'j'].join(' ')) > 0",
+			},
+			expectedCost: 7,
+		},
 	}
 
 	for i := range tests {
 		i := i
 		t.Run(tests[i].name, func(t *testing.T) {
 			t.Parallel()
 			tt := tests[i]
-			tt.costBudget = celconfig.RuntimeCELCostBudget
+			if tt.costBudget == 0 {
+				tt.costBudget = celconfig.RuntimeCELCostBudget
+			}
 			ctx := context.TODO()
 			for j := range tt.valid {
 				validRule := tt.valid[j]
@@ -2032,6 +2067,13 @@ func TestValidationExpressions(t *testing.T) {
 					for _, err := range errs {
 						t.Errorf("unexpected error: %v", err)
 					}
+
+					if tt.expectedCost != 0 {
+						if remainingBudget != tt.costBudget-tt.expectedCost {
+							t.Errorf("expected cost to be %d, but got %d", tt.expectedCost, tt.costBudget-remainingBudget)
+						}
+						return
+					}
 					if tt.expectSkipped {
 						// Skipped validations should have no cost. The only possible false positive here would be the CEL expression 'true'.
 						if remainingBudget != tt.costBudget {

diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go
@@ -26,6 +26,8 @@ import (
 	apiservercel "k8s.io/apiserver/pkg/cel"
 	"k8s.io/apiserver/pkg/cel/environment"
 	"k8s.io/apiserver/pkg/cel/library"
+	"k8s.io/apiserver/pkg/features"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 )
 
 const (
@@ -255,6 +257,12 @@ func mustBuildEnvs(baseEnv *environment.EnvSet) variableDeclEnvs {
 			if err != nil {
 				panic(fmt.Sprintf("environment misconfigured: %v", err))
 			}
+			if utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcement) {
+				extended, err = extended.Extend(environment.StrictCostOpt)
+				if err != nil {
+					panic(fmt.Sprintf("environment misconfigured: %v", err))
+				}
+			}
 			envs[OptionalVariableDeclarations{HasParams: hasParams, HasAuthorizer: hasAuthorizer}] = extended
 		}
 	}

diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile_test.go
@@ -178,7 +178,7 @@ func TestCompileValidatingPolicyExpression(t *testing.T) {
 	}
 
 	// Include the test library, which includes the test() function in the storage environment during test
-	base := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())
+	base := environment.MustBaseEnvSetWithoutStrictCost(environment.DefaultCompatibilityVersion())
 	extended, err := base.Extend(environment.VersionedOptions{
 		IntroducedVersion: version.MajorMinor(1, 999),
 		EnvOptions:        []celgo.EnvOption{library.Test()},
@@ -254,7 +254,7 @@ func TestCompileValidatingPolicyExpression(t *testing.T) {
 }
 
 func BenchmarkCompile(b *testing.B) {
-	compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
+	compiler := NewCompiler(environment.MustBaseEnvSetWithoutStrictCost(environment.DefaultCompatibilityVersion()))
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		options := OptionalVariableDeclarations{HasParams: rand.Int()%2 == 0, HasAuthorizer: rand.Int()%2 == 0}

diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition.go
@@ -32,6 +32,8 @@ import (
 	apiservercel "k8s.io/apiserver/pkg/cel"
 	"k8s.io/apiserver/pkg/cel/environment"
 	"k8s.io/apiserver/pkg/cel/lazy"
+	"k8s.io/apiserver/pkg/features"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 )
 
 const VariablesTypeName = "kubernetes.variables"
@@ -119,6 +121,12 @@ func NewCompositionEnv(typeName string, baseEnvSet *environment.EnvSet) (*Compos
 	if err != nil {
 		return nil, err
 	}
+	if utilfeature.DefaultFeatureGate.Enabled(features.StrictCostEnforcement) {
+		envSet, err = envSet.Extend(environment.StrictCostOpt)
+		if err != nil {
+			return nil, err
+		}
+	}
 	return &CompositionEnv{
 		MapType:           declType,
 		EnvSet:            envSet,

diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go
@@ -188,7 +188,7 @@ func TestCompositedPolicies(t *testing.T) {
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			compiler, err := NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
+			compiler, err := NewCompositedCompiler(environment.MustBaseEnvSetWithoutStrictCost(environment.DefaultCompatibilityVersion()))
 			if err != nil {
 				t.Fatal(err)
 			}