From da32d31aac32db57a617fd21b870b8f0c0c448b4 Mon Sep 17 00:00:00 2001 From: deads2k Date: Tue, 23 Aug 2016 15:59:53 -0400 Subject: [PATCH] remove cast utilities from rbac --- pkg/apis/rbac/validation/cast.go | 107 --------------- pkg/apis/rbac/validation/validation.go | 140 ++++++++++++-------- pkg/apis/rbac/validation/validation_test.go | 131 ++++++++++-------- 3 files changed, 161 insertions(+), 217 deletions(-) delete mode 100644 pkg/apis/rbac/validation/cast.go diff --git a/pkg/apis/rbac/validation/cast.go b/pkg/apis/rbac/validation/cast.go deleted file mode 100644 index 77edea1a0fe..00000000000 --- a/pkg/apis/rbac/validation/cast.go +++ /dev/null @@ -1,107 +0,0 @@ -/* -Copyright 2016 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 validation - -import "k8s.io/kubernetes/pkg/apis/rbac" - -// Casting utilities to and from "Cluster" level equivalents. - -func toClusterRole(in *rbac.Role) *rbac.ClusterRole { - if in == nil { - return nil - } - - ret := &rbac.ClusterRole{} - ret.ObjectMeta = in.ObjectMeta - ret.Rules = in.Rules - - return ret -} - -func toClusterRoleList(in *rbac.RoleList) *rbac.ClusterRoleList { - ret := &rbac.ClusterRoleList{} - for _, curr := range in.Items { - ret.Items = append(ret.Items, *toClusterRole(&curr)) - } - - return ret -} - -func toClusterRoleBinding(in *rbac.RoleBinding) *rbac.ClusterRoleBinding { - if in == nil { - return nil - } - - ret := &rbac.ClusterRoleBinding{} - ret.ObjectMeta = in.ObjectMeta - ret.Subjects = in.Subjects - ret.RoleRef = in.RoleRef - - return ret -} - -func toClusterRoleBindingList(in *rbac.RoleBindingList) *rbac.ClusterRoleBindingList { - ret := &rbac.ClusterRoleBindingList{} - for _, curr := range in.Items { - ret.Items = append(ret.Items, *toClusterRoleBinding(&curr)) - } - - return ret -} - -func toRole(in *rbac.ClusterRole) *rbac.Role { - if in == nil { - return nil - } - - ret := &rbac.Role{} - ret.ObjectMeta = in.ObjectMeta - ret.Rules = in.Rules - - return ret -} - -func toRoleList(in *rbac.ClusterRoleList) *rbac.RoleList { - ret := &rbac.RoleList{} - for _, curr := range in.Items { - ret.Items = append(ret.Items, *toRole(&curr)) - } - - return ret -} - -func toRoleBinding(in *rbac.ClusterRoleBinding) *rbac.RoleBinding { - if in == nil { - return nil - } - - ret := &rbac.RoleBinding{} - ret.ObjectMeta = in.ObjectMeta - ret.Subjects = in.Subjects - ret.RoleRef = in.RoleRef - - return ret -} - -func toRoleBindingList(in *rbac.ClusterRoleBindingList) *rbac.RoleBindingList { - ret := &rbac.RoleBindingList{} - for _, curr := range in.Items { - ret.Items = append(ret.Items, *toRoleBinding(&curr)) - } - - return ret -} diff --git a/pkg/apis/rbac/validation/validation.go b/pkg/apis/rbac/validation/validation.go index f4df4dbe4e2..16ae7fc5694 100644 --- a/pkg/apis/rbac/validation/validation.go +++ b/pkg/apis/rbac/validation/validation.go @@ -29,28 +29,12 @@ func minimalNameRequirements(name string, prefix bool) []string { return validation.IsValidPathSegmentName(name) } -func ValidateRole(policy *rbac.Role) field.ErrorList { - return validateRole(policy, true) -} - -func ValidateRoleUpdate(policy *rbac.Role, oldRole *rbac.Role) field.ErrorList { - return validateRoleUpdate(policy, oldRole, true) -} - -func ValidateClusterRole(policy *rbac.ClusterRole) field.ErrorList { - return validateRole(toRole(policy), false) -} - -func ValidateClusterRoleUpdate(policy *rbac.ClusterRole, oldRole *rbac.ClusterRole) field.ErrorList { - return validateRoleUpdate(toRole(policy), toRole(oldRole), false) -} - -func validateRole(role *rbac.Role, isNamespaced bool) field.ErrorList { +func ValidateRole(role *rbac.Role) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, validation.ValidateObjectMeta(&role.ObjectMeta, isNamespaced, minimalNameRequirements, field.NewPath("metadata"))...) + allErrs = append(allErrs, validation.ValidateObjectMeta(&role.ObjectMeta, true, minimalNameRequirements, field.NewPath("metadata"))...) for i, rule := range role.Rules { - if err := validatePolicyRule(rule, isNamespaced, field.NewPath("rules").Index(i)); err != nil { + if err := validatePolicyRule(rule, true, field.NewPath("rules").Index(i)); err != nil { allErrs = append(allErrs, err...) } } @@ -60,6 +44,35 @@ func validateRole(role *rbac.Role, isNamespaced bool) field.ErrorList { return nil } +func ValidateRoleUpdate(role *rbac.Role, oldRole *rbac.Role) field.ErrorList { + allErrs := ValidateRole(role) + allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&role.ObjectMeta, &oldRole.ObjectMeta, field.NewPath("metadata"))...) + + return allErrs +} + +func ValidateClusterRole(role *rbac.ClusterRole) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, validation.ValidateObjectMeta(&role.ObjectMeta, false, minimalNameRequirements, field.NewPath("metadata"))...) + + for i, rule := range role.Rules { + if err := validatePolicyRule(rule, false, field.NewPath("rules").Index(i)); err != nil { + allErrs = append(allErrs, err...) + } + } + if len(allErrs) != 0 { + return allErrs + } + return nil +} + +func ValidateClusterRoleUpdate(role *rbac.ClusterRole, oldRole *rbac.ClusterRole) field.ErrorList { + allErrs := ValidateClusterRole(role) + allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&role.ObjectMeta, &oldRole.ObjectMeta, field.NewPath("metadata"))...) + + return allErrs +} + func validatePolicyRule(rule rbac.PolicyRule, isNamespaced bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if len(rule.Verbs) == 0 { @@ -85,32 +98,9 @@ func validatePolicyRule(rule rbac.PolicyRule, isNamespaced bool, fldPath *field. return allErrs } -func validateRoleUpdate(role *rbac.Role, oldRole *rbac.Role, isNamespaced bool) field.ErrorList { - allErrs := validateRole(role, isNamespaced) - allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&role.ObjectMeta, &oldRole.ObjectMeta, field.NewPath("metadata"))...) - - return allErrs -} - -func ValidateRoleBinding(policy *rbac.RoleBinding) field.ErrorList { - return validateRoleBinding(policy, true) -} - -func ValidateRoleBindingUpdate(policy *rbac.RoleBinding, oldRoleBinding *rbac.RoleBinding) field.ErrorList { - return validateRoleBindingUpdate(policy, oldRoleBinding, true) -} - -func ValidateClusterRoleBinding(policy *rbac.ClusterRoleBinding) field.ErrorList { - return validateRoleBinding(toRoleBinding(policy), false) -} - -func ValidateClusterRoleBindingUpdate(policy *rbac.ClusterRoleBinding, oldRoleBinding *rbac.ClusterRoleBinding) field.ErrorList { - return validateRoleBindingUpdate(toRoleBinding(policy), toRoleBinding(oldRoleBinding), false) -} - -func validateRoleBinding(roleBinding *rbac.RoleBinding, isNamespaced bool) field.ErrorList { +func ValidateRoleBinding(roleBinding *rbac.RoleBinding) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, validation.ValidateObjectMeta(&roleBinding.ObjectMeta, isNamespaced, minimalNameRequirements, field.NewPath("metadata"))...) + allErrs = append(allErrs, validation.ValidateObjectMeta(&roleBinding.ObjectMeta, true, minimalNameRequirements, field.NewPath("metadata"))...) // roleRef namespace is empty when referring to global policy. if len(roleBinding.RoleRef.Namespace) > 0 { @@ -129,7 +119,56 @@ func validateRoleBinding(roleBinding *rbac.RoleBinding, isNamespaced bool) field subjectsPath := field.NewPath("subjects") for i, subject := range roleBinding.Subjects { - allErrs = append(allErrs, validateRoleBindingSubject(subject, isNamespaced, subjectsPath.Index(i))...) + allErrs = append(allErrs, validateRoleBindingSubject(subject, true, subjectsPath.Index(i))...) + } + + return allErrs +} + +func ValidateRoleBindingUpdate(roleBinding *rbac.RoleBinding, oldRoleBinding *rbac.RoleBinding) field.ErrorList { + allErrs := ValidateRoleBinding(roleBinding) + allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&roleBinding.ObjectMeta, &oldRoleBinding.ObjectMeta, field.NewPath("metadata"))...) + + if oldRoleBinding.RoleRef != roleBinding.RoleRef { + allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef"), roleBinding.RoleRef, "cannot change roleRef")) + } + + return allErrs +} + +func ValidateClusterRoleBinding(roleBinding *rbac.ClusterRoleBinding) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, validation.ValidateObjectMeta(&roleBinding.ObjectMeta, false, minimalNameRequirements, field.NewPath("metadata"))...) + + // roleRef namespace is empty when referring to global policy. + if len(roleBinding.RoleRef.Namespace) > 0 { + for _, msg := range validation.ValidateNamespaceName(roleBinding.RoleRef.Namespace, false) { + allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef", "namespace"), roleBinding.RoleRef.Namespace, msg)) + } + } + + if len(roleBinding.RoleRef.Name) == 0 { + allErrs = append(allErrs, field.Required(field.NewPath("roleRef", "name"), "")) + } else { + for _, msg := range minimalNameRequirements(roleBinding.RoleRef.Name, false) { + allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef", "name"), roleBinding.RoleRef.Name, msg)) + } + } + + subjectsPath := field.NewPath("subjects") + for i, subject := range roleBinding.Subjects { + allErrs = append(allErrs, validateRoleBindingSubject(subject, false, subjectsPath.Index(i))...) + } + + return allErrs +} + +func ValidateClusterRoleBindingUpdate(roleBinding *rbac.ClusterRoleBinding, oldRoleBinding *rbac.ClusterRoleBinding) field.ErrorList { + allErrs := ValidateClusterRoleBinding(roleBinding) + allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&roleBinding.ObjectMeta, &oldRoleBinding.ObjectMeta, field.NewPath("metadata"))...) + + if oldRoleBinding.RoleRef != roleBinding.RoleRef { + allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef"), roleBinding.RoleRef, "cannot change roleRef")) } return allErrs @@ -171,14 +210,3 @@ func validateRoleBindingSubject(subject rbac.Subject, isNamespaced bool, fldPath return allErrs } - -func validateRoleBindingUpdate(roleBinding *rbac.RoleBinding, oldRoleBinding *rbac.RoleBinding, isNamespaced bool) field.ErrorList { - allErrs := validateRoleBinding(roleBinding, isNamespaced) - allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&roleBinding.ObjectMeta, &oldRoleBinding.ObjectMeta, field.NewPath("metadata"))...) - - if oldRoleBinding.RoleRef != roleBinding.RoleRef { - allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef"), roleBinding.RoleRef, "cannot change roleRef")) - } - - return allErrs -} diff --git a/pkg/apis/rbac/validation/validation_test.go b/pkg/apis/rbac/validation/validation_test.go index 7e4f1699a1f..c969b21f2d4 100644 --- a/pkg/apis/rbac/validation/validation_test.go +++ b/pkg/apis/rbac/validation/validation_test.go @@ -25,7 +25,7 @@ import ( ) func TestValidateRoleBinding(t *testing.T) { - errs := validateRoleBinding( + errs := ValidateRoleBinding( &rbac.RoleBinding{ ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "master"}, RoleRef: api.ObjectReference{Namespace: "master", Name: "valid"}, @@ -35,7 +35,6 @@ func TestValidateRoleBinding(t *testing.T) { {Name: "valid@groupname", Kind: rbac.GroupKind}, }, }, - true, ) if len(errs) != 0 { t.Errorf("expected success: %v", errs) @@ -107,7 +106,7 @@ func TestValidateRoleBinding(t *testing.T) { }, } for k, v := range errorCases { - errs := validateRoleBinding(&v.A, true) + errs := ValidateRoleBinding(&v.A) if len(errs) == 0 { t.Errorf("expected failure %s for %v", k, v.A) continue @@ -129,13 +128,12 @@ func TestValidateRoleBindingUpdate(t *testing.T) { RoleRef: api.ObjectReference{Namespace: "master", Name: "valid"}, } - errs := validateRoleBindingUpdate( + errs := ValidateRoleBindingUpdate( &rbac.RoleBinding{ ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "master", ResourceVersion: "1"}, RoleRef: api.ObjectReference{Namespace: "master", Name: "valid"}, }, old, - true, ) if len(errs) != 0 { t.Errorf("expected success: %v", errs) @@ -156,7 +154,7 @@ func TestValidateRoleBindingUpdate(t *testing.T) { }, } for k, v := range errorCases { - errs := validateRoleBindingUpdate(&v.A, old, true) + errs := ValidateRoleBindingUpdate(&v.A, old) if len(errs) == 0 { t.Errorf("expected failure %s for %v", k, v.A) continue @@ -197,16 +195,44 @@ func TestNonResourceURLCovers(t *testing.T) { } } -type validateRoleTest struct { - role rbac.Role - isNamespaced bool - wantErr bool - errType field.ErrorType - field string +type ValidateRoleTest struct { + role rbac.Role + wantErr bool + errType field.ErrorType + field string } -func (v validateRoleTest) test(t *testing.T) { - errs := validateRole(&v.role, v.isNamespaced) +func (v ValidateRoleTest) test(t *testing.T) { + errs := ValidateRole(&v.role) + if len(errs) == 0 { + if v.wantErr { + t.Fatal("expected validation error") + } + return + } + if !v.wantErr { + t.Errorf("didn't expect error, got %v", errs) + return + } + for i := range errs { + if errs[i].Type != v.errType { + t.Errorf("expected errors to have type %s: %v", v.errType, errs[i]) + } + if errs[i].Field != v.field { + t.Errorf("expected errors to have field %s: %v", v.field, errs[i]) + } + } +} + +type ValidateClusterRoleTest struct { + role rbac.ClusterRole + wantErr bool + errType field.ErrorType + field string +} + +func (v ValidateClusterRoleTest) test(t *testing.T) { + errs := ValidateClusterRole(&v.role) if len(errs) == 0 { if v.wantErr { t.Fatal("expected validation error") @@ -228,57 +254,53 @@ func (v validateRoleTest) test(t *testing.T) { } func TestValidateRoleZeroLengthNamespace(t *testing.T) { - validateRoleTest{ + ValidateRoleTest{ role: rbac.Role{ ObjectMeta: api.ObjectMeta{Name: "default"}, }, - isNamespaced: true, - wantErr: true, - errType: field.ErrorTypeRequired, - field: "metadata.namespace", + wantErr: true, + errType: field.ErrorTypeRequired, + field: "metadata.namespace", }.test(t) } func TestValidateRoleZeroLengthName(t *testing.T) { - validateRoleTest{ + ValidateRoleTest{ role: rbac.Role{ ObjectMeta: api.ObjectMeta{Namespace: "default"}, }, - isNamespaced: true, - wantErr: true, - errType: field.ErrorTypeRequired, - field: "metadata.name", + wantErr: true, + errType: field.ErrorTypeRequired, + field: "metadata.name", }.test(t) } func TestValidateRoleValidRole(t *testing.T) { - validateRoleTest{ + ValidateRoleTest{ role: rbac.Role{ ObjectMeta: api.ObjectMeta{ Namespace: "default", Name: "default", }, }, - isNamespaced: true, - wantErr: false, + wantErr: false, }.test(t) } func TestValidateRoleValidRoleNoNamespace(t *testing.T) { - validateRoleTest{ - role: rbac.Role{ + ValidateClusterRoleTest{ + role: rbac.ClusterRole{ ObjectMeta: api.ObjectMeta{ Name: "default", }, }, - isNamespaced: false, - wantErr: false, + wantErr: false, }.test(t) } func TestValidateRoleNonResourceURL(t *testing.T) { - validateRoleTest{ - role: rbac.Role{ + ValidateClusterRoleTest{ + role: rbac.ClusterRole{ ObjectMeta: api.ObjectMeta{ Name: "default", }, @@ -289,13 +311,12 @@ func TestValidateRoleNonResourceURL(t *testing.T) { }, }, }, - isNamespaced: false, - wantErr: false, + wantErr: false, }.test(t) } func TestValidateRoleNamespacedNonResourceURL(t *testing.T) { - validateRoleTest{ + ValidateRoleTest{ role: rbac.Role{ ObjectMeta: api.ObjectMeta{ Namespace: "default", @@ -309,16 +330,15 @@ func TestValidateRoleNamespacedNonResourceURL(t *testing.T) { }, }, }, - isNamespaced: true, - wantErr: true, - errType: field.ErrorTypeInvalid, - field: "rules[0].nonResourceURLs", + wantErr: true, + errType: field.ErrorTypeInvalid, + field: "rules[0].nonResourceURLs", }.test(t) } func TestValidateRoleNonResourceURLNoVerbs(t *testing.T) { - validateRoleTest{ - role: rbac.Role{ + ValidateClusterRoleTest{ + role: rbac.ClusterRole{ ObjectMeta: api.ObjectMeta{ Name: "default", }, @@ -329,18 +349,18 @@ func TestValidateRoleNonResourceURLNoVerbs(t *testing.T) { }, }, }, - isNamespaced: false, - wantErr: true, - errType: field.ErrorTypeRequired, - field: "rules[0].verbs", + wantErr: true, + errType: field.ErrorTypeRequired, + field: "rules[0].verbs", }.test(t) } func TestValidateRoleMixedNonResourceAndResource(t *testing.T) { - validateRoleTest{ + ValidateRoleTest{ role: rbac.Role{ ObjectMeta: api.ObjectMeta{ - Name: "default", + Name: "default", + Namespace: "default", }, Rules: []rbac.PolicyRule{ { @@ -358,10 +378,11 @@ func TestValidateRoleMixedNonResourceAndResource(t *testing.T) { } func TestValidateRoleValidResource(t *testing.T) { - validateRoleTest{ + ValidateRoleTest{ role: rbac.Role{ ObjectMeta: api.ObjectMeta{ - Name: "default", + Name: "default", + Namespace: "default", }, Rules: []rbac.PolicyRule{ { @@ -376,10 +397,11 @@ func TestValidateRoleValidResource(t *testing.T) { } func TestValidateRoleNoAPIGroup(t *testing.T) { - validateRoleTest{ + ValidateRoleTest{ role: rbac.Role{ ObjectMeta: api.ObjectMeta{ - Name: "default", + Name: "default", + Namespace: "default", }, Rules: []rbac.PolicyRule{ { @@ -395,10 +417,11 @@ func TestValidateRoleNoAPIGroup(t *testing.T) { } func TestValidateRoleNoResources(t *testing.T) { - validateRoleTest{ + ValidateRoleTest{ role: rbac.Role{ ObjectMeta: api.ObjectMeta{ - Name: "default", + Name: "default", + Namespace: "default", }, Rules: []rbac.PolicyRule{ {