Merge pull request #121001 from jiahuif-forks/feature/validating-admission-policy/typed-composition-variables

ValidatingAdmissionPolicy: typed variables support.

staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go

@@ -141,6 +141,7 @@ type CompilationResult struct {
 	Program            cel.Program
 	Error              *apiservercel.Error
 	ExpressionAccessor ExpressionAccessor
+	OutputType         *cel.Type
 }
 
 // Compiler provides a CEL expression compiler configured with the desired admission related CEL variables and
@@ -214,6 +215,7 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor, op
 	return CompilationResult{
 		Program:            prog,
 		ExpressionAccessor: expressionAccessor,
+		OutputType:         ast.OutputType(),
 	}
 }
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition.go

@@ -23,6 +23,7 @@ import (
 	"github.com/google/cel-go/cel"
 	"github.com/google/cel-go/common/types"
 	"github.com/google/cel-go/common/types/ref"
+	"github.com/google/cel-go/common/types/traits"
 
 	v1 "k8s.io/api/admission/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -69,8 +70,8 @@ func (c *CompositedCompiler) CompileAndStoreVariables(variables []NamedExpressio
 }
 
 func (c *CompositedCompiler) CompileAndStoreVariable(variable NamedExpressionAccessor, options OptionalVariableDeclarations, mode environment.Type) CompilationResult {
-	c.CompositionEnv.AddField(variable.GetName())
 	result := c.Compiler.CompileCELExpression(variable, options, mode)
+	c.CompositionEnv.AddField(variable.GetName(), result.OutputType)
 	c.CompositionEnv.CompiledVariables[variable.GetName()] = result
 	return result
 }
@@ -90,8 +91,8 @@ type CompositionEnv struct {
 	CompiledVariables map[string]CompilationResult
 }
 
-func (c *CompositionEnv) AddField(name string) {
+func (c *CompositionEnv) AddField(name string, celType *cel.Type) {
-	c.MapType.Fields[name] = apiservercel.NewDeclField(name, apiservercel.DynType, true, nil, nil)
+	c.MapType.Fields[name] = apiservercel.NewDeclField(name, convertCelTypeToDeclType(celType), true, nil, nil)
 }
 
 func NewCompositionEnv(typeName string, baseEnvSet *environment.EnvSet) (*CompositionEnv, error) {
@@ -196,3 +197,48 @@ func (a *variableAccessor) Callback(_ *lazy.MapValue) ref.Val {
 	}
 	return v
 }
+
+// convertCelTypeToDeclType converts a cel.Type to DeclType, for the use of
+// the TypeProvider and the cost estimator.
+// List and map types are created on-demand with their parameters converted recursively.
+func convertCelTypeToDeclType(celType *cel.Type) *apiservercel.DeclType {
+	if celType == nil {
+		return apiservercel.DynType
+	}
+	switch celType {
+	case cel.AnyType:
+		return apiservercel.AnyType
+	case cel.BoolType:
+		return apiservercel.BoolType
+	case cel.BytesType:
+		return apiservercel.BytesType
+	case cel.DoubleType:
+		return apiservercel.DoubleType
+	case cel.DurationType:
+		return apiservercel.DurationType
+	case cel.IntType:
+		return apiservercel.IntType
+	case cel.NullType:
+		return apiservercel.NullType
+	case cel.StringType:
+		return apiservercel.StringType
+	case cel.TimestampType:
+		return apiservercel.TimestampType
+	case cel.UintType:
+		return apiservercel.UintType
+	default:
+		if celType.HasTrait(traits.ContainerType) && celType.HasTrait(traits.IndexerType) {
+			parameters := celType.Parameters()
+			switch len(parameters) {
+			case 1:
+				elemType := convertCelTypeToDeclType(parameters[0])
+				return apiservercel.NewListType(elemType, -1)
+			case 2:
+				keyType := convertCelTypeToDeclType(parameters[0])
+				valueType := convertCelTypeToDeclType(parameters[1])
+				return apiservercel.NewMapType(keyType, valueType, -1)
+			}
+		}
+		return apiservercel.DynType
+	}
+}

staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition_test.go

@@ -70,7 +70,7 @@ func TestCompositedPolicies(t *testing.T) {
 			expectedResult: true,
 		},
 		{
-			name: "delayed compile error",
+			name: "early compile error",
 			variables: []NamedExpressionAccessor{
 				&testVariable{
 					name:       "name",
@@ -80,20 +80,20 @@ func TestCompositedPolicies(t *testing.T) {
 			attributes:           endpointCreateAttributes(),
 			expression:           "variables.name == 'endpoints1'",
 			expectErr:            true,
-			expectedErrorMessage: `composited variable "name" fails to compile:`,
+			expectedErrorMessage: `found no matching overload for '_==_' applied to '(int, string)'`,
 		},
 		{
 			name: "delayed eval error",
 			variables: []NamedExpressionAccessor{
 				&testVariable{
-					name:       "name",
+					name:       "count",
-					expression: "object.spec.subsets[114514].addresses.size()", // array index out of bound
+					expression: "object.subsets[114514].addresses.size()", // array index out of bound
 				},
 			},
 			attributes:           endpointCreateAttributes(),
-			expression:           "variables.name == 'endpoints1'",
+			expression:           "variables.count == 810",
 			expectErr:            true,
-			expectedErrorMessage: `composited variable "name" fails to evaluate:`,
+			expectedErrorMessage: `composited variable "count" fails to evaluate: index out of bounds: 114514`,
 		},
 		{
 			name: "out of budget during lazy evaluation",
@@ -123,6 +123,68 @@ func TestCompositedPolicies(t *testing.T) {
 			expectedResult:    true,
 			runtimeCostBudget: 10, // enough for one lazy evaluation but not two, should pass
 		},
+		{
+			name: "single boolean variable in expression",
+			variables: []NamedExpressionAccessor{
+				&testVariable{
+					name:       "fortuneTelling",
+					expression: "true",
+				},
+			},
+			attributes:     endpointCreateAttributes(),
+			expression:     "variables.fortuneTelling",
+			expectedResult: true,
+		},
+		{
+			name: "variable of a list",
+			variables: []NamedExpressionAccessor{
+				&testVariable{
+					name:       "list",
+					expression: "[1, 2, 3, 4]",
+				},
+			},
+			attributes:     endpointCreateAttributes(),
+			expression:     "variables.list.sum() == 10",
+			expectedResult: true,
+		},
+		{
+			name: "variable of a map",
+			variables: []NamedExpressionAccessor{
+				&testVariable{
+					name:       "dict",
+					expression: `{"foo": "bar"}`,
+				},
+			},
+			attributes:     endpointCreateAttributes(),
+			expression:     "variables.dict['foo'].contains('bar')",
+			expectedResult: true,
+		},
+		{
+			name: "variable of a list but confused as a map",
+			variables: []NamedExpressionAccessor{
+				&testVariable{
+					name:       "list",
+					expression: "[1, 2, 3, 4]",
+				},
+			},
+			attributes:           endpointCreateAttributes(),
+			expression:           "variables.list['invalid'] == 'invalid'",
+			expectErr:            true,
+			expectedErrorMessage: "found no matching overload for '_[_]' applied to '(list(int), string)'",
+		},
+		{
+			name: "list of strings, but element is confused as an integer",
+			variables: []NamedExpressionAccessor{
+				&testVariable{
+					name:       "list",
+					expression: "['1', '2', '3', '4']",
+				},
+			},
+			attributes:           endpointCreateAttributes(),
+			expression:           "variables.list[0] == 1",
+			expectErr:            true,
+			expectedErrorMessage: "found no matching overload for '_==_' applied to '(string, int)'",
+		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {

test/e2e/apimachinery/validatingadmissionpolicy.go

@@ -205,11 +205,14 @@ var _ = SIGDescribe("ValidatingAdmissionPolicy [Privileged:ClusterAdmin][Alpha][
 					Expression: "object.spec.replicas",
 				}).
 				WithVariable(admissionregistrationv1beta1.Variable{
-					Name:       "replicasReminder", // a bit artificial but good for testing purpose
+					Name:       "oddReplicas",
-					Expression: "variables.replicas % 2",
+					Expression: "variables.replicas % 2 == 1",
 				}).
 				WithValidation(admissionregistrationv1beta1.Validation{
-					Expression: "variables.replicas > 1 && variables.replicasReminder == 1",
+					Expression: "variables.replicas > 1",
+				}).
+				WithValidation(admissionregistrationv1beta1.Validation{
+					Expression: "variables.oddReplicas",
 				}).
 				Build()
 			policy, err := client.AdmissionregistrationV1beta1().ValidatingAdmissionPolicies().Create(ctx, policy, metav1.CreateOptions{})