diff --git a/hack/.golint_failures b/hack/.golint_failures index 92a67928375..4b70f200a79 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -114,7 +114,6 @@ pkg/apis/policy pkg/apis/policy/v1alpha1 pkg/apis/policy/v1beta1 pkg/apis/policy/validation -pkg/apis/rbac pkg/apis/rbac/v1 pkg/apis/rbac/v1beta1 pkg/apis/rbac/validation diff --git a/pkg/apis/rbac/BUILD b/pkg/apis/rbac/BUILD index ce8db5f6b5f..f875a668704 100644 --- a/pkg/apis/rbac/BUILD +++ b/pkg/apis/rbac/BUILD @@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", + "go_test", ) go_library( @@ -43,3 +44,16 @@ filegroup( ], tags = ["automanaged"], ) + +go_test( + name = "go_default_xtest", + srcs = ["helpers_test.go"], + deps = [ + ":go_default_library", + "//pkg/api:go_default_library", + "//pkg/apis/rbac/install:go_default_library", + "//pkg/apis/rbac/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", + ], +) diff --git a/pkg/apis/rbac/helpers.go b/pkg/apis/rbac/helpers.go index 549aedc98fc..efc4c61c569 100644 --- a/pkg/apis/rbac/helpers.go +++ b/pkg/apis/rbac/helpers.go @@ -348,7 +348,7 @@ func NewRoleBindingForClusterRole(roleName, namespace string) *RoleBindingBuilde // Groups adds the specified groups as the subjects of the RoleBinding. func (r *RoleBindingBuilder) Groups(groups ...string) *RoleBindingBuilder { for _, group := range groups { - r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: GroupKind, Name: group}) + r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: GroupKind, APIGroup: GroupName, Name: group}) } return r } @@ -356,7 +356,7 @@ func (r *RoleBindingBuilder) Groups(groups ...string) *RoleBindingBuilder { // Users adds the specified users as the subjects of the RoleBinding. func (r *RoleBindingBuilder) Users(users ...string) *RoleBindingBuilder { for _, user := range users { - r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: UserKind, Name: user}) + r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: UserKind, APIGroup: GroupName, Name: user}) } return r } diff --git a/pkg/apis/rbac/helpers_test.go b/pkg/apis/rbac/helpers_test.go new file mode 100644 index 00000000000..de9f24a090a --- /dev/null +++ b/pkg/apis/rbac/helpers_test.go @@ -0,0 +1,72 @@ +/* +Copyright 2017 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 rbac_test + +import ( + "reflect" + "testing" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/rbac" + "k8s.io/kubernetes/pkg/apis/rbac/v1" + + // install RBAC types + _ "k8s.io/kubernetes/pkg/apis/rbac/install" +) + +// TestHelpersRoundTrip confirms that the rbac.New* helper functions produce RBAC objects that match objects +// that have gone through conversion and defaulting. This is required because these helper functions are +// used to create the bootstrap RBAC policy which is used during reconciliation. If they produced objects +// that did not match, reconciliation would incorrectly add duplicate data to the cluster's RBAC policy. +func TestHelpersRoundTrip(t *testing.T) { + rb := rbac.NewRoleBinding("role", "ns").Groups("g").SAs("ns", "sa").Users("u").BindingOrDie() + rbcr := rbac.NewRoleBindingForClusterRole("role", "ns").Groups("g").SAs("ns", "sa").Users("u").BindingOrDie() + crb := rbac.NewClusterBinding("role").Groups("g").SAs("ns", "sa").Users("u").BindingOrDie() + + role := &rbac.Role{ + Rules: []rbac.PolicyRule{ + rbac.NewRule("verb").Groups("g").Resources("foo").RuleOrDie(), + rbac.NewRule("verb").URLs("/foo").RuleOrDie(), + }, + } + clusterRole := &rbac.ClusterRole{ + Rules: []rbac.PolicyRule{ + rbac.NewRule("verb").Groups("g").Resources("foo").RuleOrDie(), + rbac.NewRule("verb").URLs("/foo").RuleOrDie(), + }, + } + + for _, internalObj := range []runtime.Object{&rb, &rbcr, &crb, role, clusterRole} { + v1Obj, err := api.Scheme.ConvertToVersion(internalObj, v1.SchemeGroupVersion) + if err != nil { + t.Errorf("err on %T: %v", internalObj, err) + continue + } + api.Scheme.Default(v1Obj) + roundTrippedObj, err := api.Scheme.ConvertToVersion(v1Obj, rbac.SchemeGroupVersion) + if err != nil { + t.Errorf("err on %T: %v", internalObj, err) + continue + } + if !reflect.DeepEqual(internalObj, roundTrippedObj) { + t.Errorf("err on %T: got difference:\n%s", internalObj, diff.ObjectDiff(internalObj, roundTrippedObj)) + continue + } + } +}