From 2a76fa1c8f40e70aa2597a414951e326357cee0b Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 10 Feb 2017 11:31:40 -0500 Subject: [PATCH] Switch RBAC subject apiVersion to apiGroup in v1beta1 --- .../rbac/apiserver-node-proxy-binding.yaml | 2 +- federation/pkg/kubefed/init/init_test.go | 8 +-- pkg/api/testing/fuzzer.go | 17 ++++++ pkg/apis/rbac/helpers.go | 4 +- pkg/apis/rbac/types.go | 7 +-- pkg/apis/rbac/v1alpha1/conversion.go | 41 ++++++++++++++- pkg/apis/rbac/v1alpha1/conversion_test.go | 50 ++++++++++++++++-- pkg/apis/rbac/v1alpha1/defaults.go | 13 +++++ pkg/apis/rbac/v1alpha1/types.go | 5 +- pkg/apis/rbac/v1beta1/defaults.go | 13 +++++ pkg/apis/rbac/v1beta1/types.go | 6 ++- pkg/apis/rbac/validation/validation.go | 9 ++++ pkg/apis/rbac/validation/validation_test.go | 12 ++--- pkg/kubectl/clusterrolebinding.go | 13 ++--- pkg/kubectl/rolebinding.go | 13 ++--- .../testdata/cluster-role-bindings.yaml | 24 ++++++--- plugin/pkg/auth/authorizer/rbac/rbac_test.go | 18 +++++++ .../auth/authorizer/rbac/subject_locator.go | 4 +- .../authorizer/rbac/subject_locator_test.go | 52 +++++++++---------- test/integration/auth/rbac_test.go | 2 +- 20 files changed, 240 insertions(+), 73 deletions(-) diff --git a/cluster/addons/rbac/apiserver-node-proxy-binding.yaml b/cluster/addons/rbac/apiserver-node-proxy-binding.yaml index 1dac4e4c58a..46103d006ea 100644 --- a/cluster/addons/rbac/apiserver-node-proxy-binding.yaml +++ b/cluster/addons/rbac/apiserver-node-proxy-binding.yaml @@ -9,6 +9,6 @@ roleRef: kind: ClusterRole name: node-proxy subjects: -- apiVersion: rbac/v1beta1 +- apiGroup: rbac.authorization.k8s.io kind: User name: kube-apiserver diff --git a/federation/pkg/kubefed/init/init_test.go b/federation/pkg/kubefed/init/init_test.go index 85e5eec273f..6edaa91a969 100644 --- a/federation/pkg/kubefed/init/init_test.go +++ b/federation/pkg/kubefed/init/init_test.go @@ -718,10 +718,10 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na }, Subjects: []rbacv1beta1.Subject{ { - Kind: "ServiceAccount", - APIVersion: "", - Name: "federation-controller-manager", - Namespace: "federation-system", + Kind: "ServiceAccount", + APIGroup: "", + Name: "federation-controller-manager", + Namespace: "federation-system", }, }, RoleRef: rbacv1beta1.RoleRef{ diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index a8680cb5268..b6894944286 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -590,6 +590,23 @@ func rbacFuncs(t apitesting.TestingCommon) []interface{} { r.APIGroup = rbac.GroupName } }, + func(r *rbac.Subject, c fuzz.Continue) { + switch c.Int31n(3) { + case 0: + r.Kind = rbac.ServiceAccountKind + r.APIGroup = "" + c.FuzzNoCustom(&r.Name) + c.FuzzNoCustom(&r.Namespace) + case 1: + r.Kind = rbac.UserKind + r.APIGroup = rbac.GroupName + c.FuzzNoCustom(&r.Name) + case 2: + r.Kind = rbac.GroupKind + r.APIGroup = rbac.GroupName + c.FuzzNoCustom(&r.Name) + } + }, } } diff --git a/pkg/apis/rbac/helpers.go b/pkg/apis/rbac/helpers.go index 6beec911f5b..c0bef888fd7 100644 --- a/pkg/apis/rbac/helpers.go +++ b/pkg/apis/rbac/helpers.go @@ -220,14 +220,14 @@ func NewClusterBinding(clusterRoleName string) *ClusterRoleBindingBuilder { func (r *ClusterRoleBindingBuilder) Groups(groups ...string) *ClusterRoleBindingBuilder { for _, group := range groups { - r.ClusterRoleBinding.Subjects = append(r.ClusterRoleBinding.Subjects, Subject{Kind: GroupKind, Name: group}) + r.ClusterRoleBinding.Subjects = append(r.ClusterRoleBinding.Subjects, Subject{Kind: GroupKind, APIGroup: GroupName, Name: group}) } return r } func (r *ClusterRoleBindingBuilder) Users(users ...string) *ClusterRoleBindingBuilder { for _, user := range users { - r.ClusterRoleBinding.Subjects = append(r.ClusterRoleBinding.Subjects, Subject{Kind: UserKind, Name: user}) + r.ClusterRoleBinding.Subjects = append(r.ClusterRoleBinding.Subjects, Subject{Kind: UserKind, APIGroup: GroupName, Name: user}) } return r } diff --git a/pkg/apis/rbac/types.go b/pkg/apis/rbac/types.go index 87abe1a7c83..a0b2c262457 100644 --- a/pkg/apis/rbac/types.go +++ b/pkg/apis/rbac/types.go @@ -63,9 +63,10 @@ type Subject struct { // Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount". // If the Authorizer does not recognized the kind value, the Authorizer should report an error. Kind string - // APIVersion holds the API group and version of the referenced object. For non-object references such as "Group" and "User" this is - // expected to be API version of this API group. For example, "rbac/v1alpha1". - APIVersion string + // APIGroup holds the API group of the referenced subject. + // Defaults to "" for ServiceAccount subjects. + // Defaults to "rbac.authorization.k8s.io" for User and Group subjects. + APIGroup string // Name of the object being referenced. Name string // Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty diff --git a/pkg/apis/rbac/v1alpha1/conversion.go b/pkg/apis/rbac/v1alpha1/conversion.go index 0ff7997f8a0..4c1f4d698b7 100644 --- a/pkg/apis/rbac/v1alpha1/conversion.go +++ b/pkg/apis/rbac/v1alpha1/conversion.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "k8s.io/apimachinery/pkg/conversion" + "k8s.io/apimachinery/pkg/runtime/schema" api "k8s.io/kubernetes/pkg/apis/rbac" ) @@ -30,13 +31,51 @@ func Convert_v1alpha1_Subject_To_rbac_Subject(in *Subject, out *api.Subject, s c return err } + // specifically set the APIGroup for the three subjects recognized in v1alpha1 + switch { + case in.Kind == ServiceAccountKind: + out.APIGroup = "" + case in.Kind == UserKind: + out.APIGroup = GroupName + case in.Kind == GroupKind: + out.APIGroup = GroupName + default: + // For unrecognized kinds, use the group portion of the APIVersion if we can get it + if gv, err := schema.ParseGroupVersion(in.APIVersion); err == nil { + out.APIGroup = gv.Group + } + } + // User * in v1alpha1 will only match all authenticated users // This is only for compatibility with old RBAC bindings // Special treatment for * should not be included in v1beta1 - if out.Kind == UserKind && out.Name == "*" { + if out.Kind == UserKind && out.APIGroup == GroupName && out.Name == "*" { out.Kind = GroupKind out.Name = allAuthenticated } return nil } + +func Convert_rbac_Subject_To_v1alpha1_Subject(in *api.Subject, out *Subject, s conversion.Scope) error { + if err := autoConvert_rbac_Subject_To_v1alpha1_Subject(in, out, s); err != nil { + return err + } + + switch { + case in.Kind == ServiceAccountKind && in.APIGroup == "": + // Make service accounts v1 + out.APIVersion = "v1" + case in.Kind == UserKind && in.APIGroup == GroupName: + // users in the rbac API group get v1alpha + out.APIVersion = SchemeGroupVersion.String() + case in.Kind == GroupKind && in.APIGroup == GroupName: + // groups in the rbac API group get v1alpha + out.APIVersion = SchemeGroupVersion.String() + default: + // otherwise, they get an unspecified version of a group + out.APIVersion = schema.GroupVersion{Group: in.APIGroup}.String() + } + + return nil +} diff --git a/pkg/apis/rbac/v1alpha1/conversion_test.go b/pkg/apis/rbac/v1alpha1/conversion_test.go index 4cadab6225c..137dc6b6e2e 100644 --- a/pkg/apis/rbac/v1alpha1/conversion_test.go +++ b/pkg/apis/rbac/v1alpha1/conversion_test.go @@ -34,21 +34,63 @@ func TestConversion(t *testing.T) { "specific user": { old: &v1alpha1.RoleBinding{ RoleRef: v1alpha1.RoleRef{Name: "foo", APIGroup: v1alpha1.GroupName}, - Subjects: []v1alpha1.Subject{{Kind: "User", Name: "bob"}}, + Subjects: []v1alpha1.Subject{{Kind: "User", APIVersion: v1alpha1.SchemeGroupVersion.String(), Name: "bob"}}, }, expected: &rbacapi.RoleBinding{ RoleRef: rbacapi.RoleRef{Name: "foo", APIGroup: v1alpha1.GroupName}, - Subjects: []rbacapi.Subject{{Kind: "User", Name: "bob"}}, + Subjects: []rbacapi.Subject{{Kind: "User", APIGroup: v1alpha1.GroupName, Name: "bob"}}, }, }, "wildcard user matches authenticated": { old: &v1alpha1.RoleBinding{ RoleRef: v1alpha1.RoleRef{Name: "foo", APIGroup: v1alpha1.GroupName}, - Subjects: []v1alpha1.Subject{{Kind: "User", Name: "*"}}, + Subjects: []v1alpha1.Subject{{Kind: "User", APIVersion: v1alpha1.SchemeGroupVersion.String(), Name: "*"}}, }, expected: &rbacapi.RoleBinding{ RoleRef: rbacapi.RoleRef{Name: "foo", APIGroup: v1alpha1.GroupName}, - Subjects: []rbacapi.Subject{{Kind: "Group", Name: "system:authenticated"}}, + Subjects: []rbacapi.Subject{{Kind: "Group", APIGroup: v1alpha1.GroupName, Name: "system:authenticated"}}, + }, + }, + "missing api group gets defaulted": { + old: &v1alpha1.RoleBinding{ + RoleRef: v1alpha1.RoleRef{Name: "foo", APIGroup: v1alpha1.GroupName}, + Subjects: []v1alpha1.Subject{ + {Kind: "User", Name: "myuser"}, + {Kind: "Group", Name: "mygroup"}, + {Kind: "ServiceAccount", Name: "mysa", Namespace: "myns"}, + }, + }, + expected: &rbacapi.RoleBinding{ + RoleRef: rbacapi.RoleRef{Name: "foo", APIGroup: v1alpha1.GroupName}, + Subjects: []rbacapi.Subject{ + {Kind: "User", APIGroup: v1alpha1.GroupName, Name: "myuser"}, + {Kind: "Group", APIGroup: v1alpha1.GroupName, Name: "mygroup"}, + {Kind: "ServiceAccount", APIGroup: "", Name: "mysa", Namespace: "myns"}, + }, + }, + }, + "bad api group gets defaulted": { + old: &v1alpha1.RoleBinding{ + RoleRef: v1alpha1.RoleRef{Name: "foo", APIGroup: v1alpha1.GroupName}, + Subjects: []v1alpha1.Subject{ + {Kind: "User", APIVersion: "rbac", Name: "myuser"}, + {Kind: "Group", APIVersion: "rbac", Name: "mygroup"}, + {Kind: "ServiceAccount", APIVersion: "rbac", Name: "mysa", Namespace: "myns"}, + {Kind: "User", APIVersion: "rbac/v8", Name: "myuser"}, + {Kind: "Group", APIVersion: "rbac/v8", Name: "mygroup"}, + {Kind: "ServiceAccount", APIVersion: "rbac/v8", Name: "mysa", Namespace: "myns"}, + }, + }, + expected: &rbacapi.RoleBinding{ + RoleRef: rbacapi.RoleRef{Name: "foo", APIGroup: v1alpha1.GroupName}, + Subjects: []rbacapi.Subject{ + {Kind: "User", APIGroup: v1alpha1.GroupName, Name: "myuser"}, + {Kind: "Group", APIGroup: v1alpha1.GroupName, Name: "mygroup"}, + {Kind: "ServiceAccount", APIGroup: "", Name: "mysa", Namespace: "myns"}, + {Kind: "User", APIGroup: v1alpha1.GroupName, Name: "myuser"}, + {Kind: "Group", APIGroup: v1alpha1.GroupName, Name: "mygroup"}, + {Kind: "ServiceAccount", APIGroup: "", Name: "mysa", Namespace: "myns"}, + }, }, }, } diff --git a/pkg/apis/rbac/v1alpha1/defaults.go b/pkg/apis/rbac/v1alpha1/defaults.go index c347ff01f67..49e934916e8 100644 --- a/pkg/apis/rbac/v1alpha1/defaults.go +++ b/pkg/apis/rbac/v1alpha1/defaults.go @@ -25,6 +25,7 @@ func addDefaultingFuncs(scheme *runtime.Scheme) error { return scheme.AddDefaultingFuncs( SetDefaults_ClusterRoleBinding, SetDefaults_RoleBinding, + SetDefaults_Subject, ) } @@ -38,3 +39,15 @@ func SetDefaults_RoleBinding(obj *RoleBinding) { obj.RoleRef.APIGroup = GroupName } } +func SetDefaults_Subject(obj *Subject) { + if len(obj.APIVersion) == 0 { + switch obj.Kind { + case ServiceAccountKind: + obj.APIVersion = "v1" + case UserKind: + obj.APIVersion = SchemeGroupVersion.String() + case GroupKind: + obj.APIVersion = SchemeGroupVersion.String() + } + } +} diff --git a/pkg/apis/rbac/v1alpha1/types.go b/pkg/apis/rbac/v1alpha1/types.go index 6649d60136a..fb716172c2f 100644 --- a/pkg/apis/rbac/v1alpha1/types.go +++ b/pkg/apis/rbac/v1alpha1/types.go @@ -72,7 +72,10 @@ type Subject struct { // Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount". // If the Authorizer does not recognized the kind value, the Authorizer should report an error. Kind string `json:"kind" protobuf:"bytes,1,opt,name=kind"` - // APIVersion holds the API group and version of the referenced object. + // APIVersion holds the API group and version of the referenced subject. + // Defaults to "v1" for ServiceAccount subjects. + // Defaults to "rbac.authorization.k8s.io/v1alpha1" for User and Group subjects. + // +k8s:conversion-gen=false // +optional APIVersion string `json:"apiVersion,omitempty" protobuf:"bytes,2,opt.name=apiVersion"` // Name of the object being referenced. diff --git a/pkg/apis/rbac/v1beta1/defaults.go b/pkg/apis/rbac/v1beta1/defaults.go index 76638eefc95..6c29ae500e5 100644 --- a/pkg/apis/rbac/v1beta1/defaults.go +++ b/pkg/apis/rbac/v1beta1/defaults.go @@ -25,6 +25,7 @@ func addDefaultingFuncs(scheme *runtime.Scheme) error { return scheme.AddDefaultingFuncs( SetDefaults_ClusterRoleBinding, SetDefaults_RoleBinding, + SetDefaults_Subject, ) } @@ -38,3 +39,15 @@ func SetDefaults_RoleBinding(obj *RoleBinding) { obj.RoleRef.APIGroup = GroupName } } +func SetDefaults_Subject(obj *Subject) { + if len(obj.APIGroup) == 0 { + switch obj.Kind { + case ServiceAccountKind: + obj.APIGroup = "" + case UserKind: + obj.APIGroup = GroupName + case GroupKind: + obj.APIGroup = GroupName + } + } +} diff --git a/pkg/apis/rbac/v1beta1/types.go b/pkg/apis/rbac/v1beta1/types.go index 8d0fd0321a2..ecd2e5628c8 100644 --- a/pkg/apis/rbac/v1beta1/types.go +++ b/pkg/apis/rbac/v1beta1/types.go @@ -71,9 +71,11 @@ type Subject struct { // Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount". // If the Authorizer does not recognized the kind value, the Authorizer should report an error. Kind string `json:"kind" protobuf:"bytes,1,opt,name=kind"` - // APIVersion holds the API group and version of the referenced object. + // APIGroup holds the API group of the referenced subject. + // Defaults to "" for ServiceAccount subjects. + // Defaults to "rbac.authorization.k8s.io" for User and Group subjects. // +optional - APIVersion string `json:"apiVersion,omitempty" protobuf:"bytes,2,opt.name=apiVersion"` + APIGroup string `json:"apiGroup,omitempty" protobuf:"bytes,2,opt.name=apiGroup"` // Name of the object being referenced. Name string `json:"name" protobuf:"bytes,3,opt,name=name"` // Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty diff --git a/pkg/apis/rbac/validation/validation.go b/pkg/apis/rbac/validation/validation.go index 482511c5bb4..0fc3cb1fbc3 100644 --- a/pkg/apis/rbac/validation/validation.go +++ b/pkg/apis/rbac/validation/validation.go @@ -201,6 +201,9 @@ func validateRoleBindingSubject(subject rbac.Subject, isNamespaced bool, fldPath allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, msg)) } } + if len(subject.APIGroup) > 0 { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("apiGroup"), subject.APIGroup, []string{""})) + } if !isNamespaced && len(subject.Namespace) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("namespace"), "")) } @@ -210,12 +213,18 @@ func validateRoleBindingSubject(subject rbac.Subject, isNamespaced bool, fldPath if len(subject.Name) == 0 { allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, "user name cannot be empty")) } + if subject.APIGroup != rbac.GroupName { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("apiGroup"), subject.APIGroup, []string{rbac.GroupName})) + } case rbac.GroupKind: // TODO(ericchiang): What other restrictions on group name are there? if len(subject.Name) == 0 { allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, "group name cannot be empty")) } + if subject.APIGroup != rbac.GroupName { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("apiGroup"), subject.APIGroup, []string{rbac.GroupName})) + } default: allErrs = append(allErrs, field.NotSupported(fldPath.Child("kind"), subject.Kind, []string{rbac.ServiceAccountKind, rbac.UserKind, rbac.GroupKind})) diff --git a/pkg/apis/rbac/validation/validation_test.go b/pkg/apis/rbac/validation/validation_test.go index 25416b009ca..d460c4d424d 100644 --- a/pkg/apis/rbac/validation/validation_test.go +++ b/pkg/apis/rbac/validation/validation_test.go @@ -30,9 +30,9 @@ func TestValidateClusterRoleBinding(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "master"}, RoleRef: rbac.RoleRef{APIGroup: rbac.GroupName, Kind: "ClusterRole", Name: "valid"}, Subjects: []rbac.Subject{ - {Name: "validsaname", Namespace: "foo", Kind: rbac.ServiceAccountKind}, - {Name: "valid@username", Kind: rbac.UserKind}, - {Name: "valid@groupname", Kind: rbac.GroupKind}, + {Name: "validsaname", APIGroup: "", Namespace: "foo", Kind: rbac.ServiceAccountKind}, + {Name: "valid@username", APIGroup: rbac.GroupName, Kind: rbac.UserKind}, + {Name: "valid@groupname", APIGroup: rbac.GroupName, Kind: rbac.GroupKind}, }, }, ) @@ -145,9 +145,9 @@ func TestValidateRoleBinding(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Namespace: metav1.NamespaceDefault, Name: "master"}, RoleRef: rbac.RoleRef{APIGroup: rbac.GroupName, Kind: "Role", Name: "valid"}, Subjects: []rbac.Subject{ - {Name: "validsaname", Kind: rbac.ServiceAccountKind}, - {Name: "valid@username", Kind: rbac.UserKind}, - {Name: "valid@groupname", Kind: rbac.GroupKind}, + {Name: "validsaname", APIGroup: "", Kind: rbac.ServiceAccountKind}, + {Name: "valid@username", APIGroup: rbac.GroupName, Kind: rbac.UserKind}, + {Name: "valid@groupname", APIGroup: rbac.GroupName, Kind: rbac.GroupKind}, }, }, ) diff --git a/pkg/kubectl/clusterrolebinding.go b/pkg/kubectl/clusterrolebinding.go index d3e2a6b89fe..712f7b32694 100644 --- a/pkg/kubectl/clusterrolebinding.go +++ b/pkg/kubectl/clusterrolebinding.go @@ -117,16 +117,16 @@ func (s ClusterRoleBindingGeneratorV1) StructuredGenerate() (runtime.Object, err } for _, user := range s.Users { clusterRoleBinding.Subjects = append(clusterRoleBinding.Subjects, rbac.Subject{ - Kind: rbac.UserKind, - APIVersion: "rbac.authorization.k8s.io/v1beta1", - Name: user, + Kind: rbac.UserKind, + APIGroup: rbac.GroupName, + Name: user, }) } for _, group := range s.Groups { clusterRoleBinding.Subjects = append(clusterRoleBinding.Subjects, rbac.Subject{ - Kind: rbac.GroupKind, - APIVersion: "rbac.authorization.k8s.io/v1beta1", - Name: group, + Kind: rbac.GroupKind, + APIGroup: rbac.GroupName, + Name: group, }) } for _, sa := range s.ServiceAccounts { @@ -136,6 +136,7 @@ func (s ClusterRoleBindingGeneratorV1) StructuredGenerate() (runtime.Object, err } clusterRoleBinding.Subjects = append(clusterRoleBinding.Subjects, rbac.Subject{ Kind: rbac.ServiceAccountKind, + APIGroup: "", Namespace: tokens[0], Name: tokens[1], }) diff --git a/pkg/kubectl/rolebinding.go b/pkg/kubectl/rolebinding.go index 0820e887944..55c36d1157d 100644 --- a/pkg/kubectl/rolebinding.go +++ b/pkg/kubectl/rolebinding.go @@ -132,16 +132,16 @@ func (s RoleBindingGeneratorV1) StructuredGenerate() (runtime.Object, error) { for _, user := range s.Users { roleBinding.Subjects = append(roleBinding.Subjects, rbac.Subject{ - Kind: rbac.UserKind, - APIVersion: "rbac.authorization.k8s.io/v1beta1", - Name: user, + Kind: rbac.UserKind, + APIGroup: rbac.GroupName, + Name: user, }) } for _, group := range s.Groups { roleBinding.Subjects = append(roleBinding.Subjects, rbac.Subject{ - Kind: rbac.GroupKind, - APIVersion: "rbac.authorization.k8s.io/v1beta1", - Name: group, + Kind: rbac.GroupKind, + APIGroup: rbac.GroupName, + Name: group, }) } for _, sa := range s.ServiceAccounts { @@ -151,6 +151,7 @@ func (s RoleBindingGeneratorV1) StructuredGenerate() (runtime.Object, error) { } roleBinding.Subjects = append(roleBinding.Subjects, rbac.Subject{ Kind: rbac.ServiceAccountKind, + APIGroup: "", Namespace: tokens[0], Name: tokens[1], }) diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-role-bindings.yaml b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-role-bindings.yaml index 3f0700a11a1..02937c51ca2 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-role-bindings.yaml +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-role-bindings.yaml @@ -12,7 +12,8 @@ items: kind: ClusterRole name: cluster-admin subjects: - - kind: Group + - apiGroup: rbac.authorization.k8s.io + kind: Group name: system:masters - apiVersion: rbac.authorization.k8s.io/v1beta1 kind: ClusterRoleBinding @@ -26,9 +27,11 @@ items: kind: ClusterRole name: system:basic-user subjects: - - kind: Group + - apiGroup: rbac.authorization.k8s.io + kind: Group name: system:authenticated - - kind: Group + - apiGroup: rbac.authorization.k8s.io + kind: Group name: system:unauthenticated - apiVersion: rbac.authorization.k8s.io/v1beta1 kind: ClusterRoleBinding @@ -42,9 +45,11 @@ items: kind: ClusterRole name: system:discovery subjects: - - kind: Group + - apiGroup: rbac.authorization.k8s.io + kind: Group name: system:authenticated - - kind: Group + - apiGroup: rbac.authorization.k8s.io + kind: Group name: system:unauthenticated - apiVersion: rbac.authorization.k8s.io/v1beta1 kind: ClusterRoleBinding @@ -58,7 +63,8 @@ items: kind: ClusterRole name: system:kube-controller-manager subjects: - - kind: User + - apiGroup: rbac.authorization.k8s.io + kind: User name: system:kube-controller-manager - apiVersion: rbac.authorization.k8s.io/v1beta1 kind: ClusterRoleBinding @@ -72,7 +78,8 @@ items: kind: ClusterRole name: system:node subjects: - - kind: Group + - apiGroup: rbac.authorization.k8s.io + kind: Group name: system:nodes - apiVersion: rbac.authorization.k8s.io/v1beta1 kind: ClusterRoleBinding @@ -86,7 +93,8 @@ items: kind: ClusterRole name: system:node-proxier subjects: - - kind: User + - apiGroup: rbac.authorization.k8s.io + kind: User name: system:kube-proxy kind: List metadata: {} diff --git a/plugin/pkg/auth/authorizer/rbac/rbac_test.go b/plugin/pkg/auth/authorizer/rbac/rbac_test.go index 05e52dc58b7..c4dd0ed0b34 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac_test.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac_test.go @@ -64,6 +64,15 @@ func newClusterRoleBinding(roleName string, subjects ...string) *rbac.ClusterRol for i, subject := range subjects { split := strings.SplitN(subject, ":", 2) r.Subjects[i].Kind, r.Subjects[i].Name = split[0], split[1] + + switch r.Subjects[i].Kind { + case rbac.ServiceAccountKind: + r.Subjects[i].APIGroup = "" + case rbac.UserKind, rbac.GroupKind: + r.Subjects[i].APIGroup = rbac.GroupName + default: + panic(fmt.Errorf("invalid kind %s", r.Subjects[i].Kind)) + } } return r } @@ -82,6 +91,15 @@ func newRoleBinding(namespace, roleName string, bindType uint16, subjects ...str for i, subject := range subjects { split := strings.SplitN(subject, ":", 2) r.Subjects[i].Kind, r.Subjects[i].Name = split[0], split[1] + + switch r.Subjects[i].Kind { + case rbac.ServiceAccountKind: + r.Subjects[i].APIGroup = "" + case rbac.UserKind, rbac.GroupKind: + r.Subjects[i].APIGroup = rbac.GroupName + default: + panic(fmt.Errorf("invalid kind %s", r.Subjects[i].Kind)) + } } return r } diff --git a/plugin/pkg/auth/authorizer/rbac/subject_locator.go b/plugin/pkg/auth/authorizer/rbac/subject_locator.go index eb0be95cc7b..e86df3249ce 100644 --- a/plugin/pkg/auth/authorizer/rbac/subject_locator.go +++ b/plugin/pkg/auth/authorizer/rbac/subject_locator.go @@ -54,9 +54,9 @@ func NewSubjectAccessEvaluator(roles rbacregistryvalidation.RoleGetter, roleBind // AllowedSubjects returns the subjects that can perform an action and any errors encountered while computing the list. // It is possible to have both subjects and errors returned if some rolebindings couldn't be resolved, but others could be. func (r *SubjectAccessEvaluator) AllowedSubjects(requestAttributes authorizer.Attributes) ([]rbac.Subject, error) { - subjects := []rbac.Subject{{Kind: rbac.GroupKind, Name: user.SystemPrivilegedGroup}} + subjects := []rbac.Subject{{Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: user.SystemPrivilegedGroup}} if len(r.superUser) > 0 { - subjects = append(subjects, rbac.Subject{Kind: rbac.UserKind, APIVersion: "v1alpha1", Name: r.superUser}) + subjects = append(subjects, rbac.Subject{Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: r.superUser}) } errorlist := []error{} diff --git a/plugin/pkg/auth/authorizer/rbac/subject_locator_test.go b/plugin/pkg/auth/authorizer/rbac/subject_locator_test.go index f0d85aa6411..1182a28b548 100644 --- a/plugin/pkg/auth/authorizer/rbac/subject_locator_test.go +++ b/plugin/pkg/auth/authorizer/rbac/subject_locator_test.go @@ -58,29 +58,29 @@ func TestSubjectLocator(t *testing.T) { { &defaultAttributes{"", "", "get", "Pods", "", "ns1", ""}, []rbac.Subject{ - {Kind: rbac.GroupKind, Name: user.SystemPrivilegedGroup}, - {Kind: rbac.UserKind, Name: "super-admin"}, - {Kind: rbac.GroupKind, Name: "super-admins"}, - {Kind: rbac.UserKind, Name: "admin"}, - {Kind: rbac.GroupKind, Name: "admins"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: user.SystemPrivilegedGroup}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "super-admin"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: "super-admins"}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "admin"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: "admins"}, }, }, { // cluster role matches star in namespace &defaultAttributes{"", "", "*", "Pods", "", "*", ""}, []rbac.Subject{ - {Kind: rbac.GroupKind, Name: user.SystemPrivilegedGroup}, - {Kind: rbac.UserKind, Name: "super-admin"}, - {Kind: rbac.GroupKind, Name: "super-admins"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: user.SystemPrivilegedGroup}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "super-admin"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: "super-admins"}, }, }, { // empty ns &defaultAttributes{"", "", "*", "Pods", "", "", ""}, []rbac.Subject{ - {Kind: rbac.GroupKind, Name: user.SystemPrivilegedGroup}, - {Kind: rbac.UserKind, Name: "super-admin"}, - {Kind: rbac.GroupKind, Name: "super-admins"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: user.SystemPrivilegedGroup}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "super-admin"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: "super-admins"}, }, }, }, @@ -104,32 +104,32 @@ func TestSubjectLocator(t *testing.T) { { &defaultAttributes{"", "", "get", "Pods", "", "ns1", ""}, []rbac.Subject{ - {Kind: rbac.GroupKind, Name: user.SystemPrivilegedGroup}, - {Kind: rbac.UserKind, APIVersion: "v1alpha1", Name: "foo"}, - {Kind: rbac.UserKind, Name: "super-admin"}, - {Kind: rbac.GroupKind, Name: "super-admins"}, - {Kind: rbac.UserKind, Name: "admin"}, - {Kind: rbac.GroupKind, Name: "admins"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: user.SystemPrivilegedGroup}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "foo"}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "super-admin"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: "super-admins"}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "admin"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: "admins"}, }, }, { // verb matchies correctly &defaultAttributes{"", "", "create", "Pods", "", "ns1", ""}, []rbac.Subject{ - {Kind: rbac.GroupKind, Name: user.SystemPrivilegedGroup}, - {Kind: rbac.UserKind, APIVersion: "v1alpha1", Name: "foo"}, - {Kind: rbac.UserKind, Name: "super-admin"}, - {Kind: rbac.GroupKind, Name: "super-admins"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: user.SystemPrivilegedGroup}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "foo"}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "super-admin"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: "super-admins"}, }, }, { // binding only works in correct ns &defaultAttributes{"", "", "get", "Pods", "", "ns2", ""}, []rbac.Subject{ - {Kind: rbac.GroupKind, Name: user.SystemPrivilegedGroup}, - {Kind: rbac.UserKind, APIVersion: "v1alpha1", Name: "foo"}, - {Kind: rbac.UserKind, Name: "super-admin"}, - {Kind: rbac.GroupKind, Name: "super-admins"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: user.SystemPrivilegedGroup}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "foo"}, + {Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "super-admin"}, + {Kind: rbac.GroupKind, APIGroup: rbac.GroupName, Name: "super-admins"}, }, }, }, @@ -144,7 +144,7 @@ func TestSubjectLocator(t *testing.T) { t.Errorf("case %q %d: error %v", tt.name, i, err) } if !reflect.DeepEqual(actualSubjects, action.subjects) { - t.Errorf("case %q %d: expected %v actual %v", tt.name, i, action.subjects, actualSubjects) + t.Errorf("case %q %d: expected\n%v\nactual\n%v", tt.name, i, action.subjects, actualSubjects) } } } diff --git a/test/integration/auth/rbac_test.go b/test/integration/auth/rbac_test.go index f50d8669d92..67442644b36 100644 --- a/test/integration/auth/rbac_test.go +++ b/test/integration/auth/rbac_test.go @@ -181,7 +181,7 @@ var ( "name": "write-jobs" }, "subjects": [{ - "apiVersion": "rbac/v1alpha1", + "apiGroup": "rbac.authorization.k8s.io", "kind": "User", "name": "admin" }]