Add structured labelSelector / fieldSelector to authorization webhook match conditions
This commit is contained in:
		| @@ -736,6 +736,7 @@ func compileMatchConditions(matchConditions []api.WebhookMatchCondition, fldPath | |||||||
| 	compiler := authorizationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) | 	compiler := authorizationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) | ||||||
| 	seenExpressions := sets.NewString() | 	seenExpressions := sets.NewString() | ||||||
| 	var compilationResults []authorizationcel.CompilationResult | 	var compilationResults []authorizationcel.CompilationResult | ||||||
|  | 	var usesFieldSelector, usesLabelSelector bool | ||||||
|  |  | ||||||
| 	for i, condition := range matchConditions { | 	for i, condition := range matchConditions { | ||||||
| 		fldPath := fldPath.Child("matchConditions").Index(i).Child("expression") | 		fldPath := fldPath.Child("matchConditions").Index(i).Child("expression") | ||||||
| @@ -754,12 +755,16 @@ func compileMatchConditions(matchConditions []api.WebhookMatchCondition, fldPath | |||||||
| 			continue | 			continue | ||||||
| 		} | 		} | ||||||
| 		compilationResults = append(compilationResults, compilationResult) | 		compilationResults = append(compilationResults, compilationResult) | ||||||
|  | 		usesFieldSelector = usesFieldSelector || compilationResult.UsesFieldSelector | ||||||
|  | 		usesLabelSelector = usesLabelSelector || compilationResult.UsesLabelSelector | ||||||
| 	} | 	} | ||||||
| 	if len(compilationResults) == 0 { | 	if len(compilationResults) == 0 { | ||||||
| 		return nil, allErrs | 		return nil, allErrs | ||||||
| 	} | 	} | ||||||
| 	return &authorizationcel.CELMatcher{ | 	return &authorizationcel.CELMatcher{ | ||||||
| 		CompilationResults: compilationResults, | 		CompilationResults: compilationResults, | ||||||
|  | 		UsesFieldSelector:  usesFieldSelector, | ||||||
|  | 		UsesLabelSelector:  usesLabelSelector, | ||||||
| 	}, allErrs | 	}, allErrs | ||||||
| } | } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -20,22 +20,34 @@ import ( | |||||||
| 	"fmt" | 	"fmt" | ||||||
|  |  | ||||||
| 	"github.com/google/cel-go/cel" | 	"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" | 	"github.com/google/cel-go/common/types/ref" | ||||||
|  |  | ||||||
| 	authorizationv1 "k8s.io/api/authorization/v1" | 	authorizationv1 "k8s.io/api/authorization/v1" | ||||||
| 	"k8s.io/apimachinery/pkg/util/version" | 	"k8s.io/apimachinery/pkg/util/version" | ||||||
| 	apiservercel "k8s.io/apiserver/pkg/cel" | 	apiservercel "k8s.io/apiserver/pkg/cel" | ||||||
| 	"k8s.io/apiserver/pkg/cel/environment" | 	"k8s.io/apiserver/pkg/cel/environment" | ||||||
|  | 	genericfeatures "k8s.io/apiserver/pkg/features" | ||||||
|  | 	utilfeature "k8s.io/apiserver/pkg/util/feature" | ||||||
| ) | ) | ||||||
|  |  | ||||||
| const ( | const ( | ||||||
| 	subjectAccessReviewRequestVarName = "request" | 	subjectAccessReviewRequestVarName = "request" | ||||||
|  |  | ||||||
|  | 	fieldSelectorVarName = "fieldSelector" | ||||||
|  | 	labelSelectorVarName = "labelSelector" | ||||||
| ) | ) | ||||||
|  |  | ||||||
| // CompilationResult represents a compiled authorization cel expression. | // CompilationResult represents a compiled authorization cel expression. | ||||||
| type CompilationResult struct { | type CompilationResult struct { | ||||||
| 	Program            cel.Program | 	Program            cel.Program | ||||||
| 	ExpressionAccessor ExpressionAccessor | 	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 | // 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) | 		return resultError(reason, apiservercel.ErrorTypeInvalid) | ||||||
| 	} | 	} | ||||||
| 	_, err = cel.AstToCheckedExpr(ast) | 	checkedExpr, err := cel.AstToCheckedExpr(ast) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		// should be impossible since env.Compile returned no issues | 		// should be impossible since env.Compile returned no issues | ||||||
| 		return resultError("unexpected compilation error: "+err.Error(), apiservercel.ErrorTypeInternal) | 		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) | 	prog, err := env.Program(ast) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return resultError("program instantiation failed: "+err.Error(), apiservercel.ErrorTypeInternal) | 		return resultError("program instantiation failed: "+err.Error(), apiservercel.ErrorTypeInternal) | ||||||
| @@ -108,6 +159,8 @@ func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor) (C | |||||||
| 	return CompilationResult{ | 	return CompilationResult{ | ||||||
| 		Program:            prog, | 		Program:            prog, | ||||||
| 		ExpressionAccessor: expressionAccessor, | 		ExpressionAccessor: expressionAccessor, | ||||||
|  | 		UsesFieldSelector:  usesFieldSelector, | ||||||
|  | 		UsesLabelSelector:  usesLabelSelector, | ||||||
| 	}, nil | 	}, nil | ||||||
| } | } | ||||||
|  |  | ||||||
| @@ -161,7 +214,7 @@ func buildRequestType(field func(name string, declType *apiservercel.DeclType, r | |||||||
| // buildResourceAttributesType generates a DeclType for ResourceAttributes. | // buildResourceAttributesType generates a DeclType for ResourceAttributes. | ||||||
| // if attributes are added here, also add to convertObjectToUnstructured. | // 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 { | 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("namespace", apiservercel.StringType, false), | ||||||
| 		field("verb", apiservercel.StringType, false), | 		field("verb", apiservercel.StringType, false), | ||||||
| 		field("group", 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("resource", apiservercel.StringType, false), | ||||||
| 		field("subresource", apiservercel.StringType, false), | 		field("subresource", apiservercel.StringType, false), | ||||||
| 		field("name", 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 | 	// Construct version containing every SubjectAccessReview user and string attribute field, even omitempty ones, for evaluation by CEL | ||||||
| 	extra := obj.Extra | 	extra := obj.Extra | ||||||
| 	if extra == nil { | 	if extra == nil { | ||||||
| @@ -194,7 +274,7 @@ func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) m | |||||||
| 		"extra":  extra, | 		"extra":  extra, | ||||||
| 	} | 	} | ||||||
| 	if obj.ResourceAttributes != nil { | 	if obj.ResourceAttributes != nil { | ||||||
| 		ret["resourceAttributes"] = map[string]string{ | 		resourceAttributes := map[string]interface{}{ | ||||||
| 			"namespace":   obj.ResourceAttributes.Namespace, | 			"namespace":   obj.ResourceAttributes.Namespace, | ||||||
| 			"verb":        obj.ResourceAttributes.Verb, | 			"verb":        obj.ResourceAttributes.Verb, | ||||||
| 			"group":       obj.ResourceAttributes.Group, | 			"group":       obj.ResourceAttributes.Group, | ||||||
| @@ -203,6 +283,42 @@ func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) m | |||||||
| 			"subresource": obj.ResourceAttributes.Subresource, | 			"subresource": obj.ResourceAttributes.Subresource, | ||||||
| 			"name":        obj.ResourceAttributes.Name, | 			"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 { | 	if obj.NonResourceAttributes != nil { | ||||||
| 		ret["nonResourceAttributes"] = map[string]string{ | 		ret["nonResourceAttributes"] = map[string]string{ | ||||||
|   | |||||||
| @@ -23,8 +23,12 @@ import ( | |||||||
| 	"testing" | 	"testing" | ||||||
|  |  | ||||||
| 	v1 "k8s.io/api/authorization/v1" | 	v1 "k8s.io/api/authorization/v1" | ||||||
|  | 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||
| 	apiservercel "k8s.io/apiserver/pkg/cel" | 	apiservercel "k8s.io/apiserver/pkg/cel" | ||||||
| 	"k8s.io/apiserver/pkg/cel/environment" | 	"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) { | func TestCompileCELExpression(t *testing.T) { | ||||||
| @@ -32,6 +36,8 @@ func TestCompileCELExpression(t *testing.T) { | |||||||
| 		name          string | 		name          string | ||||||
| 		expression    string | 		expression    string | ||||||
| 		expectedError string | 		expectedError string | ||||||
|  |  | ||||||
|  | 		authorizeWithSelectorsEnabled bool | ||||||
| 	}{ | 	}{ | ||||||
| 		{ | 		{ | ||||||
| 			name:       "SubjectAccessReviewSpec user comparison", | 			name:       "SubjectAccessReviewSpec user comparison", | ||||||
| @@ -57,12 +63,44 @@ func TestCompileCELExpression(t *testing.T) { | |||||||
| 			expression:    "x.user", | 			expression:    "x.user", | ||||||
| 			expectedError: "undeclared reference", | 			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 { | 	for _, tc := range cases { | ||||||
| 		t.Run(tc.name, func(t *testing.T) { | 		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{ | 			_, err := compiler.CompileCELExpression(&SubjectAccessReviewMatchCondition{ | ||||||
| 				Expression: tc.expression, | 				Expression: tc.expression, | ||||||
| 			}) | 			}) | ||||||
| @@ -77,6 +115,7 @@ func TestCompileCELExpression(t *testing.T) { | |||||||
| } | } | ||||||
|  |  | ||||||
| func TestBuildRequestType(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 { | 	f := func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField { | ||||||
| 		return apiservercel.NewDeclField(name, declType, required, nil, nil) | 		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) | 			t.Fatalf("expected field %q to be present", nativeField.Name) | ||||||
| 		} | 		} | ||||||
| 		declFieldType := nativeTypeToCELType(t, nativeField.Type, field, fields) | 		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) | 			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 { | 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 { | 	switch nativeType { | ||||||
| 	case reflect.TypeOf(""): | 	case reflect.TypeOf(""), reflect.TypeOf(metav1.LabelSelectorOperator("")), reflect.TypeOf(metav1.FieldSelectorOperator("")): | ||||||
| 		return apiservercel.StringType | 		return apiservercel.StringType | ||||||
| 	case reflect.TypeOf([]string{}): | 	case reflect.TypeOf([]string{}): | ||||||
| 		return apiservercel.NewListType(apiservercel.StringType, -1) | 		return apiservercel.NewListType(apiservercel.StringType, -1) | ||||||
| @@ -151,6 +193,34 @@ func nativeTypeToCELType(t *testing.T, nativeType reflect.Type, field func(name | |||||||
| 			return nil | 			return nil | ||||||
| 		} | 		} | ||||||
| 		return nonResourceAttributesDeclType | 		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: | 	default: | ||||||
| 		t.Fatalf("unsupported type %v", nativeType) | 		t.Fatalf("unsupported type %v", nativeType) | ||||||
| 	} | 	} | ||||||
|   | |||||||
| @@ -30,6 +30,11 @@ import ( | |||||||
| type CELMatcher struct { | type CELMatcher struct { | ||||||
| 	CompilationResults []CompilationResult | 	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 | 	// These are optional fields which can be populated if metrics reporting is desired | ||||||
| 	Metrics        MatcherMetrics | 	Metrics        MatcherMetrics | ||||||
| 	AuthorizerType string | 	AuthorizerType string | ||||||
| @@ -53,7 +58,7 @@ func (c *CELMatcher) Eval(ctx context.Context, r *authorizationv1.SubjectAccessR | |||||||
| 	}() | 	}() | ||||||
|  |  | ||||||
| 	va := map[string]interface{}{ | 	va := map[string]interface{}{ | ||||||
| 		"request": convertObjectToUnstructured(&r.Spec), | 		"request": convertObjectToUnstructured(&r.Spec, c.UsesFieldSelector, c.UsesLabelSelector), | ||||||
| 	} | 	} | ||||||
| 	for _, compilationResult := range c.CompilationResults { | 	for _, compilationResult := range c.CompilationResults { | ||||||
| 		evalResult, _, err := compilationResult.Program.ContextEval(ctx, va) | 		evalResult, _, err := compilationResult.Program.ContextEval(ctx, va) | ||||||
|   | |||||||
| @@ -40,6 +40,9 @@ import ( | |||||||
|  |  | ||||||
| 	authorizationv1 "k8s.io/api/authorization/v1" | 	authorizationv1 "k8s.io/api/authorization/v1" | ||||||
| 	metav1 "k8s.io/apimachinery/pkg/apis/meta/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/apimachinery/pkg/util/wait" | ||||||
| 	"k8s.io/apiserver/pkg/apis/apiserver" | 	"k8s.io/apiserver/pkg/apis/apiserver" | ||||||
| 	"k8s.io/apiserver/pkg/authentication/user" | 	"k8s.io/apiserver/pkg/authentication/user" | ||||||
| @@ -700,6 +703,8 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { | |||||||
| 	} | 	} | ||||||
| 	defer s.Close() | 	defer s.Close() | ||||||
|  |  | ||||||
|  | 	labelRequirement, _ := labels.NewRequirement("baz", selection.Equals, []string{"qux"}) | ||||||
|  |  | ||||||
| 	type webhookMatchConditionsTestCase struct { | 	type webhookMatchConditionsTestCase struct { | ||||||
| 		name               string | 		name               string | ||||||
| 		attr               authorizer.AttributesRecord | 		attr               authorizer.AttributesRecord | ||||||
| @@ -709,6 +714,7 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { | |||||||
| 		expectedDecision   authorizer.Decision | 		expectedDecision   authorizer.Decision | ||||||
| 		expressions        []apiserver.WebhookMatchCondition | 		expressions        []apiserver.WebhookMatchCondition | ||||||
| 		featureEnabled     bool | 		featureEnabled     bool | ||||||
|  | 		selectorEnabled    bool | ||||||
| 	} | 	} | ||||||
| 	aliceAttr := authorizer.AttributesRecord{ | 	aliceAttr := authorizer.AttributesRecord{ | ||||||
| 		User: &user.DefaultInfo{ | 		User: &user.DefaultInfo{ | ||||||
| @@ -721,6 +727,19 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { | |||||||
| 		Namespace:       "kittensandponies", | 		Namespace:       "kittensandponies", | ||||||
| 		Verb:            "get", | 		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{ | 	tests := []webhookMatchConditionsTestCase{ | ||||||
| 		{ | 		{ | ||||||
| 			name:               "no match condition does not require feature enablement", | 			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", | 			name:               "feature enabled, match all against all expressions", | ||||||
| 			attr:               aliceAttr, | 			attr:               aliceWithSelectorsAttr, | ||||||
| 			allow:              true, | 			allow:              true, | ||||||
| 			expectedCompileErr: false, | 			expectedCompileErr: false, | ||||||
| 			expectedDecision:   authorizer.DecisionAllow, | 			expectedDecision:   authorizer.DecisionAllow, | ||||||
| @@ -763,14 +782,28 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { | |||||||
| 				{ | 				{ | ||||||
| 					Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", | 					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 { | 	for i, test := range tests { | ||||||
| 		t.Run(test.name, func(t *testing.T) { | 		t.Run(test.name, func(t *testing.T) { | ||||||
| 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthorizationConfiguration, test.featureEnabled) | 			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, "") | 			wh, err := newV1Authorizer(s.URL, clientCert, clientKey, caCert, 0, noopAuthorizerMetrics(), test.expressions, "") | ||||||
| 			if test.expectedCompileErr && err == nil { | 			if test.expectedCompileErr && err == nil { | ||||||
| 				t.Fatalf("%d: Expected compile error", i) | 				t.Fatalf("%d: Expected compile error", i) | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Jordan Liggitt
					Jordan Liggitt