diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go index e38e364539b..5e80e2dc69d 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go @@ -736,6 +736,7 @@ func compileMatchConditions(matchConditions []api.WebhookMatchCondition, fldPath compiler := authorizationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) seenExpressions := sets.NewString() var compilationResults []authorizationcel.CompilationResult + var usesFieldSelector, usesLabelSelector bool for i, condition := range matchConditions { fldPath := fldPath.Child("matchConditions").Index(i).Child("expression") @@ -754,12 +755,16 @@ func compileMatchConditions(matchConditions []api.WebhookMatchCondition, fldPath continue } compilationResults = append(compilationResults, compilationResult) + usesFieldSelector = usesFieldSelector || compilationResult.UsesFieldSelector + usesLabelSelector = usesLabelSelector || compilationResult.UsesLabelSelector } if len(compilationResults) == 0 { return nil, allErrs } return &authorizationcel.CELMatcher{ CompilationResults: compilationResults, + UsesFieldSelector: usesFieldSelector, + UsesLabelSelector: usesLabelSelector, }, allErrs } diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go index 0d9293dd704..829ff91d395 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go @@ -20,22 +20,34 @@ import ( "fmt" "github.com/google/cel-go/cel" + celast "github.com/google/cel-go/common/ast" + "github.com/google/cel-go/common/operators" "github.com/google/cel-go/common/types/ref" authorizationv1 "k8s.io/api/authorization/v1" "k8s.io/apimachinery/pkg/util/version" apiservercel "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/cel/environment" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" ) const ( subjectAccessReviewRequestVarName = "request" + + fieldSelectorVarName = "fieldSelector" + labelSelectorVarName = "labelSelector" ) // CompilationResult represents a compiled authorization cel expression. type CompilationResult struct { Program cel.Program ExpressionAccessor ExpressionAccessor + + // These track if a given expression uses fieldSelector and labelSelector, + // so construction of data passed to the CEL expression can be optimized if those fields are unused. + UsesFieldSelector bool + UsesLabelSelector bool } // EvaluationResult contains the minimal required fields and metadata of a cel evaluation @@ -96,11 +108,50 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor) (C return resultError(reason, apiservercel.ErrorTypeInvalid) } - _, err = cel.AstToCheckedExpr(ast) + checkedExpr, err := cel.AstToCheckedExpr(ast) if err != nil { // should be impossible since env.Compile returned no issues return resultError("unexpected compilation error: "+err.Error(), apiservercel.ErrorTypeInternal) } + celAST, err := celast.ToAST(checkedExpr) + if err != nil { + // should be impossible since env.Compile returned no issues + return resultError("unexpected compilation error: "+err.Error(), apiservercel.ErrorTypeInternal) + } + + var usesFieldSelector, usesLabelSelector bool + celast.PreOrderVisit(celast.NavigateAST(celAST), celast.NewExprVisitor(func(e celast.Expr) { + // we already know we use both, no need to inspect more + if usesFieldSelector && usesLabelSelector { + return + } + + var fieldName string + switch e.Kind() { + case celast.SelectKind: + // simple select (.fieldSelector / .labelSelector) + fieldName = e.AsSelect().FieldName() + case celast.CallKind: + // optional select (.?fieldSelector / .?labelSelector) + if e.AsCall().FunctionName() != operators.OptSelect { + return + } + args := e.AsCall().Args() + // args[0] is the receiver (what comes before the `.?`), args[1] is the field name being optionally selected (what comes after the `.?`) + if len(args) != 2 || args[1].Kind() != celast.LiteralKind || args[1].AsLiteral().Type() != cel.StringType { + return + } + fieldName, _ = args[1].AsLiteral().Value().(string) + } + + switch fieldName { + case fieldSelectorVarName: + usesFieldSelector = true + case labelSelectorVarName: + usesLabelSelector = true + } + })) + prog, err := env.Program(ast) if err != nil { return resultError("program instantiation failed: "+err.Error(), apiservercel.ErrorTypeInternal) @@ -108,6 +159,8 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor) (C return CompilationResult{ Program: prog, ExpressionAccessor: expressionAccessor, + UsesFieldSelector: usesFieldSelector, + UsesLabelSelector: usesLabelSelector, }, nil } @@ -161,7 +214,7 @@ func buildRequestType(field func(name string, declType *apiservercel.DeclType, r // buildResourceAttributesType generates a DeclType for ResourceAttributes. // if attributes are added here, also add to convertObjectToUnstructured. func buildResourceAttributesType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { - return apiservercel.NewObjectType("kubernetes.ResourceAttributes", fields( + resourceAttributesFields := []*apiservercel.DeclField{ field("namespace", apiservercel.StringType, false), field("verb", apiservercel.StringType, false), field("group", apiservercel.StringType, false), @@ -169,6 +222,33 @@ func buildResourceAttributesType(field func(name string, declType *apiservercel. field("resource", apiservercel.StringType, false), field("subresource", apiservercel.StringType, false), field("name", apiservercel.StringType, false), + } + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + resourceAttributesFields = append(resourceAttributesFields, field("fieldSelector", buildFieldSelectorType(field, fields), false)) + resourceAttributesFields = append(resourceAttributesFields, field("labelSelector", buildLabelSelectorType(field, fields), false)) + } + return apiservercel.NewObjectType("kubernetes.ResourceAttributes", fields(resourceAttributesFields...)) +} + +func buildFieldSelectorType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { + return apiservercel.NewObjectType("kubernetes.FieldSelectorAttributes", fields( + field("rawSelector", apiservercel.StringType, false), + field("requirements", apiservercel.NewListType(buildSelectorRequirementType(field, fields), -1), false), + )) +} + +func buildLabelSelectorType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { + return apiservercel.NewObjectType("kubernetes.LabelSelectorAttributes", fields( + field("rawSelector", apiservercel.StringType, false), + field("requirements", apiservercel.NewListType(buildSelectorRequirementType(field, fields), -1), false), + )) +} + +func buildSelectorRequirementType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { + return apiservercel.NewObjectType("kubernetes.SelectorRequirement", fields( + field("key", apiservercel.StringType, false), + field("operator", apiservercel.StringType, false), + field("values", apiservercel.NewListType(apiservercel.StringType, -1), false), )) } @@ -181,7 +261,7 @@ func buildNonResourceAttributesType(field func(name string, declType *apiserverc )) } -func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) map[string]interface{} { +func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec, includeFieldSelector, includeLabelSelector bool) map[string]interface{} { // Construct version containing every SubjectAccessReview user and string attribute field, even omitempty ones, for evaluation by CEL extra := obj.Extra if extra == nil { @@ -194,7 +274,7 @@ func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) m "extra": extra, } if obj.ResourceAttributes != nil { - ret["resourceAttributes"] = map[string]string{ + resourceAttributes := map[string]interface{}{ "namespace": obj.ResourceAttributes.Namespace, "verb": obj.ResourceAttributes.Verb, "group": obj.ResourceAttributes.Group, @@ -203,6 +283,42 @@ func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) m "subresource": obj.ResourceAttributes.Subresource, "name": obj.ResourceAttributes.Name, } + + if includeFieldSelector && obj.ResourceAttributes.FieldSelector != nil { + if len(obj.ResourceAttributes.FieldSelector.Requirements) > 0 { + requirements := make([]map[string]interface{}, 0, len(obj.ResourceAttributes.FieldSelector.Requirements)) + for _, r := range obj.ResourceAttributes.FieldSelector.Requirements { + requirements = append(requirements, map[string]interface{}{ + "key": r.Key, + "operator": r.Operator, + "values": r.Values, + }) + } + resourceAttributes[fieldSelectorVarName] = map[string]interface{}{"requirements": requirements} + } + if len(obj.ResourceAttributes.FieldSelector.RawSelector) > 0 { + resourceAttributes[fieldSelectorVarName] = map[string]interface{}{"rawSelector": obj.ResourceAttributes.FieldSelector.RawSelector} + } + } + + if includeLabelSelector && obj.ResourceAttributes.LabelSelector != nil { + if len(obj.ResourceAttributes.LabelSelector.Requirements) > 0 { + requirements := make([]map[string]interface{}, 0, len(obj.ResourceAttributes.LabelSelector.Requirements)) + for _, r := range obj.ResourceAttributes.LabelSelector.Requirements { + requirements = append(requirements, map[string]interface{}{ + "key": r.Key, + "operator": r.Operator, + "values": r.Values, + }) + } + resourceAttributes[labelSelectorVarName] = map[string]interface{}{"requirements": requirements} + } + if len(obj.ResourceAttributes.LabelSelector.RawSelector) > 0 { + resourceAttributes[labelSelectorVarName] = map[string]interface{}{"rawSelector": obj.ResourceAttributes.LabelSelector.RawSelector} + } + } + + ret["resourceAttributes"] = resourceAttributes } if obj.NonResourceAttributes != nil { ret["nonResourceAttributes"] = map[string]string{ diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go index aecb3146384..0ed3a9d1077 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go @@ -23,8 +23,12 @@ import ( "testing" v1 "k8s.io/api/authorization/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apiservercel "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/cel/environment" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" ) func TestCompileCELExpression(t *testing.T) { @@ -32,6 +36,8 @@ func TestCompileCELExpression(t *testing.T) { name string expression string expectedError string + + authorizeWithSelectorsEnabled bool }{ { name: "SubjectAccessReviewSpec user comparison", @@ -57,12 +63,44 @@ func TestCompileCELExpression(t *testing.T) { expression: "x.user", expectedError: "undeclared reference", }, + { + name: "fieldSelector not enabled", + expression: "request.resourceAttributes.fieldSelector.rawSelector == 'foo'", + authorizeWithSelectorsEnabled: false, + expectedError: `undefined field 'fieldSelector'`, + }, + { + name: "fieldSelector rawSelector", + expression: "request.resourceAttributes.fieldSelector.rawSelector == 'foo'", + authorizeWithSelectorsEnabled: true, + }, + { + name: "fieldSelector requirement", + expression: "request.resourceAttributes.fieldSelector.requirements.exists(r, r.key == 'foo' && r.operator == 'In' && ('bar' in r.values))", + authorizeWithSelectorsEnabled: true, + }, + { + name: "labelSelector not enabled", + expression: "request.resourceAttributes.labelSelector.rawSelector == 'foo'", + authorizeWithSelectorsEnabled: false, + expectedError: `undefined field 'labelSelector'`, + }, + { + name: "labelSelector rawSelector", + expression: "request.resourceAttributes.labelSelector.rawSelector == 'foo'", + authorizeWithSelectorsEnabled: true, + }, + { + name: "labelSelector requirement", + expression: "request.resourceAttributes.labelSelector.requirements.exists(r, r.key == 'foo' && r.operator == 'In' && ('bar' in r.values))", + authorizeWithSelectorsEnabled: true, + }, } - compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) - for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, tc.authorizeWithSelectorsEnabled) + compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) _, err := compiler.CompileCELExpression(&SubjectAccessReviewMatchCondition{ Expression: tc.expression, }) @@ -77,6 +115,7 @@ func TestCompileCELExpression(t *testing.T) { } func TestBuildRequestType(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true) f := func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField { return apiservercel.NewDeclField(name, declType, required, nil, nil) } @@ -122,7 +161,10 @@ func compareFieldsForType(t *testing.T, nativeType reflect.Type, declType *apise t.Fatalf("expected field %q to be present", nativeField.Name) } declFieldType := nativeTypeToCELType(t, nativeField.Type, field, fields) - if declFieldType != nil && declFieldType.CelType().Equal(declField.Type.CelType()).Value() != true { + if declFieldType == nil { + return fmt.Errorf("no field type found for %s %v", nativeField.Name, nativeField.Type) + } + if declFieldType.CelType().Equal(declField.Type.CelType()).Value() != true { return fmt.Errorf("expected native field %q to have type %v, got %v", nativeField.Name, nativeField.Type, declField.Type) } } @@ -131,7 +173,7 @@ func compareFieldsForType(t *testing.T, nativeType reflect.Type, declType *apise func nativeTypeToCELType(t *testing.T, nativeType reflect.Type, field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { switch nativeType { - case reflect.TypeOf(""): + case reflect.TypeOf(""), reflect.TypeOf(metav1.LabelSelectorOperator("")), reflect.TypeOf(metav1.FieldSelectorOperator("")): return apiservercel.StringType case reflect.TypeOf([]string{}): return apiservercel.NewListType(apiservercel.StringType, -1) @@ -151,6 +193,34 @@ func nativeTypeToCELType(t *testing.T, nativeType reflect.Type, field func(name return nil } return nonResourceAttributesDeclType + case reflect.TypeOf(&v1.FieldSelectorAttributes{}): + selectorAttributesDeclType := buildFieldSelectorType(field, fields) + if err := compareFieldsForType(t, reflect.TypeOf(v1.FieldSelectorAttributes{}), selectorAttributesDeclType, field, fields); err != nil { + t.Error(err) + return nil + } + return selectorAttributesDeclType + case reflect.TypeOf(&v1.LabelSelectorAttributes{}): + selectorAttributesDeclType := buildLabelSelectorType(field, fields) + if err := compareFieldsForType(t, reflect.TypeOf(v1.LabelSelectorAttributes{}), selectorAttributesDeclType, field, fields); err != nil { + t.Error(err) + return nil + } + return selectorAttributesDeclType + case reflect.TypeOf([]metav1.FieldSelectorRequirement{}): + requirementType := buildSelectorRequirementType(field, fields) + if err := compareFieldsForType(t, reflect.TypeOf(metav1.FieldSelectorRequirement{}), requirementType, field, fields); err != nil { + t.Error(err) + return nil + } + return apiservercel.NewListType(requirementType, -1) + case reflect.TypeOf([]metav1.LabelSelectorRequirement{}): + requirementType := buildSelectorRequirementType(field, fields) + if err := compareFieldsForType(t, reflect.TypeOf(metav1.LabelSelectorRequirement{}), requirementType, field, fields); err != nil { + t.Error(err) + return nil + } + return apiservercel.NewListType(requirementType, -1) default: t.Fatalf("unsupported type %v", nativeType) } diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go index 3202173a8dc..eac97100c13 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go @@ -30,6 +30,11 @@ import ( type CELMatcher struct { CompilationResults []CompilationResult + // These track if any expressions use fieldSelector and labelSelector, + // so construction of data passed to the CEL expression can be optimized if those fields are unused. + UsesLabelSelector bool + UsesFieldSelector bool + // These are optional fields which can be populated if metrics reporting is desired Metrics MatcherMetrics AuthorizerType string @@ -53,7 +58,7 @@ func (c *CELMatcher) Eval(ctx context.Context, r *authorizationv1.SubjectAccessR }() va := map[string]interface{}{ - "request": convertObjectToUnstructured(&r.Spec), + "request": convertObjectToUnstructured(&r.Spec, c.UsesFieldSelector, c.UsesLabelSelector), } for _, compilationResult := range c.CompilationResults { evalResult, _, err := compilationResult.Program.ContextEval(ctx, va) diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go index 9790e0490e0..f04a8b267cc 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go @@ -40,6 +40,9 @@ import ( authorizationv1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/apis/apiserver" "k8s.io/apiserver/pkg/authentication/user" @@ -700,6 +703,8 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { } defer s.Close() + labelRequirement, _ := labels.NewRequirement("baz", selection.Equals, []string{"qux"}) + type webhookMatchConditionsTestCase struct { name string attr authorizer.AttributesRecord @@ -709,6 +714,7 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { expectedDecision authorizer.Decision expressions []apiserver.WebhookMatchCondition featureEnabled bool + selectorEnabled bool } aliceAttr := authorizer.AttributesRecord{ User: &user.DefaultInfo{ @@ -721,6 +727,19 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { Namespace: "kittensandponies", Verb: "get", } + aliceWithSelectorsAttr := authorizer.AttributesRecord{ + User: &user.DefaultInfo{ + Name: "alice", + UID: "1", + Groups: []string{"group1", "group2"}, + Extra: map[string][]string{"key1": {"a", "b", "c"}}, + }, + ResourceRequest: true, + Namespace: "kittensandponies", + Verb: "get", + FieldSelectorRequirements: fields.Requirements{fields.Requirement{Field: "foo", Operator: selection.Equals, Value: "bar"}}, + LabelSelectorRequirements: labels.Requirements{*labelRequirement}, + } tests := []webhookMatchConditionsTestCase{ { name: "no match condition does not require feature enablement", @@ -746,7 +765,7 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { }, { name: "feature enabled, match all against all expressions", - attr: aliceAttr, + attr: aliceWithSelectorsAttr, allow: true, expectedCompileErr: false, expectedDecision: authorizer.DecisionAllow, @@ -763,14 +782,28 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { { Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", }, + { + Expression: "request.?resourceAttributes.fieldSelector.requirements.orValue([]).exists(r, r.key=='foo' && r.operator=='In' && ('bar' in r.values))", + }, + { + Expression: "request.?resourceAttributes.labelSelector.requirements.orValue([]).exists(r, r.key=='baz' && r.operator=='In' && ('qux' in r.values))", + }, + { + Expression: "request.resourceAttributes.?labelSelector.requirements.orValue([]).exists(r, r.key=='baz' && r.operator=='In' && ('qux' in r.values))", + }, + { + Expression: "request.resourceAttributes.labelSelector.?requirements.orValue([]).exists(r, r.key=='baz' && r.operator=='In' && ('qux' in r.values))", + }, }, - featureEnabled: true, + featureEnabled: true, + selectorEnabled: true, }, } for i, test := range tests { t.Run(test.name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthorizationConfiguration, test.featureEnabled) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AuthorizeWithSelectors, test.selectorEnabled) wh, err := newV1Authorizer(s.URL, clientCert, clientKey, caCert, 0, noopAuthorizerMetrics(), test.expressions, "") if test.expectedCompileErr && err == nil { t.Fatalf("%d: Expected compile error", i)