Adding the feature gates to fix cost for VAP and webhook matchConditi…

…ons. Kubernetes-commit: 3d896724760a957e8059ff80e9f399248eacac66
kubernetes · May 1, 2024 · e025ab4 · e025ab4
1 parent a30c80f
commit e025ab4
Show file tree

Hide file tree

Showing 22 changed files with 495 additions and 96 deletions.
diff --git a/pkg/admission/plugin/cel/compile.go b/pkg/admission/plugin/cel/compile.go
@@ -222,40 +222,48 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor, op
 func mustBuildEnvs(baseEnv *environment.EnvSet) variableDeclEnvs {
 	requestType := BuildRequestType()
 	namespaceType := BuildNamespaceType()
-	envs := make(variableDeclEnvs, 4) // since the number of variable combinations is small, pre-build a environment for each
+	envs := make(variableDeclEnvs, 8) // since the number of variable combinations is small, pre-build a environment for each
 	for _, hasParams := range []bool{false, true} {
 		for _, hasAuthorizer := range []bool{false, true} {
-			var envOpts []cel.EnvOption
-			if hasParams {
-				envOpts = append(envOpts, cel.Variable(ParamsVarName, cel.DynType))
-			}
-			if hasAuthorizer {
+			for _, strictCost := range []bool{false, true} {
+				var envOpts []cel.EnvOption
+				if hasParams {
+					envOpts = append(envOpts, cel.Variable(ParamsVarName, cel.DynType))
+				}
+				if hasAuthorizer {
+					envOpts = append(envOpts,
+						cel.Variable(AuthorizerVarName, library.AuthorizerType),
+						cel.Variable(RequestResourceAuthorizerVarName, library.ResourceCheckType))
+				}
 				envOpts = append(envOpts,
-					cel.Variable(AuthorizerVarName, library.AuthorizerType),
-					cel.Variable(RequestResourceAuthorizerVarName, library.ResourceCheckType))
-			}
-			envOpts = append(envOpts,
-				cel.Variable(ObjectVarName, cel.DynType),
-				cel.Variable(OldObjectVarName, cel.DynType),
-				cel.Variable(NamespaceVarName, namespaceType.CelType()),
-				cel.Variable(RequestVarName, requestType.CelType()))
+					cel.Variable(ObjectVarName, cel.DynType),
+					cel.Variable(OldObjectVarName, cel.DynType),
+					cel.Variable(NamespaceVarName, namespaceType.CelType()),
+					cel.Variable(RequestVarName, requestType.CelType()))
 
-			extended, err := baseEnv.Extend(
-				environment.VersionedOptions{
-					// Feature epoch was actually 1.26, but we artificially set it to 1.0 because these
-					// options should always be present.
-					IntroducedVersion: version.MajorMinor(1, 0),
-					EnvOptions:        envOpts,
-					DeclTypes: []*apiservercel.DeclType{
-						namespaceType,
-						requestType,
+				extended, err := baseEnv.Extend(
+					environment.VersionedOptions{
+						// Feature epoch was actually 1.26, but we artificially set it to 1.0 because these
+						// options should always be present.
+						IntroducedVersion: version.MajorMinor(1, 0),
+						EnvOptions:        envOpts,
+						DeclTypes: []*apiservercel.DeclType{
+							namespaceType,
+							requestType,
+						},
 					},
-				},
-			)
-			if err != nil {
-				panic(fmt.Sprintf("environment misconfigured: %v", err))
+				)
+				if err != nil {
+					panic(fmt.Sprintf("environment misconfigured: %v", err))
+				}
+				if strictCost {
+					extended, err = extended.Extend(environment.StrictCostOpt)
+					if err != nil {
+						panic(fmt.Sprintf("environment misconfigured: %v", err))
+					}
+				}
+				envs[OptionalVariableDeclarations{HasParams: hasParams, HasAuthorizer: hasAuthorizer, StrictCost: strictCost}] = extended
 			}
-			envs[OptionalVariableDeclarations{HasParams: hasParams, HasAuthorizer: hasAuthorizer}] = extended
 		}
 	}
 	return envs

diff --git a/pkg/admission/plugin/cel/compile_test.go b/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.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)
 	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.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		options := OptionalVariableDeclarations{HasParams: rand.Int()%2 == 0, HasAuthorizer: rand.Int()%2 == 0}

diff --git a/pkg/admission/plugin/cel/composition_test.go b/pkg/admission/plugin/cel/composition_test.go
@@ -48,14 +48,15 @@ func (t *testVariable) GetName() string {
 
 func TestCompositedPolicies(t *testing.T) {
 	cases := []struct {
-		name                 string
-		variables            []NamedExpressionAccessor
-		expression           string
-		attributes           admission.Attributes
-		expectedResult       any
-		expectErr            bool
-		expectedErrorMessage string
-		runtimeCostBudget    int64
+		name                  string
+		variables             []NamedExpressionAccessor
+		expression            string
+		attributes            admission.Attributes
+		expectedResult        any
+		expectErr             bool
+		expectedErrorMessage  string
+		runtimeCostBudget     int64
+		strictCostEnforcement bool
 	}{
 		{
 			name: "simple",
@@ -185,16 +186,45 @@ func TestCompositedPolicies(t *testing.T) {
 			expectErr:            true,
 			expectedErrorMessage: "found no matching overload for '_==_' applied to '(string, int)'",
 		},
+		{
+			name: "with strictCostEnforcement on: exceeds cost budget",
+			variables: []NamedExpressionAccessor{
+				&testVariable{
+					name:       "dict",
+					expression: "'abc 123 def 123'.split(' ')",
+				},
+			},
+			attributes:            endpointCreateAttributes(),
+			expression:            "size(variables.dict) > 0",
+			expectErr:             true,
+			expectedErrorMessage:  "validation failed due to running out of cost budget, no further validation rules will be run",
+			runtimeCostBudget:     5,
+			strictCostEnforcement: true,
+		},
+		{
+			name: "with strictCostEnforcement off: not exceed cost budget",
+			variables: []NamedExpressionAccessor{
+				&testVariable{
+					name:       "dict",
+					expression: "'abc 123 def 123'.split(' ')",
+				},
+			},
+			attributes:            endpointCreateAttributes(),
+			expression:            "size(variables.dict) > 0",
+			expectedResult:        true,
+			runtimeCostBudget:     5,
+			strictCostEnforcement: false,
+		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			compiler, err := NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
+			compiler, err := NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), tc.strictCostEnforcement))
 			if err != nil {
 				t.Fatal(err)
 			}
-			compiler.CompileAndStoreVariables(tc.variables, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.NewExpressions)
+			compiler.CompileAndStoreVariables(tc.variables, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false, StrictCost: tc.strictCostEnforcement}, environment.NewExpressions)
 			validations := []ExpressionAccessor{&condition{Expression: tc.expression}}
-			f := compiler.Compile(validations, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.NewExpressions)
+			f := compiler.Compile(validations, OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false, StrictCost: tc.strictCostEnforcement}, environment.NewExpressions)
 			versionedAttr, err := admission.NewVersionedAttributes(tc.attributes, tc.attributes.GetKind(), newObjectInterfacesForTest())
 			if err != nil {
 				t.Fatal(err)