diff --git a/pkg/apis/authorization/validation/validation.go b/pkg/apis/authorization/validation/validation.go index 06b9a8da1de..2548cf704e8 100644 --- a/pkg/apis/authorization/validation/validation.go +++ b/pkg/apis/authorization/validation/validation.go @@ -17,8 +17,11 @@ limitations under the License. package validation import ( + "fmt" + apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/util/validation/field" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" ) @@ -36,6 +39,7 @@ func ValidateSubjectAccessReviewSpec(spec authorizationapi.SubjectAccessReviewSp if len(spec.User) == 0 && len(spec.Groups) == 0 { allErrs = append(allErrs, field.Invalid(fldPath.Child("user"), spec.User, `at least one of user or group must be specified`)) } + allErrs = append(allErrs, validateResourceAttributes(spec.ResourceAttributes, field.NewPath("spec.resourceAttributes"))...) return allErrs } @@ -50,6 +54,7 @@ func ValidateSelfSubjectAccessReviewSpec(spec authorizationapi.SelfSubjectAccess if spec.ResourceAttributes == nil && spec.NonResourceAttributes == nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("resourceAttributes"), spec.NonResourceAttributes, `exactly one of nonResourceAttributes or resourceAttributes must be specified`)) } + allErrs = append(allErrs, validateResourceAttributes(spec.ResourceAttributes, field.NewPath("spec.resourceAttributes"))...) return allErrs } @@ -99,3 +104,59 @@ func ValidateLocalSubjectAccessReview(sar *authorizationapi.LocalSubjectAccessRe return allErrs } + +func validateResourceAttributes(resourceAttributes *authorizationapi.ResourceAttributes, fldPath *field.Path) field.ErrorList { + if resourceAttributes == nil { + return nil + } + allErrs := field.ErrorList{} + + allErrs = append(allErrs, validateFieldSelectorAttributes(resourceAttributes.FieldSelector, fldPath.Child("fieldSelector"))...) + allErrs = append(allErrs, validateLabelSelectorAttributes(resourceAttributes.LabelSelector, fldPath.Child("labelSelector"))...) + + return allErrs +} + +func validateFieldSelectorAttributes(selector *authorizationapi.FieldSelectorAttributes, fldPath *field.Path) field.ErrorList { + if selector == nil { + return nil + } + allErrs := field.ErrorList{} + + if len(selector.RawSelector) > 0 && len(selector.Requirements) > 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("rawSelector"), selector.RawSelector, "may not specified at the same time as requirements")) + } + if len(selector.RawSelector) == 0 && len(selector.Requirements) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("requirements"), fmt.Sprintf("when %s is specified, requirements or rawSelector is required", fldPath))) + } + + // AllowUnknownOperatorInRequirement enables *SubjectAccessReview requests from newer skewed clients which understand operators kube-apiserver does not know about to be authorized. + validationOptions := metav1validation.FieldSelectorValidationOptions{AllowUnknownOperatorInRequirement: true} + for i, requirement := range selector.Requirements { + allErrs = append(allErrs, metav1validation.ValidateFieldSelectorRequirement(requirement, validationOptions, fldPath.Child("requirements").Index(i))...) + } + + return allErrs +} + +func validateLabelSelectorAttributes(selector *authorizationapi.LabelSelectorAttributes, fldPath *field.Path) field.ErrorList { + if selector == nil { + return nil + } + allErrs := field.ErrorList{} + + if len(selector.RawSelector) > 0 && len(selector.Requirements) > 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("rawSelector"), selector.RawSelector, "may not specified at the same time as requirements")) + } + if len(selector.RawSelector) == 0 && len(selector.Requirements) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("requirements"), fmt.Sprintf("when %s is specified, requirements or rawSelector is required", fldPath))) + } + + // AllowUnknownOperatorInRequirement enables *SubjectAccessReview requests from newer skewed clients which understand operators kube-apiserver does not know about to be authorized. + validationOptions := metav1validation.LabelSelectorValidationOptions{AllowUnknownOperatorInRequirement: true} + for i, requirement := range selector.Requirements { + allErrs = append(allErrs, metav1validation.ValidateLabelSelectorRequirement(requirement, validationOptions, fldPath.Child("requirements").Index(i))...) + } + + return allErrs +} diff --git a/pkg/apis/authorization/validation/validation_test.go b/pkg/apis/authorization/validation/validation_test.go index 36e6d46dbe7..feb8627e0e0 100644 --- a/pkg/apis/authorization/validation/validation_test.go +++ b/pkg/apis/authorization/validation/validation_test.go @@ -29,6 +29,50 @@ func TestValidateSARSpec(t *testing.T) { successCases := []authorizationapi.SubjectAccessReviewSpec{ {ResourceAttributes: &authorizationapi.ResourceAttributes{}, User: "me"}, {NonResourceAttributes: &authorizationapi.NonResourceAttributes{}, Groups: []string{"my-group"}}, + { // field raw selector + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + RawSelector: "***foo", + }, + }, + }, + { // label raw selector + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + RawSelector: "***foo", + }, + }, + }, + { // unknown field operator + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "k", + Operator: metav1.FieldSelectorOperator("fake"), + Values: []string{"val"}, + }, + }, + }, + }, + }, + { // unknown label operator + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "k", + Operator: metav1.LabelSelectorOperator("fake"), + Values: []string{"val"}, + }, + }, + }, + }, + }, } for _, successCase := range successCases { if errs := ValidateSubjectAccessReviewSpec(successCase, field.NewPath("spec")); len(errs) != 0 { @@ -58,29 +102,257 @@ func TestValidateSARSpec(t *testing.T) { ResourceAttributes: &authorizationapi.ResourceAttributes{}, }, msg: `spec.user: Invalid value: "": at least one of user or group must be specified`, + }, { + name: "resource attributes: field selector specify both", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + RawSelector: "foo", + Requirements: []metav1.FieldSelectorRequirement{ + {}, + }, + }, + }, + }, + msg: `spec.resourceAttributes.fieldSelector.rawSelector: Invalid value: "foo": may not specified at the same time as requirements`, + }, { + name: "resource attributes: field selector specify neither", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{}, + }, + }, + msg: `spec.resourceAttributes.fieldSelector.requirements: Required value: when spec.resourceAttributes.fieldSelector is specified, requirements or rawSelector is required`, + }, { + name: "resource attributes: field selector no key", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "", + }, + }, + }, + }, + }, + msg: `spec.resourceAttributes.fieldSelector.requirements[0].key: Required value: must be specified`, + }, { + name: "resource attributes: field selector no value for in", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "k", + Operator: metav1.FieldSelectorOpIn, + Values: []string{}, + }, + }, + }, + }, + }, + msg: "spec.resourceAttributes.fieldSelector.requirements[0].values: Required value: must be specified when `operator` is 'In' or 'NotIn'", + }, { + name: "resource attributes: field selector no value for not in", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "k", + Operator: metav1.FieldSelectorOpNotIn, + Values: []string{}, + }, + }, + }, + }, + }, + msg: "spec.resourceAttributes.fieldSelector.requirements[0].values: Required value: must be specified when `operator` is 'In' or 'NotIn'", + }, { + name: "resource attributes: field selector values for exists", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "k", + Operator: metav1.FieldSelectorOpExists, + Values: []string{"val"}, + }, + }, + }, + }, + }, + msg: "spec.resourceAttributes.fieldSelector.requirements[0].values: Forbidden: may not be specified when `operator` is 'Exists' or 'DoesNotExist'", + }, { + name: "resource attributes: field selector values for not exists", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "k", + Operator: metav1.FieldSelectorOpDoesNotExist, + Values: []string{"val"}, + }, + }, + }, + }, + }, + msg: "spec.resourceAttributes.fieldSelector.requirements[0].values: Forbidden: may not be specified when `operator` is 'Exists' or 'DoesNotExist'", + }, { + name: "resource attributes: label selector specify both", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + RawSelector: "foo", + Requirements: []metav1.LabelSelectorRequirement{ + {}, + }, + }, + }, + }, + msg: `spec.resourceAttributes.labelSelector.rawSelector: Invalid value: "foo": may not specified at the same time as requirements`, + }, { + name: "resource attributes: label selector specify neither", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{}, + }, + }, + msg: `spec.resourceAttributes.labelSelector.requirements: Required value: when spec.resourceAttributes.labelSelector is specified, requirements or rawSelector is required`, + }, { + name: "resource attributes: label selector no key", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "", + }, + }, + }, + }, + }, + msg: `spec.resourceAttributes.labelSelector.requirements[0].key: Invalid value: "": name part must be non-empty`, + }, { + name: "resource attributes: label selector invalid label name", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "()foo", + }, + }, + }, + }, + }, + msg: `spec.resourceAttributes.labelSelector.requirements[0].key: Invalid value: "()foo": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')`, + }, { + name: "resource attributes: label selector no value for in", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "k", + Operator: metav1.LabelSelectorOpIn, + Values: []string{}, + }, + }, + }, + }, + }, + msg: "spec.resourceAttributes.labelSelector.requirements[0].values: Required value: must be specified when `operator` is 'In' or 'NotIn'", + }, { + name: "resource attributes: label selector no value for not in", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "k", + Operator: metav1.LabelSelectorOpNotIn, + Values: []string{}, + }, + }, + }, + }, + }, + msg: "spec.resourceAttributes.labelSelector.requirements[0].values: Required value: must be specified when `operator` is 'In' or 'NotIn'", + }, { + name: "resource attributes: label selector values for exists", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "k", + Operator: metav1.LabelSelectorOpExists, + Values: []string{"val"}, + }, + }, + }, + }, + }, + msg: "spec.resourceAttributes.labelSelector.requirements[0].values: Forbidden: may not be specified when `operator` is 'Exists' or 'DoesNotExist'", + }, { + name: "resource attributes: label selector values for not exists", + obj: authorizationapi.SubjectAccessReviewSpec{ + User: "me", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "k", + Operator: metav1.LabelSelectorOpDoesNotExist, + Values: []string{"val"}, + }, + }, + }, + }, + }, + msg: "spec.resourceAttributes.labelSelector.requirements[0].values: Forbidden: may not be specified when `operator` is 'Exists' or 'DoesNotExist'", }} for _, c := range errorCases { - errs := ValidateSubjectAccessReviewSpec(c.obj, field.NewPath("spec")) - if len(errs) == 0 { - t.Errorf("%s: expected failure for %q", c.name, c.msg) - } else if !strings.Contains(errs[0].Error(), c.msg) { - t.Errorf("%s: unexpected error: %q, expected: %q", c.name, errs[0], c.msg) - } - - errs = ValidateSubjectAccessReview(&authorizationapi.SubjectAccessReview{Spec: c.obj}) - if len(errs) == 0 { - t.Errorf("%s: expected failure for %q", c.name, c.msg) - } else if !strings.Contains(errs[0].Error(), c.msg) { - t.Errorf("%s: unexpected error: %q, expected: %q", c.name, errs[0], c.msg) - } - errs = ValidateLocalSubjectAccessReview(&authorizationapi.LocalSubjectAccessReview{Spec: c.obj}) - if len(errs) == 0 { - t.Errorf("%s: expected failure for %q", c.name, c.msg) - } else if !strings.Contains(errs[0].Error(), c.msg) { - t.Errorf("%s: unexpected error: %q, expected: %q", c.name, errs[0], c.msg) - } + t.Run(c.name, func(t *testing.T) { + errs := ValidateSubjectAccessReviewSpec(c.obj, field.NewPath("spec")) + if len(errs) == 0 { + t.Errorf("%s: expected failure for %q", c.name, c.msg) + } else if !strings.Contains(errs[0].Error(), c.msg) { + t.Errorf("%s: unexpected error: %q, expected: %q", c.name, errs[0], c.msg) + } + errs = ValidateSubjectAccessReview(&authorizationapi.SubjectAccessReview{Spec: c.obj}) + if len(errs) == 0 { + t.Errorf("%s: expected failure for %q", c.name, c.msg) + } else if !strings.Contains(errs[0].Error(), c.msg) { + t.Errorf("%s: unexpected error: %q, expected: %q", c.name, errs[0], c.msg) + } + errs = ValidateLocalSubjectAccessReview(&authorizationapi.LocalSubjectAccessReview{Spec: c.obj}) + if len(errs) == 0 { + t.Errorf("%s: expected failure for %q", c.name, c.msg) + } else if !strings.Contains(errs[0].Error(), c.msg) { + t.Errorf("%s: unexpected error: %q, expected: %q", c.name, errs[0], c.msg) + } + }) } } @@ -109,6 +381,20 @@ func TestValidateSelfSAR(t *testing.T) { NonResourceAttributes: &authorizationapi.NonResourceAttributes{}, }, msg: "cannot be specified in combination with resourceAttributes", + }, { + // here we only test one to be sure the function is called. The more exhaustive suite is tested above. + name: "resource attributes: label selector specify both", + obj: authorizationapi.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + RawSelector: "foo", + Requirements: []metav1.LabelSelectorRequirement{ + {}, + }, + }, + }, + }, + msg: `spec.resourceAttributes.labelSelector.rawSelector: Invalid value: "foo": may not specified at the same time as requirements`, }} for _, c := range errorCases { @@ -175,6 +461,23 @@ func TestValidateLocalSAR(t *testing.T) { }, }, msg: "disallowed on this kind of request", + }, { + // here we only test one to be sure the function is called. The more exhaustive suite is tested above. + name: "resource attributes: label selector specify both", + obj: &authorizationapi.LocalSubjectAccessReview{ + Spec: authorizationapi.SubjectAccessReviewSpec{ + User: "user", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + RawSelector: "foo", + Requirements: []metav1.LabelSelectorRequirement{ + {}, + }, + }, + }, + }, + }, + msg: `spec.resourceAttributes.labelSelector.rawSelector: Invalid value: "foo": may not specified at the same time as requirements`, }} for _, c := range errorCases { diff --git a/pkg/controlplane/import_known_versions_test.go b/pkg/controlplane/import_known_versions_test.go index 7ba0187a497..1bdeb8b03c4 100644 --- a/pkg/controlplane/import_known_versions_test.go +++ b/pkg/controlplane/import_known_versions_test.go @@ -55,23 +55,25 @@ func TestGroupVersions(t *testing.T) { // but are also registered in internal versions (or referenced from internal types), // so we explicitly allow tags for them var typesAllowedTags = map[reflect.Type]bool{ - reflect.TypeOf(intstr.IntOrString{}): true, - reflect.TypeOf(metav1.Time{}): true, - reflect.TypeOf(metav1.MicroTime{}): true, - reflect.TypeOf(metav1.Duration{}): true, - reflect.TypeOf(metav1.TypeMeta{}): true, - reflect.TypeOf(metav1.ListMeta{}): true, - reflect.TypeOf(metav1.ObjectMeta{}): true, - reflect.TypeOf(metav1.OwnerReference{}): true, - reflect.TypeOf(metav1.LabelSelector{}): true, - reflect.TypeOf(metav1.GetOptions{}): true, - reflect.TypeOf(metav1.ListOptions{}): true, - reflect.TypeOf(metav1.DeleteOptions{}): true, - reflect.TypeOf(metav1.GroupVersionKind{}): true, - reflect.TypeOf(metav1.GroupVersionResource{}): true, - reflect.TypeOf(metav1.Status{}): true, - reflect.TypeOf(metav1.Condition{}): true, - reflect.TypeOf(runtime.RawExtension{}): true, + reflect.TypeOf(intstr.IntOrString{}): true, + reflect.TypeOf(metav1.Time{}): true, + reflect.TypeOf(metav1.MicroTime{}): true, + reflect.TypeOf(metav1.Duration{}): true, + reflect.TypeOf(metav1.TypeMeta{}): true, + reflect.TypeOf(metav1.ListMeta{}): true, + reflect.TypeOf(metav1.ObjectMeta{}): true, + reflect.TypeOf(metav1.OwnerReference{}): true, + reflect.TypeOf(metav1.LabelSelector{}): true, + reflect.TypeOf(metav1.LabelSelectorRequirement{}): true, + reflect.TypeOf(metav1.FieldSelectorRequirement{}): true, + reflect.TypeOf(metav1.GetOptions{}): true, + reflect.TypeOf(metav1.ListOptions{}): true, + reflect.TypeOf(metav1.DeleteOptions{}): true, + reflect.TypeOf(metav1.GroupVersionKind{}): true, + reflect.TypeOf(metav1.GroupVersionResource{}): true, + reflect.TypeOf(metav1.Status{}): true, + reflect.TypeOf(metav1.Condition{}): true, + reflect.TypeOf(runtime.RawExtension{}): true, } // These fields are limited exceptions to the standard JSON naming structure. diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 93598a7b7c5..51eb8fed5e4 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -1253,6 +1253,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS genericfeatures.APIServingWithRoutine: {Default: true, PreRelease: featuregate.Beta}, + genericfeatures.AuthorizeWithSelectors: {Default: false, PreRelease: featuregate.Alpha}, + genericfeatures.ConsistentListFromCache: {Default: false, PreRelease: featuregate.Alpha}, genericfeatures.CustomResourceValidationExpressions: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.31 diff --git a/pkg/registry/authorization/localsubjectaccessreview/rest.go b/pkg/registry/authorization/localsubjectaccessreview/rest.go index 78389075064..f961a954351 100644 --- a/pkg/registry/authorization/localsubjectaccessreview/rest.go +++ b/pkg/registry/authorization/localsubjectaccessreview/rest.go @@ -25,7 +25,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authorization/authorizer" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + genericfeatures "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" + utilfeature "k8s.io/apiserver/pkg/util/feature" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" authorizationvalidation "k8s.io/kubernetes/pkg/apis/authorization/validation" authorizationutil "k8s.io/kubernetes/pkg/registry/authorization/util" @@ -64,6 +66,14 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("not a LocaLocalSubjectAccessReview: %#v", obj)) } + // clear fields if the featuregate is disabled + if !utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + if localSubjectAccessReview.Spec.ResourceAttributes != nil { + localSubjectAccessReview.Spec.ResourceAttributes.FieldSelector = nil + localSubjectAccessReview.Spec.ResourceAttributes.LabelSelector = nil + } + } + if errs := authorizationvalidation.ValidateLocalSubjectAccessReview(localSubjectAccessReview); len(errs) > 0 { return nil, apierrors.NewInvalid(authorizationapi.Kind(localSubjectAccessReview.Kind), "", errs) } @@ -89,9 +99,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation Denied: (decision == authorizer.DecisionDeny), Reason: reason, } - if evaluationErr != nil { - localSubjectAccessReview.Status.EvaluationError = evaluationErr.Error() - } + localSubjectAccessReview.Status.EvaluationError = authorizationutil.BuildEvaluationError(evaluationErr, authorizationAttributes) return localSubjectAccessReview, nil } diff --git a/pkg/registry/authorization/selfsubjectaccessreview/rest.go b/pkg/registry/authorization/selfsubjectaccessreview/rest.go index f5e6be5227a..a64a84cabfa 100644 --- a/pkg/registry/authorization/selfsubjectaccessreview/rest.go +++ b/pkg/registry/authorization/selfsubjectaccessreview/rest.go @@ -25,7 +25,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authorization/authorizer" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + genericfeatures "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" + utilfeature "k8s.io/apiserver/pkg/util/feature" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" authorizationvalidation "k8s.io/kubernetes/pkg/apis/authorization/validation" authorizationutil "k8s.io/kubernetes/pkg/registry/authorization/util" @@ -64,6 +66,13 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("not a SelfSubjectAccessReview: %#v", obj)) } + // clear fields if the featuregate is disabled + if !utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + if selfSAR.Spec.ResourceAttributes != nil { + selfSAR.Spec.ResourceAttributes.FieldSelector = nil + selfSAR.Spec.ResourceAttributes.LabelSelector = nil + } + } if errs := authorizationvalidation.ValidateSelfSubjectAccessReview(selfSAR); len(errs) > 0 { return nil, apierrors.NewInvalid(authorizationapi.Kind(selfSAR.Kind), "", errs) } @@ -92,9 +101,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation Denied: (decision == authorizer.DecisionDeny), Reason: reason, } - if evaluationErr != nil { - selfSAR.Status.EvaluationError = evaluationErr.Error() - } + selfSAR.Status.EvaluationError = authorizationutil.BuildEvaluationError(evaluationErr, authorizationAttributes) return selfSAR, nil } diff --git a/pkg/registry/authorization/subjectaccessreview/rest.go b/pkg/registry/authorization/subjectaccessreview/rest.go index 5a6edb56451..023123f75d4 100644 --- a/pkg/registry/authorization/subjectaccessreview/rest.go +++ b/pkg/registry/authorization/subjectaccessreview/rest.go @@ -24,7 +24,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authorization/authorizer" + genericfeatures "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" + utilfeature "k8s.io/apiserver/pkg/util/feature" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" authorizationvalidation "k8s.io/kubernetes/pkg/apis/authorization/validation" authorizationutil "k8s.io/kubernetes/pkg/registry/authorization/util" @@ -63,6 +65,13 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("not a SubjectAccessReview: %#v", obj)) } + // clear fields if the featuregate is disabled + if !utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + if subjectAccessReview.Spec.ResourceAttributes != nil { + subjectAccessReview.Spec.ResourceAttributes.FieldSelector = nil + subjectAccessReview.Spec.ResourceAttributes.LabelSelector = nil + } + } if errs := authorizationvalidation.ValidateSubjectAccessReview(subjectAccessReview); len(errs) > 0 { return nil, apierrors.NewInvalid(authorizationapi.Kind(subjectAccessReview.Kind), "", errs) } @@ -81,9 +90,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation Denied: (decision == authorizer.DecisionDeny), Reason: reason, } - if evaluationErr != nil { - subjectAccessReview.Status.EvaluationError = evaluationErr.Error() - } + subjectAccessReview.Status.EvaluationError = authorizationutil.BuildEvaluationError(evaluationErr, authorizationAttributes) return subjectAccessReview, nil } diff --git a/pkg/registry/authorization/subjectaccessreview/rest_test.go b/pkg/registry/authorization/subjectaccessreview/rest_test.go index 35f8b58449c..7816093c7c5 100644 --- a/pkg/registry/authorization/subjectaccessreview/rest_test.go +++ b/pkg/registry/authorization/subjectaccessreview/rest_test.go @@ -24,10 +24,15 @@ import ( "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + genericfeatures "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" ) @@ -187,8 +192,52 @@ func TestCreate(t *testing.T) { Denied: true, }, }, + + "resource denied, valid selectors": { + spec: authorizationapi.SubjectAccessReviewSpec{ + User: "bob", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{RawSelector: "foo=bar"}, + LabelSelector: &authorizationapi.LabelSelectorAttributes{RawSelector: "key=value"}, + }, + }, + decision: authorizer.DecisionDeny, + expectedAttrs: authorizer.AttributesRecord{ + User: &user.DefaultInfo{Name: "bob"}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorRequirements: fields.Requirements{{Operator: "=", Field: "foo", Value: "bar"}}, + LabelSelectorRequirements: mustParse("key=value"), + }, + expectedStatus: authorizationapi.SubjectAccessReviewStatus{ + Allowed: false, + Denied: true, + }, + }, + "resource denied, invalid selectors": { + spec: authorizationapi.SubjectAccessReviewSpec{ + User: "bob", + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{RawSelector: "key in value"}, + LabelSelector: &authorizationapi.LabelSelectorAttributes{RawSelector: "&"}, + }, + }, + decision: authorizer.DecisionDeny, + expectedAttrs: authorizer.AttributesRecord{ + User: &user.DefaultInfo{Name: "bob"}, + ResourceRequest: true, + APIVersion: "*", + }, + expectedStatus: authorizationapi.SubjectAccessReviewStatus{ + Allowed: false, + Denied: true, + EvaluationError: `spec.resourceAttributes.fieldSelector ignored due to parse error; spec.resourceAttributes.labelSelector ignored due to parse error`, + }, + }, } + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true) + for k, tc := range testcases { auth := &fakeAuthorizer{ decision: tc.decision, @@ -208,8 +257,13 @@ func TestCreate(t *testing.T) { } continue } - if !reflect.DeepEqual(auth.attrs, tc.expectedAttrs) { - t.Errorf("%s: expected\n%#v\ngot\n%#v", k, tc.expectedAttrs, auth.attrs) + gotAttrs := auth.attrs.(authorizer.AttributesRecord) + if tc.expectedStatus.EvaluationError != "" { + gotAttrs.FieldSelectorParsingErr = nil + gotAttrs.LabelSelectorParsingErr = nil + } + if !reflect.DeepEqual(gotAttrs, tc.expectedAttrs) { + t.Errorf("%s: expected\n%#v\ngot\n%#v", k, tc.expectedAttrs, gotAttrs) } status := result.(*authorizationapi.SubjectAccessReview).Status if !reflect.DeepEqual(status, tc.expectedStatus) { @@ -217,3 +271,12 @@ func TestCreate(t *testing.T) { } } } + +func mustParse(s string) labels.Requirements { + selector, err := labels.Parse(s) + if err != nil { + panic(err) + } + reqs, _ := selector.Requirements() + return reqs +} diff --git a/pkg/registry/authorization/util/helpers.go b/pkg/registry/authorization/util/helpers.go index f939f7b826a..d5ead7d7b90 100644 --- a/pkg/registry/authorization/util/helpers.go +++ b/pkg/registry/authorization/util/helpers.go @@ -17,14 +17,24 @@ limitations under the License. package util import ( + "fmt" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" ) // ResourceAttributesFrom combines the API object information and the user.Info from the context to build a full authorizer.AttributesRecord for resource access func ResourceAttributesFrom(user user.Info, in authorizationapi.ResourceAttributes) authorizer.AttributesRecord { - return authorizer.AttributesRecord{ + ret := authorizer.AttributesRecord{ User: user, Verb: in.Verb, Namespace: in.Namespace, @@ -35,6 +45,129 @@ func ResourceAttributesFrom(user user.Info, in authorizationapi.ResourceAttribut Name: in.Name, ResourceRequest: true, } + + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + if in.LabelSelector != nil { + if len(in.LabelSelector.RawSelector) > 0 { + labelSelector, err := labels.Parse(in.LabelSelector.RawSelector) + if err != nil { + ret.LabelSelectorRequirements, ret.LabelSelectorParsingErr = nil, err + } else { + requirements, _ /*selectable*/ := labelSelector.Requirements() + ret.LabelSelectorRequirements, ret.LabelSelectorParsingErr = requirements, nil + } + } + if len(in.LabelSelector.Requirements) > 0 { + ret.LabelSelectorRequirements, ret.LabelSelectorParsingErr = labelSelectorAsSelector(in.LabelSelector.Requirements) + } + } + + if in.FieldSelector != nil { + if len(in.FieldSelector.RawSelector) > 0 { + fieldSelector, err := fields.ParseSelector(in.FieldSelector.RawSelector) + if err != nil { + ret.FieldSelectorRequirements, ret.FieldSelectorParsingErr = nil, err + } else { + ret.FieldSelectorRequirements, ret.FieldSelectorParsingErr = fieldSelector.Requirements(), nil + } + } + if len(in.FieldSelector.Requirements) > 0 { + ret.FieldSelectorRequirements, ret.FieldSelectorParsingErr = fieldSelectorAsSelector(in.FieldSelector.Requirements) + } + } + } + + return ret +} + +var labelSelectorOpToSelectionOp = map[metav1.LabelSelectorOperator]selection.Operator{ + metav1.LabelSelectorOpIn: selection.In, + metav1.LabelSelectorOpNotIn: selection.NotIn, + metav1.LabelSelectorOpExists: selection.Exists, + metav1.LabelSelectorOpDoesNotExist: selection.DoesNotExist, +} + +func labelSelectorAsSelector(requirements []metav1.LabelSelectorRequirement) (labels.Requirements, error) { + if len(requirements) == 0 { + return nil, nil + } + reqs := make([]labels.Requirement, 0, len(requirements)) + var errs []error + for _, expr := range requirements { + op, ok := labelSelectorOpToSelectionOp[expr.Operator] + if !ok { + errs = append(errs, fmt.Errorf("%q is not a valid label selector operator", expr.Operator)) + continue + } + values := expr.Values + if len(values) == 0 { + values = nil + } + req, err := labels.NewRequirement(expr.Key, op, values) + if err != nil { + errs = append(errs, err) + continue + } + reqs = append(reqs, *req) + } + + // If this happens, it means all requirements ended up getting skipped. + // Return nil rather than []. + if len(reqs) == 0 { + reqs = nil + } + + // Return any accumulated errors along with any accumulated requirements, so recognized / valid requirements can be considered by authorization. + // This is safe because requirements are ANDed together so dropping unknown / invalid ones results in a strictly broader authorization check. + return labels.Requirements(reqs), utilerrors.NewAggregate(errs) +} + +func fieldSelectorAsSelector(requirements []metav1.FieldSelectorRequirement) (fields.Requirements, error) { + if len(requirements) == 0 { + return nil, nil + } + + reqs := make([]fields.Requirement, 0, len(requirements)) + var errs []error + for _, expr := range requirements { + if len(expr.Values) > 1 { + errs = append(errs, fmt.Errorf("fieldSelectors do not yet support multiple values")) + continue + } + + switch expr.Operator { + case metav1.FieldSelectorOpIn: + if len(expr.Values) != 1 { + errs = append(errs, fmt.Errorf("fieldSelectors in must have one value")) + continue + } + // when converting to fields.Requirement, use Equals to match how parsed field selectors behave + reqs = append(reqs, fields.Requirement{Field: expr.Key, Operator: selection.Equals, Value: expr.Values[0]}) + case metav1.FieldSelectorOpNotIn: + if len(expr.Values) != 1 { + errs = append(errs, fmt.Errorf("fieldSelectors not in must have one value")) + continue + } + // when converting to fields.Requirement, use NotEquals to match how parsed field selectors behave + reqs = append(reqs, fields.Requirement{Field: expr.Key, Operator: selection.NotEquals, Value: expr.Values[0]}) + case metav1.FieldSelectorOpExists, metav1.FieldSelectorOpDoesNotExist: + errs = append(errs, fmt.Errorf("fieldSelectors do not yet support %v", expr.Operator)) + continue + default: + errs = append(errs, fmt.Errorf("%q is not a valid field selector operator", expr.Operator)) + continue + } + } + + // If this happens, it means all requirements ended up getting skipped. + // Return nil rather than []. + if len(reqs) == 0 { + reqs = nil + } + + // Return any accumulated errors along with any accumulated requirements, so recognized / valid requirements can be considered by authorization. + // This is safe because requirements are ANDed together so dropping unknown / invalid ones results in a strictly broader authorization check. + return fields.Requirements(reqs), utilerrors.NewAggregate(errs) } // NonResourceAttributesFrom combines the API object information and the user.Info from the context to build a full authorizer.AttributesRecord for non resource access @@ -85,3 +218,27 @@ func matchAllVersionIfEmpty(version string) string { } return version } + +// BuildEvaluationError constructs the evaluation error string to include in *SubjectAccessReview status +// based on the authorizer evaluation error and any field and label selector parse errors. +func BuildEvaluationError(evaluationError error, attrs authorizer.AttributesRecord) string { + var evaluationErrors []string + if evaluationError != nil { + evaluationErrors = append(evaluationErrors, evaluationError.Error()) + } + if reqs, err := attrs.GetFieldSelector(); err != nil { + if len(reqs) > 0 { + evaluationErrors = append(evaluationErrors, "spec.resourceAttributes.fieldSelector partially ignored due to parse error") + } else { + evaluationErrors = append(evaluationErrors, "spec.resourceAttributes.fieldSelector ignored due to parse error") + } + } + if reqs, err := attrs.GetLabelSelector(); err != nil { + if len(reqs) > 0 { + evaluationErrors = append(evaluationErrors, "spec.resourceAttributes.labelSelector partially ignored due to parse error") + } else { + evaluationErrors = append(evaluationErrors, "spec.resourceAttributes.labelSelector ignored due to parse error") + } + } + return strings.Join(evaluationErrors, "; ") +} diff --git a/pkg/registry/authorization/util/helpers_test.go b/pkg/registry/authorization/util/helpers_test.go index c086e70e12a..0d59899e683 100644 --- a/pkg/registry/authorization/util/helpers_test.go +++ b/pkg/registry/authorization/util/helpers_test.go @@ -17,12 +17,21 @@ limitations under the License. package util import ( + "errors" "reflect" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" ) @@ -37,6 +46,10 @@ func TestResourceAttributesFrom(t *testing.T) { "Subresource", "Name", + // Fields we read and parse in ResourceAttributesFrom + "FieldSelector", + "LabelSelector", + // Fields we copy in NonResourceAttributesFrom "Path", "Verb", @@ -60,6 +73,12 @@ func TestResourceAttributesFrom(t *testing.T) { "Name", "ResourceRequest", + // Fields we compute and set in ResourceAttributesFrom + "FieldSelectorRequirements", + "FieldSelectorParsingErr", + "LabelSelectorRequirements", + "LabelSelectorParsingErr", + // Fields we set in NonResourceAttributesFrom "User", "ResourceRequest", @@ -75,13 +94,22 @@ func TestResourceAttributesFrom(t *testing.T) { } func TestAuthorizationAttributesFrom(t *testing.T) { + mustRequirement := func(key string, op selection.Operator, vals []string) labels.Requirement { + ret, err := labels.NewRequirement(key, op, vals) + if err != nil { + panic(err) + } + return *ret + } + type args struct { spec authorizationapi.SubjectAccessReviewSpec } tests := []struct { - name string - args args - want authorizer.AttributesRecord + name string + args args + want authorizer.AttributesRecord + enableAuthorizationSelector bool }{ { name: "nonresource", @@ -162,11 +190,461 @@ func TestAuthorizationAttributesFrom(t *testing.T) { ResourceRequest: true, }, }, + { + name: "field: ignore when featuregate off", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + RawSelector: "foo=bar", + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + }, + }, + { + name: "field: raw selector", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + RawSelector: "foo=bar", + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorRequirements: fields.Requirements{ + {Operator: "=", Field: "foo", Value: "bar"}, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "field: raw selector error", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + RawSelector: "&foo", + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorParsingErr: errors.New("invalid selector: '&foo'; can't understand '&foo'"), + }, + enableAuthorizationSelector: true, + }, + { + name: "field: requirements", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "one", + Operator: "In", + Values: []string{"apple"}, + }, + { + Key: "two", + Operator: "NotIn", + Values: []string{"banana"}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorRequirements: fields.Requirements{ + {Operator: "=", Field: "one", Value: "apple"}, + {Operator: "!=", Field: "two", Value: "banana"}, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "field: requirements too many values", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "one", + Operator: "In", + Values: []string{"apple", "other"}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorParsingErr: utilerrors.NewAggregate([]error{errors.New("fieldSelectors do not yet support multiple values")}), + }, + enableAuthorizationSelector: true, + }, + { + name: "field: requirements missing in value", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "one", + Operator: "In", + Values: []string{}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorParsingErr: utilerrors.NewAggregate([]error{errors.New("fieldSelectors in must have one value")}), + }, + enableAuthorizationSelector: true, + }, + { + name: "field: requirements missing notin value", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "one", + Operator: "NotIn", + Values: []string{}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorParsingErr: utilerrors.NewAggregate([]error{errors.New("fieldSelectors not in must have one value")}), + }, + enableAuthorizationSelector: true, + }, + { + name: "field: requirements exists", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "one", + Operator: "Exists", + Values: []string{}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorParsingErr: utilerrors.NewAggregate([]error{errors.New("fieldSelectors do not yet support Exists")}), + }, + enableAuthorizationSelector: true, + }, + { + name: "field: requirements DoesNotExist", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "one", + Operator: "DoesNotExist", + Values: []string{}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorParsingErr: utilerrors.NewAggregate([]error{errors.New("fieldSelectors do not yet support DoesNotExist")}), + }, + enableAuthorizationSelector: true, + }, + { + name: "field: requirements bad operator", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + FieldSelector: &authorizationapi.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "one", + Operator: "bad", + Values: []string{}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + FieldSelectorParsingErr: utilerrors.NewAggregate([]error{errors.New("\"bad\" is not a valid field selector operator")}), + }, + enableAuthorizationSelector: true, + }, + + { + name: "label: ignore when featuregate off", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + RawSelector: "foo=bar", + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + }, + }, + { + name: "label: raw selector", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + RawSelector: "foo=bar", + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + LabelSelectorRequirements: labels.Requirements{ + mustRequirement("foo", "=", []string{"bar"}), + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label: raw selector error", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + RawSelector: "&foo", + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + LabelSelectorParsingErr: errors.New("unable to parse requirement: : Invalid value: \"&foo\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"), + }, + enableAuthorizationSelector: true, + }, + { + name: "label: requirements", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "one", + Operator: "In", + Values: []string{"apple"}, + }, + { + Key: "two", + Operator: "NotIn", + Values: []string{"banana"}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + LabelSelectorRequirements: labels.Requirements{ + mustRequirement("one", "in", []string{"apple"}), + mustRequirement("two", "notin", []string{"banana"}), + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label: requirements multiple values", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "one", + Operator: "In", + Values: []string{"apple", "other"}, + }, + { + Key: "two", + Operator: "NotIn", + Values: []string{"carrot", "donut"}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + LabelSelectorRequirements: labels.Requirements{ + mustRequirement("one", "in", []string{"apple", "other"}), + mustRequirement("two", "notin", []string{"carrot", "donut"}), + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label: requirements exists", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "one", + Operator: "Exists", + Values: []string{}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + LabelSelectorRequirements: labels.Requirements{ + mustRequirement("one", "exists", nil), + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label: requirements DoesNotExist", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "one", + Operator: "DoesNotExist", + Values: []string{}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + LabelSelectorRequirements: labels.Requirements{ + mustRequirement("one", "!", nil), + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label: requirements bad operator", + args: args{ + spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + LabelSelector: &authorizationapi.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "one", + Operator: "bad", + Values: []string{}, + }, + }, + }, + }, + }, + }, + want: authorizer.AttributesRecord{ + User: &user.DefaultInfo{}, + ResourceRequest: true, + APIVersion: "*", + LabelSelectorParsingErr: utilerrors.NewAggregate([]error{errors.New("\"bad\" is not a valid label selector operator")}), + }, + enableAuthorizationSelector: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + if tt.enableAuthorizationSelector { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true) + } + if got := AuthorizationAttributesFrom(tt.args.spec); !reflect.DeepEqual(got, tt.want) { - t.Errorf("AuthorizationAttributesFrom() = %v, want %v", got, tt.want) + if got.LabelSelectorParsingErr != nil { + t.Logf("labelSelectorErr=%q", got.LabelSelectorParsingErr) + } + t.Errorf("AuthorizationAttributesFrom(), got:\n%#v\nwant:\n%#v", got, tt.want) } }) } diff --git a/plugin/pkg/auth/authorizer/rbac/rbac_test.go b/plugin/pkg/auth/authorizer/rbac/rbac_test.go index 6f61d12a433..dbfb2b891ee 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac_test.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac_test.go @@ -24,6 +24,8 @@ import ( rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" rbacv1helpers "k8s.io/kubernetes/pkg/apis/rbac/v1" @@ -135,6 +137,12 @@ func (d *defaultAttributes) GetAPIGroup() string { return d.apiGroup } func (d *defaultAttributes) GetAPIVersion() string { return "" } func (d *defaultAttributes) IsResourceRequest() bool { return true } func (d *defaultAttributes) GetPath() string { return "" } +func (d *defaultAttributes) GetFieldSelector() (fields.Requirements, error) { + panic("not supported for RBAC") +} +func (d *defaultAttributes) GetLabelSelector() (labels.Requirements, error) { + panic("not supported for RBAC") +} func TestAuthorizer(t *testing.T) { tests := []struct { @@ -263,135 +271,139 @@ func TestAuthorizer(t *testing.T) { } func TestRuleMatches(t *testing.T) { + type requestToTest struct { + request authorizer.AttributesRecord + expected bool + } tests := []struct { name string rule rbacv1.PolicyRule - requestsToExpected map[authorizer.AttributesRecord]bool + requestsToExpected []*requestToTest }{ { name: "star verb, exact match other", rule: rbacv1helpers.NewRule("*").Groups("group1").Resources("resource1").RuleOrDie(), - requestsToExpected: map[authorizer.AttributesRecord]bool{ - resourceRequest("verb1").Group("group1").Resource("resource1").New(): true, - resourceRequest("verb1").Group("group2").Resource("resource1").New(): false, - resourceRequest("verb1").Group("group1").Resource("resource2").New(): false, - resourceRequest("verb1").Group("group2").Resource("resource2").New(): false, - resourceRequest("verb2").Group("group1").Resource("resource1").New(): true, - resourceRequest("verb2").Group("group2").Resource("resource1").New(): false, - resourceRequest("verb2").Group("group1").Resource("resource2").New(): false, - resourceRequest("verb2").Group("group2").Resource("resource2").New(): false, + requestsToExpected: []*requestToTest{ + {resourceRequest("verb1").Group("group1").Resource("resource1").New(), true}, + {resourceRequest("verb1").Group("group2").Resource("resource1").New(), false}, + {resourceRequest("verb1").Group("group1").Resource("resource2").New(), false}, + {resourceRequest("verb1").Group("group2").Resource("resource2").New(), false}, + {resourceRequest("verb2").Group("group1").Resource("resource1").New(), true}, + {resourceRequest("verb2").Group("group2").Resource("resource1").New(), false}, + {resourceRequest("verb2").Group("group1").Resource("resource2").New(), false}, + {resourceRequest("verb2").Group("group2").Resource("resource2").New(), false}, }, }, { name: "star group, exact match other", rule: rbacv1helpers.NewRule("verb1").Groups("*").Resources("resource1").RuleOrDie(), - requestsToExpected: map[authorizer.AttributesRecord]bool{ - resourceRequest("verb1").Group("group1").Resource("resource1").New(): true, - resourceRequest("verb1").Group("group2").Resource("resource1").New(): true, - resourceRequest("verb1").Group("group1").Resource("resource2").New(): false, - resourceRequest("verb1").Group("group2").Resource("resource2").New(): false, - resourceRequest("verb2").Group("group1").Resource("resource1").New(): false, - resourceRequest("verb2").Group("group2").Resource("resource1").New(): false, - resourceRequest("verb2").Group("group1").Resource("resource2").New(): false, - resourceRequest("verb2").Group("group2").Resource("resource2").New(): false, + requestsToExpected: []*requestToTest{ + {resourceRequest("verb1").Group("group1").Resource("resource1").New(), true}, + {resourceRequest("verb1").Group("group2").Resource("resource1").New(), true}, + {resourceRequest("verb1").Group("group1").Resource("resource2").New(), false}, + {resourceRequest("verb1").Group("group2").Resource("resource2").New(), false}, + {resourceRequest("verb2").Group("group1").Resource("resource1").New(), false}, + {resourceRequest("verb2").Group("group2").Resource("resource1").New(), false}, + {resourceRequest("verb2").Group("group1").Resource("resource2").New(), false}, + {resourceRequest("verb2").Group("group2").Resource("resource2").New(), false}, }, }, { name: "star resource, exact match other", rule: rbacv1helpers.NewRule("verb1").Groups("group1").Resources("*").RuleOrDie(), - requestsToExpected: map[authorizer.AttributesRecord]bool{ - resourceRequest("verb1").Group("group1").Resource("resource1").New(): true, - resourceRequest("verb1").Group("group2").Resource("resource1").New(): false, - resourceRequest("verb1").Group("group1").Resource("resource2").New(): true, - resourceRequest("verb1").Group("group2").Resource("resource2").New(): false, - resourceRequest("verb2").Group("group1").Resource("resource1").New(): false, - resourceRequest("verb2").Group("group2").Resource("resource1").New(): false, - resourceRequest("verb2").Group("group1").Resource("resource2").New(): false, - resourceRequest("verb2").Group("group2").Resource("resource2").New(): false, + requestsToExpected: []*requestToTest{ + {resourceRequest("verb1").Group("group1").Resource("resource1").New(), true}, + {resourceRequest("verb1").Group("group2").Resource("resource1").New(), false}, + {resourceRequest("verb1").Group("group1").Resource("resource2").New(), true}, + {resourceRequest("verb1").Group("group2").Resource("resource2").New(), false}, + {resourceRequest("verb2").Group("group1").Resource("resource1").New(), false}, + {resourceRequest("verb2").Group("group2").Resource("resource1").New(), false}, + {resourceRequest("verb2").Group("group1").Resource("resource2").New(), false}, + {resourceRequest("verb2").Group("group2").Resource("resource2").New(), false}, }, }, { name: "tuple expansion", rule: rbacv1helpers.NewRule("verb1", "verb2").Groups("group1", "group2").Resources("resource1", "resource2").RuleOrDie(), - requestsToExpected: map[authorizer.AttributesRecord]bool{ - resourceRequest("verb1").Group("group1").Resource("resource1").New(): true, - resourceRequest("verb1").Group("group2").Resource("resource1").New(): true, - resourceRequest("verb1").Group("group1").Resource("resource2").New(): true, - resourceRequest("verb1").Group("group2").Resource("resource2").New(): true, - resourceRequest("verb2").Group("group1").Resource("resource1").New(): true, - resourceRequest("verb2").Group("group2").Resource("resource1").New(): true, - resourceRequest("verb2").Group("group1").Resource("resource2").New(): true, - resourceRequest("verb2").Group("group2").Resource("resource2").New(): true, + requestsToExpected: []*requestToTest{ + {resourceRequest("verb1").Group("group1").Resource("resource1").New(), true}, + {resourceRequest("verb1").Group("group2").Resource("resource1").New(), true}, + {resourceRequest("verb1").Group("group1").Resource("resource2").New(), true}, + {resourceRequest("verb1").Group("group2").Resource("resource2").New(), true}, + {resourceRequest("verb2").Group("group1").Resource("resource1").New(), true}, + {resourceRequest("verb2").Group("group2").Resource("resource1").New(), true}, + {resourceRequest("verb2").Group("group1").Resource("resource2").New(), true}, + {resourceRequest("verb2").Group("group2").Resource("resource2").New(), true}, }, }, { name: "subresource expansion", rule: rbacv1helpers.NewRule("*").Groups("*").Resources("resource1/subresource1").RuleOrDie(), - requestsToExpected: map[authorizer.AttributesRecord]bool{ - resourceRequest("verb1").Group("group1").Resource("resource1").Subresource("subresource1").New(): true, - resourceRequest("verb1").Group("group2").Resource("resource1").Subresource("subresource2").New(): false, - resourceRequest("verb1").Group("group1").Resource("resource2").Subresource("subresource1").New(): false, - resourceRequest("verb1").Group("group2").Resource("resource2").Subresource("subresource1").New(): false, - resourceRequest("verb2").Group("group1").Resource("resource1").Subresource("subresource1").New(): true, - resourceRequest("verb2").Group("group2").Resource("resource1").Subresource("subresource2").New(): false, - resourceRequest("verb2").Group("group1").Resource("resource2").Subresource("subresource1").New(): false, - resourceRequest("verb2").Group("group2").Resource("resource2").Subresource("subresource1").New(): false, + requestsToExpected: []*requestToTest{ + {resourceRequest("verb1").Group("group1").Resource("resource1").Subresource("subresource1").New(), true}, + {resourceRequest("verb1").Group("group2").Resource("resource1").Subresource("subresource2").New(), false}, + {resourceRequest("verb1").Group("group1").Resource("resource2").Subresource("subresource1").New(), false}, + {resourceRequest("verb1").Group("group2").Resource("resource2").Subresource("subresource1").New(), false}, + {resourceRequest("verb2").Group("group1").Resource("resource1").Subresource("subresource1").New(), true}, + {resourceRequest("verb2").Group("group2").Resource("resource1").Subresource("subresource2").New(), false}, + {resourceRequest("verb2").Group("group1").Resource("resource2").Subresource("subresource1").New(), false}, + {resourceRequest("verb2").Group("group2").Resource("resource2").Subresource("subresource1").New(), false}, }, }, { name: "star nonresource, exact match other", rule: rbacv1helpers.NewRule("verb1").URLs("*").RuleOrDie(), - requestsToExpected: map[authorizer.AttributesRecord]bool{ - nonresourceRequest("verb1").URL("/foo").New(): true, - nonresourceRequest("verb1").URL("/foo/bar").New(): true, - nonresourceRequest("verb1").URL("/foo/baz").New(): true, - nonresourceRequest("verb1").URL("/foo/bar/one").New(): true, - nonresourceRequest("verb1").URL("/foo/baz/one").New(): true, - nonresourceRequest("verb2").URL("/foo").New(): false, - nonresourceRequest("verb2").URL("/foo/bar").New(): false, - nonresourceRequest("verb2").URL("/foo/baz").New(): false, - nonresourceRequest("verb2").URL("/foo/bar/one").New(): false, - nonresourceRequest("verb2").URL("/foo/baz/one").New(): false, + requestsToExpected: []*requestToTest{ + {nonresourceRequest("verb1").URL("/foo").New(), true}, + {nonresourceRequest("verb1").URL("/foo/bar").New(), true}, + {nonresourceRequest("verb1").URL("/foo/baz").New(), true}, + {nonresourceRequest("verb1").URL("/foo/bar/one").New(), true}, + {nonresourceRequest("verb1").URL("/foo/baz/one").New(), true}, + {nonresourceRequest("verb2").URL("/foo").New(), false}, + {nonresourceRequest("verb2").URL("/foo/bar").New(), false}, + {nonresourceRequest("verb2").URL("/foo/baz").New(), false}, + {nonresourceRequest("verb2").URL("/foo/bar/one").New(), false}, + {nonresourceRequest("verb2").URL("/foo/baz/one").New(), false}, }, }, { name: "star nonresource subpath", rule: rbacv1helpers.NewRule("verb1").URLs("/foo/*").RuleOrDie(), - requestsToExpected: map[authorizer.AttributesRecord]bool{ - nonresourceRequest("verb1").URL("/foo").New(): false, - nonresourceRequest("verb1").URL("/foo/bar").New(): true, - nonresourceRequest("verb1").URL("/foo/baz").New(): true, - nonresourceRequest("verb1").URL("/foo/bar/one").New(): true, - nonresourceRequest("verb1").URL("/foo/baz/one").New(): true, - nonresourceRequest("verb1").URL("/notfoo").New(): false, - nonresourceRequest("verb1").URL("/notfoo/bar").New(): false, - nonresourceRequest("verb1").URL("/notfoo/baz").New(): false, - nonresourceRequest("verb1").URL("/notfoo/bar/one").New(): false, - nonresourceRequest("verb1").URL("/notfoo/baz/one").New(): false, + requestsToExpected: []*requestToTest{ + {nonresourceRequest("verb1").URL("/foo").New(), false}, + {nonresourceRequest("verb1").URL("/foo/bar").New(), true}, + {nonresourceRequest("verb1").URL("/foo/baz").New(), true}, + {nonresourceRequest("verb1").URL("/foo/bar/one").New(), true}, + {nonresourceRequest("verb1").URL("/foo/baz/one").New(), true}, + {nonresourceRequest("verb1").URL("/notfoo").New(), false}, + {nonresourceRequest("verb1").URL("/notfoo/bar").New(), false}, + {nonresourceRequest("verb1").URL("/notfoo/baz").New(), false}, + {nonresourceRequest("verb1").URL("/notfoo/bar/one").New(), false}, + {nonresourceRequest("verb1").URL("/notfoo/baz/one").New(), false}, }, }, { name: "star verb, exact nonresource", rule: rbacv1helpers.NewRule("*").URLs("/foo", "/foo/bar/one").RuleOrDie(), - requestsToExpected: map[authorizer.AttributesRecord]bool{ - nonresourceRequest("verb1").URL("/foo").New(): true, - nonresourceRequest("verb1").URL("/foo/bar").New(): false, - nonresourceRequest("verb1").URL("/foo/baz").New(): false, - nonresourceRequest("verb1").URL("/foo/bar/one").New(): true, - nonresourceRequest("verb1").URL("/foo/baz/one").New(): false, - nonresourceRequest("verb2").URL("/foo").New(): true, - nonresourceRequest("verb2").URL("/foo/bar").New(): false, - nonresourceRequest("verb2").URL("/foo/baz").New(): false, - nonresourceRequest("verb2").URL("/foo/bar/one").New(): true, - nonresourceRequest("verb2").URL("/foo/baz/one").New(): false, + requestsToExpected: []*requestToTest{ + {nonresourceRequest("verb1").URL("/foo").New(), true}, + {nonresourceRequest("verb1").URL("/foo/bar").New(), false}, + {nonresourceRequest("verb1").URL("/foo/baz").New(), false}, + {nonresourceRequest("verb1").URL("/foo/bar/one").New(), true}, + {nonresourceRequest("verb1").URL("/foo/baz/one").New(), false}, + {nonresourceRequest("verb2").URL("/foo").New(), true}, + {nonresourceRequest("verb2").URL("/foo/bar").New(), false}, + {nonresourceRequest("verb2").URL("/foo/baz").New(), false}, + {nonresourceRequest("verb2").URL("/foo/bar/one").New(), true}, + {nonresourceRequest("verb2").URL("/foo/baz/one").New(), false}, }, }, } for _, tc := range tests { - for request, expected := range tc.requestsToExpected { - if e, a := expected, RuleAllows(request, &tc.rule); e != a { - t.Errorf("%q: expected %v, got %v for %v", tc.name, e, a, request) + for _, requestToTest := range tc.requestsToExpected { + if e, a := requestToTest.expected, RuleAllows(requestToTest.request, &tc.rule); e != a { + t.Errorf("%q: expected %v, got %v for %v", tc.name, e, a, requestToTest.request) } } } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go index a0f709ad862..3eba5ba5417 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go @@ -32,6 +32,10 @@ import ( type LabelSelectorValidationOptions struct { // Allow invalid label value in selector AllowInvalidLabelValueInSelector bool + + // Allows an operator that is not interpretable to pass validation. This is useful for cases where a broader check + // can be performed, as in a *SubjectAccessReview + AllowUnknownOperatorInRequirement bool } // LabelSelectorHasInvalidLabelValue returns true if the given selector contains an invalid label value in a match expression. @@ -79,7 +83,9 @@ func ValidateLabelSelectorRequirement(sr metav1.LabelSelectorRequirement, opts L allErrs = append(allErrs, field.Forbidden(fldPath.Child("values"), "may not be specified when `operator` is 'Exists' or 'DoesNotExist'")) } default: - allErrs = append(allErrs, field.Invalid(fldPath.Child("operator"), sr.Operator, "not a valid selector operator")) + if !opts.AllowUnknownOperatorInRequirement { + allErrs = append(allErrs, field.Invalid(fldPath.Child("operator"), sr.Operator, "not a valid selector operator")) + } } allErrs = append(allErrs, ValidateLabelName(sr.Key, fldPath.Child("key"))...) if !opts.AllowInvalidLabelValueInSelector { @@ -113,6 +119,39 @@ func ValidateLabels(labels map[string]string, fldPath *field.Path) field.ErrorLi return allErrs } +// FieldSelectorValidationOptions is a struct that can be passed to ValidateFieldSelectorRequirement to record the validate options +type FieldSelectorValidationOptions struct { + // Allows an operator that is not interpretable to pass validation. This is useful for cases where a broader check + // can be performed, as in a *SubjectAccessReview + AllowUnknownOperatorInRequirement bool +} + +// ValidateLabelSelectorRequirement validates the requirement according to the opts and returns any validation errors. +func ValidateFieldSelectorRequirement(requirement metav1.FieldSelectorRequirement, opts FieldSelectorValidationOptions, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if len(requirement.Key) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("key"), "must be specified")) + } + + switch requirement.Operator { + case metav1.FieldSelectorOpIn, metav1.FieldSelectorOpNotIn: + if len(requirement.Values) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("values"), "must be specified when `operator` is 'In' or 'NotIn'")) + } + case metav1.FieldSelectorOpExists, metav1.FieldSelectorOpDoesNotExist: + if len(requirement.Values) > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("values"), "may not be specified when `operator` is 'Exists' or 'DoesNotExist'")) + } + default: + if !opts.AllowUnknownOperatorInRequirement { + allErrs = append(allErrs, field.Invalid(fldPath.Child("operator"), requirement.Operator, "not a valid selector operator")) + } + } + + return allErrs +} + func ValidateDeleteOptions(options *metav1.DeleteOptions) field.ErrorList { allErrs := field.ErrorList{} //lint:file-ignore SA1019 Keep validation for deprecated OrphanDependents option until it's being removed diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go index e9c6c161620..3296d4f61b2 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go @@ -470,7 +470,7 @@ func TestLabelSelectorMatchExpression(t *testing.T) { }} for index, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - allErrs := ValidateLabelSelector(testCase.labelSelector, LabelSelectorValidationOptions{false}, field.NewPath("labelSelector")) + allErrs := ValidateLabelSelector(testCase.labelSelector, LabelSelectorValidationOptions{AllowInvalidLabelValueInSelector: false}, field.NewPath("labelSelector")) if len(allErrs) != testCase.wantErrorNumber { t.Errorf("case[%d]: expected failure", index) } diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go index 5e601424051..9e22a005642 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go @@ -45,6 +45,19 @@ var ( // Requirements is AND of all requirements. type Requirements []Requirement +func (r Requirements) String() string { + var sb strings.Builder + + for i, requirement := range r { + if i > 0 { + sb.WriteString(", ") + } + sb.WriteString(requirement.String()) + } + + return sb.String() +} + // Selector represents a label selector. type Selector interface { // Matches returns true if this selector matches the given set of labels. @@ -285,6 +298,13 @@ func (r *Requirement) Values() sets.String { return ret } +// ValuesUnsorted returns a copy of requirement values as passed to NewRequirement without sorting. +func (r *Requirement) ValuesUnsorted() []string { + ret := make([]string, 0, len(r.strValues)) + ret = append(ret, r.strValues...) + return ret +} + // Equal checks the equality of requirement. func (r Requirement) Equal(x Requirement) bool { if r.key != x.key { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer.go index fbefd595e57..ac13dbeee37 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer.go @@ -22,6 +22,8 @@ import ( "sort" "strings" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" ) @@ -58,6 +60,8 @@ var _ authorizer.Attributes = (interface { GetAPIVersion() string IsResourceRequest() bool GetPath() string + GetFieldSelector() (fields.Requirements, error) + GetLabelSelector() (labels.Requirements, error) })(nil) // The user info accessors known to cache key construction. If this fails to compile, the cache @@ -72,16 +76,31 @@ var _ user.Info = (interface { // Authorize returns an authorization decision by delegating to another Authorizer. If an equivalent // check has already been performed, a cached result is returned. Not safe for concurrent use. func (ca *cachingAuthorizer) Authorize(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { - serializableAttributes := authorizer.AttributesRecord{ - Verb: a.GetVerb(), - Namespace: a.GetNamespace(), - APIGroup: a.GetAPIGroup(), - APIVersion: a.GetAPIVersion(), - Resource: a.GetResource(), - Subresource: a.GetSubresource(), - Name: a.GetName(), - ResourceRequest: a.IsResourceRequest(), - Path: a.GetPath(), + type SerializableAttributes struct { + authorizer.AttributesRecord + LabelSelector string + } + + serializableAttributes := SerializableAttributes{ + AttributesRecord: authorizer.AttributesRecord{ + Verb: a.GetVerb(), + Namespace: a.GetNamespace(), + APIGroup: a.GetAPIGroup(), + APIVersion: a.GetAPIVersion(), + Resource: a.GetResource(), + Subresource: a.GetSubresource(), + Name: a.GetName(), + ResourceRequest: a.IsResourceRequest(), + Path: a.GetPath(), + }, + } + // in the error case, we won't honor this field selector, so the cache doesn't need it. + if fieldSelector, err := a.GetFieldSelector(); len(fieldSelector) > 0 { + serializableAttributes.FieldSelectorRequirements, serializableAttributes.FieldSelectorParsingErr = fieldSelector, err + } + if labelSelector, _ := a.GetLabelSelector(); len(labelSelector) > 0 { + // the labels requirements have private elements so those don't help us serialize to a unique key + serializableAttributes.LabelSelector = labelSelector.String() } if u := a.GetUser(); u != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer_test.go index da7f219fa1c..5831cc3febf 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/caching_authorizer_test.go @@ -18,14 +18,31 @@ package validating import ( "context" + "errors" "fmt" "testing" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" ) +func mustParseLabelSelector(str string) labels.Requirements { + ret, err := labels.Parse(str) + if err != nil { + panic(err) + } + retRequirements, _ /*selectable*/ := ret.Requirements() + return retRequirements +} + func TestCachingAuthorizer(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true) + type result struct { decision authorizer.Decision reason string @@ -216,6 +233,261 @@ func TestCachingAuthorizer(t *testing.T) { }, }, }, + { + name: "honor good field selector", + calls: []invocation{ + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + FieldSelectorRequirements: fields.ParseSelectorOrDie("foo=bar").Requirements(), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason 2", + error: fmt.Errorf("test error 2"), + }, + }, + { + // now this should be cached + attributes: authorizer.AttributesRecord{ + Name: "test name", + FieldSelectorRequirements: fields.ParseSelectorOrDie("foo=bar").Requirements(), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, + backend: []result{ + { + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + { + decision: authorizer.DecisionAllow, + reason: "test reason 2", + error: fmt.Errorf("test error 2"), + }, + }, + }, + { + name: "ignore malformed field selector first", + calls: []invocation{ + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + FieldSelectorParsingErr: errors.New("malformed"), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + { + // notice that this does not have the malformed field selector. + // it should use the cached result + attributes: authorizer.AttributesRecord{ + Name: "test name", + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, + backend: []result{ + { + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, + { + name: "ignore malformed field selector second", + calls: []invocation{ + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + { + // this should use the broader cached value because the selector will be ignored + attributes: authorizer.AttributesRecord{ + Name: "test name", + FieldSelectorParsingErr: errors.New("malformed"), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, + backend: []result{ + { + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, + + { + name: "honor good label selector", + calls: []invocation{ + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + LabelSelectorRequirements: mustParseLabelSelector("foo=bar"), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason 2", + error: fmt.Errorf("test error 2"), + }, + }, + { + // now this should be cached + attributes: authorizer.AttributesRecord{ + Name: "test name", + LabelSelectorRequirements: mustParseLabelSelector("foo=bar"), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + LabelSelectorRequirements: mustParseLabelSelector("diff=zero"), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason 3", + error: fmt.Errorf("test error 3"), + }, + }, + }, + backend: []result{ + { + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + { + decision: authorizer.DecisionAllow, + reason: "test reason 2", + error: fmt.Errorf("test error 2"), + }, + { + decision: authorizer.DecisionAllow, + reason: "test reason 3", + error: fmt.Errorf("test error 3"), + }, + }, + }, + { + name: "ignore malformed label selector first", + calls: []invocation{ + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + LabelSelectorParsingErr: errors.New("malformed mess"), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + { + // notice that this does not have the malformed field selector. + // it should use the cached result + attributes: authorizer.AttributesRecord{ + Name: "test name", + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, + backend: []result{ + { + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, + { + name: "ignore malformed label selector second", + calls: []invocation{ + { + attributes: authorizer.AttributesRecord{ + Name: "test name", + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + { + // this should use the broader cached value because the selector will be ignored + attributes: authorizer.AttributesRecord{ + Name: "test name", + LabelSelectorParsingErr: errors.New("malformed mess"), + }, + expected: result{ + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, + backend: []result{ + { + decision: authorizer.DecisionAllow, + reason: "test reason", + error: fmt.Errorf("test error"), + }, + }, + }, } { t.Run(tc.name, func(t *testing.T) { var misses int diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/authorizer/interfaces.go b/staging/src/k8s.io/apiserver/pkg/authorization/authorizer/interfaces.go index 8261c5b5830..d39deb17eb2 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/authorizer/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/authorizer/interfaces.go @@ -20,6 +20,8 @@ import ( "context" "net/http" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/authentication/user" ) @@ -62,6 +64,16 @@ type Attributes interface { // GetPath returns the path of the request GetPath() string + + // ParseFieldSelector is lazy, thread-safe, and stores the parsed result and error. + // It returns an error if the field selector cannot be parsed. + // The returned requirements must be treated as readonly and not modified. + GetFieldSelector() (fields.Requirements, error) + + // ParseLabelSelector is lazy, thread-safe, and stores the parsed result and error. + // It returns an error if the label selector cannot be parsed. + // The returned requirements must be treated as readonly and not modified. + GetLabelSelector() (labels.Requirements, error) } // Authorizer makes an authorization decision based on information gained by making @@ -100,6 +112,11 @@ type AttributesRecord struct { Name string ResourceRequest bool Path string + + FieldSelectorRequirements fields.Requirements + FieldSelectorParsingErr error + LabelSelectorRequirements labels.Requirements + LabelSelectorParsingErr error } func (a AttributesRecord) GetUser() user.Info { @@ -146,6 +163,14 @@ func (a AttributesRecord) GetPath() string { return a.Path } +func (a AttributesRecord) GetFieldSelector() (fields.Requirements, error) { + return a.FieldSelectorRequirements, a.FieldSelectorParsingErr +} + +func (a AttributesRecord) GetLabelSelector() (labels.Requirements, error) { + return a.LabelSelectorRequirements, a.LabelSelectorParsingErr +} + type Decision int const ( diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go index e102a1e3281..eec02e5722f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go @@ -22,6 +22,11 @@ import ( "net/http" "time" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/klog/v2" "k8s.io/apimachinery/pkg/runtime" @@ -117,5 +122,31 @@ func GetAuthorizerAttributes(ctx context.Context) (authorizer.Attributes, error) attribs.Namespace = requestInfo.Namespace attribs.Name = requestInfo.Name + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + // parsing here makes it easy to keep the AttributesRecord type value-only and avoids any mutex copies when + // doing shallow copies in other steps. + if len(requestInfo.FieldSelector) > 0 { + fieldSelector, err := fields.ParseSelector(requestInfo.FieldSelector) + if err != nil { + attribs.FieldSelectorRequirements, attribs.FieldSelectorParsingErr = nil, err + } else { + if requirements := fieldSelector.Requirements(); len(requirements) > 0 { + attribs.FieldSelectorRequirements, attribs.FieldSelectorParsingErr = fieldSelector.Requirements(), nil + } + } + } + + if len(requestInfo.LabelSelector) > 0 { + labelSelector, err := labels.Parse(requestInfo.LabelSelector) + if err != nil { + attribs.LabelSelectorRequirements, attribs.LabelSelectorParsingErr = nil, err + } else { + if requirements, _ /*selectable*/ := labelSelector.Requirements(); len(requirements) > 0 { + attribs.LabelSelectorRequirements, attribs.LabelSelectorParsingErr = requirements, nil + } + } + } + } + return &attribs, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization_test.go index 5840d08b063..deef9054b18 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization_test.go @@ -19,6 +19,12 @@ package filters import ( "context" "errors" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" "net/http" "net/http/httptest" "reflect" @@ -34,10 +40,16 @@ import ( ) func TestGetAuthorizerAttributes(t *testing.T) { + basicLabelRequirement, err := labels.NewRequirement("foo", selection.DoubleEquals, []string{"bar"}) + if err != nil { + t.Fatal(err) + } + testcases := map[string]struct { - Verb string - Path string - ExpectedAttributes *authorizer.AttributesRecord + Verb string + Path string + ExpectedAttributes *authorizer.AttributesRecord + EnableAuthorizationSelector bool }{ "non-resource root": { Verb: "POST", @@ -102,26 +114,122 @@ func TestGetAuthorizerAttributes(t *testing.T) { Resource: "jobs", }, }, + "disabled, ignore good field selector": { + Verb: "GET", + Path: "/apis/batch/v1/namespaces/myns/jobs?fieldSelector%=foo%3Dbar", + ExpectedAttributes: &authorizer.AttributesRecord{ + Verb: "list", + Path: "/apis/batch/v1/namespaces/myns/jobs", + ResourceRequest: true, + APIGroup: batch.GroupName, + APIVersion: "v1", + Namespace: "myns", + Resource: "jobs", + }, + }, + "enabled, good field selector": { + Verb: "GET", + Path: "/apis/batch/v1/namespaces/myns/jobs?fieldSelector=foo%3D%3Dbar", + ExpectedAttributes: &authorizer.AttributesRecord{ + Verb: "list", + Path: "/apis/batch/v1/namespaces/myns/jobs", + ResourceRequest: true, + APIGroup: batch.GroupName, + APIVersion: "v1", + Namespace: "myns", + Resource: "jobs", + FieldSelectorRequirements: fields.Requirements{ + fields.OneTermEqualSelector("foo", "bar").Requirements()[0], + }, + }, + EnableAuthorizationSelector: true, + }, + "enabled, bad field selector": { + Verb: "GET", + Path: "/apis/batch/v1/namespaces/myns/jobs?fieldSelector=%2Abar", + ExpectedAttributes: &authorizer.AttributesRecord{ + Verb: "list", + Path: "/apis/batch/v1/namespaces/myns/jobs", + ResourceRequest: true, + APIGroup: batch.GroupName, + APIVersion: "v1", + Namespace: "myns", + Resource: "jobs", + FieldSelectorParsingErr: errors.New("invalid selector: '*bar'; can't understand '*bar'"), + }, + EnableAuthorizationSelector: true, + }, + "disabled, ignore good label selector": { + Verb: "GET", + Path: "/apis/batch/v1/namespaces/myns/jobs?labelSelector%=foo%3Dbar", + ExpectedAttributes: &authorizer.AttributesRecord{ + Verb: "list", + Path: "/apis/batch/v1/namespaces/myns/jobs", + ResourceRequest: true, + APIGroup: batch.GroupName, + APIVersion: "v1", + Namespace: "myns", + Resource: "jobs", + }, + }, + "enabled, good label selector": { + Verb: "GET", + Path: "/apis/batch/v1/namespaces/myns/jobs?labelSelector=foo%3D%3Dbar", + ExpectedAttributes: &authorizer.AttributesRecord{ + Verb: "list", + Path: "/apis/batch/v1/namespaces/myns/jobs", + ResourceRequest: true, + APIGroup: batch.GroupName, + APIVersion: "v1", + Namespace: "myns", + Resource: "jobs", + LabelSelectorRequirements: labels.Requirements{ + *basicLabelRequirement, + }, + }, + EnableAuthorizationSelector: true, + }, + "enabled, bad label selector": { + Verb: "GET", + Path: "/apis/batch/v1/namespaces/myns/jobs?labelSelector=%2Abar", + ExpectedAttributes: &authorizer.AttributesRecord{ + Verb: "list", + Path: "/apis/batch/v1/namespaces/myns/jobs", + ResourceRequest: true, + APIGroup: batch.GroupName, + APIVersion: "v1", + Namespace: "myns", + Resource: "jobs", + LabelSelectorParsingErr: errors.New("unable to parse requirement: : Invalid value: \"*bar\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"), + }, + EnableAuthorizationSelector: true, + }, } for k, tc := range testcases { - req, _ := http.NewRequest(tc.Verb, tc.Path, nil) - req.RemoteAddr = "127.0.0.1" + t.Run(k, func(t *testing.T) { + if tc.EnableAuthorizationSelector { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true) + } - var attribs authorizer.Attributes - var err error - var handler http.Handler = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - ctx := req.Context() - attribs, err = GetAuthorizerAttributes(ctx) + req, _ := http.NewRequest(tc.Verb, tc.Path, nil) + req.RemoteAddr = "127.0.0.1" + + var attribs authorizer.Attributes + var err error + var handler http.Handler = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + ctx := req.Context() + attribs, err = GetAuthorizerAttributes(ctx) + }) + handler = WithRequestInfo(handler, newTestRequestInfoResolver()) + handler.ServeHTTP(httptest.NewRecorder(), req) + + if err != nil { + t.Errorf("%s: unexpected error: %v", k, err) + } else if !reflect.DeepEqual(attribs, tc.ExpectedAttributes) { + t.Errorf("%s: expected\n\t%#v\ngot\n\t%#v", k, tc.ExpectedAttributes, attribs) + } }) - handler = WithRequestInfo(handler, newTestRequestInfoResolver()) - handler.ServeHTTP(httptest.NewRecorder(), req) - - if err != nil { - t.Errorf("%s: unexpected error: %v", k, err) - } else if !reflect.DeepEqual(attribs, tc.ExpectedAttributes) { - t.Errorf("%s: expected\n\t%#v\ngot\n\t%#v", k, tc.ExpectedAttributes, attribs) - } } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo.go b/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo.go index 2558494bd9a..808943d1615 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo.go @@ -27,6 +27,8 @@ import ( metainternalversionscheme "k8s.io/apimachinery/pkg/apis/meta/internalversion/scheme" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" ) @@ -62,6 +64,13 @@ type RequestInfo struct { Name string // Parts are the path parts for the request, always starting with /{resource}/{name} Parts []string + + // FieldSelector contains the unparsed field selector from a request. It is only present if the apiserver + // honors field selectors for the verb this request is associated with. + FieldSelector string + // LabelSelector contains the unparsed field selector from a request. It is only present if the apiserver + // honors field selectors for the verb this request is associated with. + LabelSelector string } // specialVerbs contains just strings which are used in REST paths for special actions that don't fall under the normal @@ -77,6 +86,9 @@ var specialVerbsNoSubresources = sets.NewString("proxy") // this list allows the parser to distinguish between a namespace subresource, and a namespaced resource var namespaceSubresources = sets.NewString("status", "finalize") +// verbsWithSelectors is the list of verbs which support fieldSelector and labelSelector parameters +var verbsWithSelectors = sets.NewString("list", "watch", "deletecollection") + // NamespaceSubResourcesForTest exports namespaceSubresources for testing in pkg/controlplane/master_test.go, so we never drift var NamespaceSubResourcesForTest = sets.NewString(namespaceSubresources.List()...) @@ -151,6 +163,7 @@ func (r *RequestInfoFactory) NewRequestInfo(req *http.Request) (*RequestInfo, er currentParts = currentParts[1:] // handle input of form /{specialVerb}/* + verbViaPathPrefix := false if specialVerbs.Has(currentParts[0]) { if len(currentParts) < 2 { return &requestInfo, fmt.Errorf("unable to determine kind and namespace from url, %v", req.URL) @@ -158,6 +171,7 @@ func (r *RequestInfoFactory) NewRequestInfo(req *http.Request) (*RequestInfo, er requestInfo.Verb = currentParts[0] currentParts = currentParts[1:] + verbViaPathPrefix = true } else { switch req.Method { @@ -238,11 +252,28 @@ func (r *RequestInfoFactory) NewRequestInfo(req *http.Request) (*RequestInfo, er } } } + // if there's no name on the request and we thought it was a delete before, then the actual verb is deletecollection if len(requestInfo.Name) == 0 && requestInfo.Verb == "delete" { requestInfo.Verb = "deletecollection" } + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + // Don't support selector authorization on requests that used the deprecated verb-via-path mechanism, since they don't support selectors consistently. + // There are multi-object and single-object watch endpoints, and only the multi-object one supports selectors. + if !verbViaPathPrefix && verbsWithSelectors.Has(requestInfo.Verb) { + // interestingly these are parsed above, but the current structure there means that if one (or anything) in the + // listOptions fails to decode, the field and label selectors are lost. + // therefore, do the straight query param read here. + if vals := req.URL.Query()["fieldSelector"]; len(vals) > 0 { + requestInfo.FieldSelector = vals[0] + } + if vals := req.URL.Query()["labelSelector"]; len(vals) > 0 { + requestInfo.LabelSelector = vals[0] + } + } + } + return &requestInfo, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo_test.go index a5c521e5b4d..552957089a0 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo_test.go @@ -25,6 +25,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" ) func TestGetAPIRequestInfo(t *testing.T) { @@ -190,64 +193,129 @@ func newTestRequestInfoResolver() *RequestInfoFactory { } } -func TestFieldSelectorParsing(t *testing.T) { +func TestSelectorParsing(t *testing.T) { tests := []struct { - name string - url string - expectedName string - expectedErr error - expectedVerb string + name string + method string + url string + expectedName string + expectedErr error + expectedVerb string + expectedFieldSelector string + expectedLabelSelector string }{ { - name: "no selector", - url: "/apis/group/version/resource", - expectedVerb: "list", + name: "no selector", + method: "GET", + url: "/apis/group/version/resource", + expectedVerb: "list", + expectedFieldSelector: "", }, { - name: "metadata.name selector", - url: "/apis/group/version/resource?fieldSelector=metadata.name=name1", - expectedName: "name1", - expectedVerb: "list", + name: "metadata.name selector", + method: "GET", + url: "/apis/group/version/resource?fieldSelector=metadata.name=name1", + expectedName: "name1", + expectedVerb: "list", + expectedFieldSelector: "metadata.name=name1", }, { - name: "metadata.name selector with watch", - url: "/apis/group/version/resource?watch=true&fieldSelector=metadata.name=name1", - expectedName: "name1", - expectedVerb: "watch", + name: "metadata.name selector with watch", + method: "GET", + url: "/apis/group/version/resource?watch=true&fieldSelector=metadata.name=name1", + expectedName: "name1", + expectedVerb: "watch", + expectedFieldSelector: "metadata.name=name1", }, { - name: "random selector", - url: "/apis/group/version/resource?fieldSelector=foo=bar", - expectedName: "", - expectedVerb: "list", + name: "random selector", + method: "GET", + url: "/apis/group/version/resource?fieldSelector=foo=bar&labelSelector=baz=qux", + expectedName: "", + expectedVerb: "list", + expectedFieldSelector: "foo=bar", + expectedLabelSelector: "baz=qux", }, { - name: "invalid selector with metadata.name", - url: "/apis/group/version/resource?fieldSelector=metadata.name=name1,foo", - expectedName: "", - expectedErr: fmt.Errorf("invalid selector"), - expectedVerb: "list", + name: "invalid selector with metadata.name", + method: "GET", + url: "/apis/group/version/resource?fieldSelector=metadata.name=name1,foo", + expectedName: "", + expectedErr: fmt.Errorf("invalid selector"), + expectedVerb: "list", + expectedFieldSelector: "metadata.name=name1,foo", }, { - name: "invalid selector with metadata.name with watch", - url: "/apis/group/version/resource?fieldSelector=metadata.name=name1,foo&watch=true", - expectedName: "", - expectedErr: fmt.Errorf("invalid selector"), - expectedVerb: "watch", + name: "invalid selector with metadata.name with watch", + method: "GET", + url: "/apis/group/version/resource?fieldSelector=metadata.name=name1,foo&watch=true", + expectedName: "", + expectedErr: fmt.Errorf("invalid selector"), + expectedVerb: "watch", + expectedFieldSelector: "metadata.name=name1,foo", }, { - name: "invalid selector with metadata.name with watch false", - url: "/apis/group/version/resource?fieldSelector=metadata.name=name1,foo&watch=false", - expectedName: "", - expectedErr: fmt.Errorf("invalid selector"), - expectedVerb: "list", + name: "invalid selector with metadata.name with watch false", + method: "GET", + url: "/apis/group/version/resource?fieldSelector=metadata.name=name1,foo&watch=false", + expectedName: "", + expectedErr: fmt.Errorf("invalid selector"), + expectedVerb: "list", + expectedFieldSelector: "metadata.name=name1,foo", + }, + { + name: "selector on deletecollection is honored", + method: "DELETE", + url: "/apis/group/version/resource?fieldSelector=foo=bar&labelSelector=baz=qux", + expectedName: "", + expectedVerb: "deletecollection", + expectedFieldSelector: "foo=bar", + expectedLabelSelector: "baz=qux", + }, + { + name: "selector on repeated param matches parsed param", + method: "GET", + url: "/apis/group/version/resource?fieldSelector=metadata.name=foo&fieldSelector=metadata.name=bar&labelSelector=foo=bar&labelSelector=foo=baz", + expectedName: "foo", + expectedVerb: "list", + expectedFieldSelector: "metadata.name=foo", + expectedLabelSelector: "foo=bar", + }, + { + name: "selector on other verb is ignored", + method: "GET", + url: "/apis/group/version/resource/name?fieldSelector=foo=bar&labelSelector=foo=bar", + expectedName: "name", + expectedVerb: "get", + expectedFieldSelector: "", + expectedLabelSelector: "", + }, + { + name: "selector on deprecated root type watch is not parsed", + method: "GET", + url: "/apis/group/version/watch/resource?fieldSelector=metadata.name=foo&labelSelector=foo=bar", + expectedName: "", + expectedVerb: "watch", + expectedFieldSelector: "", + expectedLabelSelector: "", + }, + { + name: "selector on deprecated root item watch is not parsed", + method: "GET", + url: "/apis/group/version/watch/resource/name?fieldSelector=metadata.name=foo&labelSelector=foo=bar", + expectedName: "name", + expectedVerb: "watch", + expectedFieldSelector: "", + expectedLabelSelector: "", }, } resolver := newTestRequestInfoResolver() + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true) + for _, tc := range tests { - req, _ := http.NewRequest("GET", tc.url, nil) + req, _ := http.NewRequest(tc.method, tc.url, nil) apiRequestInfo, err := resolver.NewRequestInfo(req) if err != nil { @@ -261,5 +329,11 @@ func TestFieldSelectorParsing(t *testing.T) { if e, a := tc.expectedVerb, apiRequestInfo.Verb; e != a { t.Errorf("%s: expected verb %v, actual %v", tc.name, e, a) } + if e, a := tc.expectedFieldSelector, apiRequestInfo.FieldSelector; e != a { + t.Errorf("%s: expected fieldSelector %v, actual %v", tc.name, e, a) + } + if e, a := tc.expectedLabelSelector, apiRequestInfo.LabelSelector; e != a { + t.Errorf("%s: expected labelSelector %v, actual %v", tc.name, e, a) + } } } diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index 200838247e9..6b09e9b776a 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -95,6 +95,13 @@ const ( // Enables serving watch requests in separate goroutines. APIServingWithRoutine featuregate.Feature = "APIServingWithRoutine" + // owner: @deads2k + // kep: https://kep.k8s.io/4601 + // alpha: v1.31 + // + // Allows authorization to use field and label selectors. + AuthorizeWithSelectors featuregate.Feature = "AuthorizeWithSelectors" + // owner: @cici37 @jpbetz // kep: http://kep.k8s.io/3488 // alpha: v1.26 @@ -358,6 +365,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS APIServingWithRoutine: {Default: true, PreRelease: featuregate.Beta}, + AuthorizeWithSelectors: {Default: false, PreRelease: featuregate.Alpha}, + ValidatingAdmissionPolicy: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 CustomResourceValidationExpressions: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.31 diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/round_trip_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/round_trip_test.go index 7b5ea4cf220..73f6adf006c 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/round_trip_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/round_trip_test.go @@ -81,13 +81,15 @@ func v1beta1ResourceAttributesToV1ResourceAttributes(in *authorizationv1beta1.Re return nil } return &authorizationv1.ResourceAttributes{ - Namespace: in.Namespace, - Verb: in.Verb, - Group: in.Group, - Version: in.Version, - Resource: in.Resource, - Subresource: in.Subresource, - Name: in.Name, + Namespace: in.Namespace, + Verb: in.Verb, + Group: in.Group, + Version: in.Version, + Resource: in.Resource, + Subresource: in.Subresource, + Name: in.Name, + FieldSelector: in.FieldSelector, + LabelSelector: in.LabelSelector, } } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go index 589899d7234..ebc4949d920 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go @@ -32,6 +32,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/cache" utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/wait" @@ -40,7 +41,7 @@ import ( "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" authorizationcel "k8s.io/apiserver/pkg/authorization/cel" - "k8s.io/apiserver/pkg/features" + genericfeatures "k8s.io/apiserver/pkg/features" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/apiserver/plugin/pkg/authorizer/webhook/metrics" @@ -196,15 +197,7 @@ func (w *WebhookAuthorizer) Authorize(ctx context.Context, attr authorizer.Attri } if attr.IsResourceRequest() { - r.Spec.ResourceAttributes = &authorizationv1.ResourceAttributes{ - Namespace: attr.GetNamespace(), - Verb: attr.GetVerb(), - Group: attr.GetAPIGroup(), - Version: attr.GetAPIVersion(), - Resource: attr.GetResource(), - Subresource: attr.GetSubresource(), - Name: attr.GetName(), - } + r.Spec.ResourceAttributes = resourceAttributesFrom(attr) } else { r.Spec.NonResourceAttributes = &authorizationv1.NonResourceAttributes{ Path: attr.GetPath(), @@ -212,7 +205,7 @@ func (w *WebhookAuthorizer) Authorize(ctx context.Context, attr authorizer.Attri } } // skipping match when feature is not enabled - if utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthorizationConfiguration) { + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.StructuredAuthorizationConfiguration) { // Process Match Conditions before calling the webhook matches, err := w.match(ctx, r) // If at least one matchCondition evaluates to an error (but none are FALSE): @@ -305,6 +298,109 @@ func (w *WebhookAuthorizer) Authorize(ctx context.Context, attr authorizer.Attri } +func resourceAttributesFrom(attr authorizer.Attributes) *authorizationv1.ResourceAttributes { + ret := &authorizationv1.ResourceAttributes{ + Namespace: attr.GetNamespace(), + Verb: attr.GetVerb(), + Group: attr.GetAPIGroup(), + Version: attr.GetAPIVersion(), + Resource: attr.GetResource(), + Subresource: attr.GetSubresource(), + Name: attr.GetName(), + } + + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + // If we are able to get any requirements while parsing selectors, use them, even if there's an error. + // This is because selectors only narrow, so if a subset of selector requirements are available, the request can be allowed. + if selectorRequirements, _ := fieldSelectorToAuthorizationAPI(attr); len(selectorRequirements) > 0 { + ret.FieldSelector = &authorizationv1.FieldSelectorAttributes{ + Requirements: selectorRequirements, + } + } + + if selectorRequirements, _ := labelSelectorToAuthorizationAPI(attr); len(selectorRequirements) > 0 { + ret.LabelSelector = &authorizationv1.LabelSelectorAttributes{ + Requirements: selectorRequirements, + } + } + } + + return ret +} + +func fieldSelectorToAuthorizationAPI(attr authorizer.Attributes) ([]metav1.FieldSelectorRequirement, error) { + requirements, getFieldSelectorErr := attr.GetFieldSelector() + if len(requirements) == 0 { + return nil, getFieldSelectorErr + } + + retRequirements := []metav1.FieldSelectorRequirement{} + for _, requirement := range requirements { + retRequirement := metav1.FieldSelectorRequirement{} + switch { + case requirement.Operator == selection.Equals || requirement.Operator == selection.DoubleEquals || requirement.Operator == selection.In: + retRequirement.Operator = metav1.FieldSelectorOpIn + retRequirement.Key = requirement.Field + retRequirement.Values = []string{requirement.Value} + case requirement.Operator == selection.NotEquals || requirement.Operator == selection.NotIn: + retRequirement.Operator = metav1.FieldSelectorOpNotIn + retRequirement.Key = requirement.Field + retRequirement.Values = []string{requirement.Value} + default: + // ignore this particular requirement. since requirements are AND'd, it is safe to ignore unknown requirements + // for authorization since the resulting check will only be as broad or broader than the intended. + continue + } + retRequirements = append(retRequirements, retRequirement) + } + + if len(retRequirements) == 0 { + // this means that all requirements were dropped (likely due to unknown operators), so we are checking the broader + // unrestricted action. + return nil, getFieldSelectorErr + } + return retRequirements, getFieldSelectorErr +} + +func labelSelectorToAuthorizationAPI(attr authorizer.Attributes) ([]metav1.LabelSelectorRequirement, error) { + requirements, getLabelSelectorErr := attr.GetLabelSelector() + if len(requirements) == 0 { + return nil, getLabelSelectorErr + } + + retRequirements := []metav1.LabelSelectorRequirement{} + for _, requirement := range requirements { + retRequirement := metav1.LabelSelectorRequirement{ + Key: requirement.Key(), + } + if values := requirement.ValuesUnsorted(); len(values) > 0 { + retRequirement.Values = values + } + switch requirement.Operator() { + case selection.Equals, selection.DoubleEquals, selection.In: + retRequirement.Operator = metav1.LabelSelectorOpIn + case selection.NotEquals, selection.NotIn: + retRequirement.Operator = metav1.LabelSelectorOpNotIn + case selection.Exists: + retRequirement.Operator = metav1.LabelSelectorOpExists + case selection.DoesNotExist: + retRequirement.Operator = metav1.LabelSelectorOpDoesNotExist + default: + // ignore this particular requirement. since requirements are AND'd, it is safe to ignore unknown requirements + // for authorization since the resulting check will only be as broad or broader than the intended. + continue + } + retRequirements = append(retRequirements, retRequirement) + } + + if len(retRequirements) == 0 { + // this means that all requirements were dropped (likely due to unknown operators), so we are checking the broader + // unrestricted action. + return nil, getLabelSelectorErr + } + return retRequirements, getLabelSelectorErr +} + // TODO: need to finish the method to get the rules when using webhook mode func (w *WebhookAuthorizer) RulesFor(user user.Info, namespace string) ([]authorizer.ResourceRuleInfo, []authorizer.NonResourceRuleInfo, bool, error) { var ( @@ -475,13 +571,15 @@ func v1ResourceAttributesToV1beta1ResourceAttributes(in *authorizationv1.Resourc return nil } return &authorizationv1beta1.ResourceAttributes{ - Namespace: in.Namespace, - Verb: in.Verb, - Group: in.Group, - Version: in.Version, - Resource: in.Resource, - Subresource: in.Subresource, - Name: in.Name, + Namespace: in.Namespace, + Verb: in.Verb, + Group: in.Group, + Version: in.Version, + Resource: in.Resource, + Subresource: in.Subresource, + Name: in.Name, + FieldSelector: in.FieldSelector, + LabelSelector: in.LabelSelector, } } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_test.go new file mode 100644 index 00000000000..98c8bbacf77 --- /dev/null +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_test.go @@ -0,0 +1,334 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package webhook + +import ( + "errors" + "reflect" + "testing" + + 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/apiserver/pkg/authorization/authorizer" + genericfeatures "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" +) + +func mustLabelRequirement(selector string) labels.Requirements { + ret, err := labels.Parse(selector) + if err != nil { + panic(err) + } + requirements, _ := ret.Requirements() + return requirements +} + +func Test_resourceAttributesFrom(t *testing.T) { + type args struct { + attr authorizer.Attributes + } + tests := []struct { + name string + args args + want *authorizationv1.ResourceAttributes + enableAuthorizationSelector bool + }{ + { + name: "field selector: don't parse when disabled", + args: args{ + attr: authorizer.AttributesRecord{ + FieldSelectorRequirements: fields.Requirements{ + fields.OneTermEqualSelector("foo", "bar").Requirements()[0], + }, + FieldSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{}, + }, + { + name: "label selector: don't parse when disabled", + args: args{ + attr: authorizer.AttributesRecord{ + LabelSelectorRequirements: mustLabelRequirement("foo in (bar,baz)"), + LabelSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{}, + }, + { + name: "field selector: ignore error", + args: args{ + attr: authorizer.AttributesRecord{ + FieldSelectorRequirements: fields.Requirements{ + fields.OneTermEqualSelector("foo", "bar").Requirements()[0], + }, + FieldSelectorParsingErr: errors.New("failed"), + }, + }, + want: &authorizationv1.ResourceAttributes{ + FieldSelector: &authorizationv1.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{{Key: "foo", Operator: "In", Values: []string{"bar"}}}, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label selector: ignore error", + args: args{ + attr: authorizer.AttributesRecord{ + LabelSelectorRequirements: mustLabelRequirement("foo in (bar,baz)"), + LabelSelectorParsingErr: errors.New("failed"), + }, + }, + want: &authorizationv1.ResourceAttributes{ + LabelSelector: &authorizationv1.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{{Key: "foo", Operator: "In", Values: []string{"bar", "baz"}}}, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "field selector: equals, double equals, in", + args: args{ + attr: authorizer.AttributesRecord{ + FieldSelectorRequirements: fields.Requirements{ + {Operator: selection.Equals, Field: "foo", Value: "bar"}, + {Operator: selection.DoubleEquals, Field: "one", Value: "two"}, + {Operator: selection.In, Field: "apple", Value: "banana"}, + }, + FieldSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{ + FieldSelector: &authorizationv1.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "foo", + Operator: "In", + Values: []string{"bar"}, + }, + { + Key: "one", + Operator: "In", + Values: []string{"two"}, + }, + { + Key: "apple", + Operator: "In", + Values: []string{"banana"}, + }, + }, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "field selector: not equals, not in", + args: args{ + attr: authorizer.AttributesRecord{ + FieldSelectorRequirements: fields.Requirements{ + {Operator: selection.NotEquals, Field: "foo", Value: "bar"}, + {Operator: selection.NotIn, Field: "apple", Value: "banana"}, + }, + FieldSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{ + FieldSelector: &authorizationv1.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "foo", + Operator: "NotIn", + Values: []string{"bar"}, + }, + { + Key: "apple", + Operator: "NotIn", + Values: []string{"banana"}, + }, + }, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "field selector: unknown operator skipped", + args: args{ + attr: authorizer.AttributesRecord{ + FieldSelectorRequirements: fields.Requirements{ + {Operator: selection.NotEquals, Field: "foo", Value: "bar"}, + {Operator: selection.Operator("bad"), Field: "apple", Value: "banana"}, + }, + FieldSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{ + FieldSelector: &authorizationv1.FieldSelectorAttributes{ + Requirements: []metav1.FieldSelectorRequirement{ + { + Key: "foo", + Operator: "NotIn", + Values: []string{"bar"}, + }, + }, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "field selector: no requirements has no fieldselector", + args: args{ + attr: authorizer.AttributesRecord{ + FieldSelectorRequirements: fields.Requirements{ + {Operator: selection.Operator("bad"), Field: "apple", Value: "banana"}, + }, + FieldSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{}, + enableAuthorizationSelector: true, + }, + { + name: "label selector: in, equals, double equals", + args: args{ + attr: authorizer.AttributesRecord{ + LabelSelectorRequirements: mustLabelRequirement("foo in (bar,baz), one=two, apple==banana"), + LabelSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{ + LabelSelector: &authorizationv1.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "apple", + Operator: "In", + Values: []string{"banana"}, + }, + { + Key: "foo", + Operator: "In", + Values: []string{"bar", "baz"}, + }, + { + Key: "one", + Operator: "In", + Values: []string{"two"}, + }, + }, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label selector: not in, not equals", + args: args{ + attr: authorizer.AttributesRecord{ + LabelSelectorRequirements: mustLabelRequirement("foo notin (bar,baz), one!=two"), + LabelSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{ + LabelSelector: &authorizationv1.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: "NotIn", + Values: []string{"bar", "baz"}, + }, + { + Key: "one", + Operator: "NotIn", + Values: []string{"two"}, + }, + }, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label selector: exists, not exists", + args: args{ + attr: authorizer.AttributesRecord{ + LabelSelectorRequirements: mustLabelRequirement("foo, !one"), + LabelSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{ + LabelSelector: &authorizationv1.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: "Exists", + }, + { + Key: "one", + Operator: "DoesNotExist", + }, + }, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label selector: unknown operator skipped", + args: args{ + attr: authorizer.AttributesRecord{ + LabelSelectorRequirements: mustLabelRequirement("foo != bar, apple > 1"), + LabelSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{ + LabelSelector: &authorizationv1.LabelSelectorAttributes{ + Requirements: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: "NotIn", + Values: []string{"bar"}, + }, + }, + }, + }, + enableAuthorizationSelector: true, + }, + { + name: "label selector: no requirements has no labelselector", + args: args{ + attr: authorizer.AttributesRecord{ + LabelSelectorRequirements: mustLabelRequirement("apple > 1"), + LabelSelectorParsingErr: nil, + }, + }, + want: &authorizationv1.ResourceAttributes{}, + enableAuthorizationSelector: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.enableAuthorizationSelector { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true) + } + + if got := resourceAttributesFrom(tt.args.attr); !reflect.DeepEqual(got, tt.want) { + t.Errorf("resourceAttributesFrom() = %v, want %v", got, tt.want) + } + }) + } +}