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 index 25ee108ea95..b7b589d273a 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go +++ b/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(), } } 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 index 38b80a304aa..646c640fcae 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/composition.go +++ b/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) { - c.MapType.Fields[name] = apiservercel.NewDeclField(name, apiservercel.DynType, true, nil, nil) +func (c *CompositionEnv) AddField(name string, celType *cel.Type) { + 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 + } +} 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 index ebbb51bbbb7..aedc7c969f0 100644 --- 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 @@ -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", - expression: "object.spec.subsets[114514].addresses.size()", // array index out of bound + name: "count", + 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) { diff --git a/test/e2e/apimachinery/validatingadmissionpolicy.go b/test/e2e/apimachinery/validatingadmissionpolicy.go index 4f06cac7986..c2f2547d7b8 100644 --- a/test/e2e/apimachinery/validatingadmissionpolicy.go +++ b/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 - Expression: "variables.replicas % 2", + Name: "oddReplicas", + 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{})