Skip to content

Commit 667621f

Browse files
author
Matt Rogers
committed
Add --rolebinding-name to policy commands
Add the --rolebinding-name option to the rolebinding and clusterrolebinding add commands for specifying the name of the rolebinding to modify. Fixes #13035
1 parent 9fae408 commit 667621f

File tree

2 files changed

+244
-1
lines changed

2 files changed

+244
-1
lines changed

‎pkg/cmd/admin/policy/modify_roles.go

+27-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ var (
4141
type RoleModificationOptions struct {
4242
RoleNamespace string
4343
RoleName string
44+
RoleBindingName string
4445
RoleBindingAccessor RoleBindingAccessor
4546

4647
Targets []string
@@ -71,6 +72,7 @@ func NewCmdAddRoleToGroup(name, fullName string, f *clientcmd.Factory, out io.Wr
7172
},
7273
}
7374

75+
cmd.Flags().StringVar(&options.RoleBindingName, "rolebinding-name", "", "Name of the rolebinding to modify or create. If left empty, appends to the first rolebinding found for the given role")
7476
cmd.Flags().StringVar(&options.RoleNamespace, "role-namespace", "", "namespace where the role is located: empty means a role defined in cluster policy")
7577

7678
return cmd
@@ -99,6 +101,7 @@ func NewCmdAddRoleToUser(name, fullName string, f *clientcmd.Factory, out io.Wri
99101
},
100102
}
101103

104+
cmd.Flags().StringVar(&options.RoleBindingName, "rolebinding-name", "", "Name of the rolebinding to modify or create. If left empty, appends to the first rolebinding found for the given role")
102105
cmd.Flags().StringVar(&options.RoleNamespace, "role-namespace", "", "namespace where the role is located: empty means a role defined in cluster policy")
103106
cmd.Flags().StringSliceVarP(&saNames, "serviceaccount", "z", saNames, "service account in the current namespace to use as a user")
104107

@@ -180,6 +183,7 @@ func NewCmdAddClusterRoleToGroup(name, fullName string, f *clientcmd.Factory, ou
180183
},
181184
}
182185

186+
cmd.Flags().StringVar(&options.RoleBindingName, "rolebinding-name", "", "Name of the rolebinding to modify or create. If left empty, appends to the first rolebinding found for the given role")
183187
return cmd
184188
}
185189

@@ -205,6 +209,7 @@ func NewCmdAddClusterRoleToUser(name, fullName string, f *clientcmd.Factory, out
205209
},
206210
}
207211

212+
cmd.Flags().StringVar(&options.RoleBindingName, "rolebinding-name", "", "Name of the rolebinding to modify or create. If left empty, appends to the first rolebinding found for the given role")
208213
cmd.Flags().StringSliceVarP(&saNames, "serviceaccount", "z", saNames, "service account in the current namespace to use as a user")
209214

210215
return cmd
@@ -267,6 +272,7 @@ func (o *RoleModificationOptions) CompleteUserWithSA(f *clientcmd.Factory, args
267272
}
268273

269274
o.RoleName = args[0]
275+
270276
if len(args) > 1 {
271277
o.Users = append(o.Users, args[1:]...)
272278
}
@@ -336,6 +342,19 @@ func (o *RoleModificationOptions) AddRole() error {
336342
if err != nil {
337343
return err
338344
}
345+
346+
if len(o.RoleBindingName) > 0 {
347+
filteredRoleBindings := []*authorizationapi.RoleBinding{}
348+
for _, rb := range roleBindings {
349+
if rb.Name == o.RoleBindingName {
350+
filteredRoleBindings = append(filteredRoleBindings, rb)
351+
}
352+
}
353+
if len(filteredRoleBindings) > 0 {
354+
roleBindings = filteredRoleBindings
355+
}
356+
}
357+
339358
roleBindingNames, err := o.RoleBindingAccessor.GetExistingRoleBindingNames()
340359
if err != nil {
341360
return err
@@ -371,9 +390,16 @@ subjectCheck:
371390
}
372391

373392
if isUpdate {
393+
if len(o.RoleBindingName) > 0 {
394+
roleBinding.Name = o.RoleBindingName
395+
}
374396
err = o.RoleBindingAccessor.UpdateRoleBinding(roleBinding)
375397
} else {
376-
roleBinding.Name = getUniqueName(o.RoleName, roleBindingNames)
398+
if len(o.RoleBindingName) > 0 {
399+
roleBinding.Name = o.RoleBindingName
400+
} else {
401+
roleBinding.Name = getUniqueName(o.RoleName, roleBindingNames)
402+
}
377403
err = o.RoleBindingAccessor.CreateRoleBinding(roleBinding)
378404
// If the rolebinding was created in the meantime, rerun
379405
if kapierrors.IsAlreadyExists(err) {

‎pkg/cmd/admin/policy/modify_roles_test.go

+217
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
package policy
2+
3+
import (
4+
"testing"
5+
"reflect"
6+
7+
"k8s.io/apimachinery/pkg/runtime"
8+
"github.com/openshift/origin/pkg/client/testclient"
9+
10+
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
11+
clientgotesting "k8s.io/client-go/testing"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
kapi "k8s.io/kubernetes/pkg/api"
14+
)
15+
16+
func TestModifyRoleBindingName(t *testing.T) {
17+
var fake *testclient.Fake
18+
fake = testclient.NewSimpleFake()
19+
20+
tests := map[string]struct {
21+
rolename string
22+
rolebindingname string
23+
users []string
24+
accessor LocalRoleBindingAccessor
25+
startPolicyBinding *authorizationapi.PolicyBinding
26+
}{
27+
"create-default-binding": {
28+
rolename: "edit",
29+
rolebindingname: "",
30+
users: []string{
31+
"foo",
32+
},
33+
accessor: NewLocalRoleBindingAccessor(metav1.NamespaceDefault, fake),
34+
startPolicyBinding: &authorizationapi.PolicyBinding {
35+
ObjectMeta: metav1.ObjectMeta{
36+
Namespace: "namespace",
37+
Name: "policybinding",
38+
},
39+
PolicyRef: kapi.ObjectReference{
40+
Namespace: "namespace",
41+
},
42+
RoleBindings: map[string]*authorizationapi.RoleBinding{},
43+
},
44+
},
45+
"create-named-binding": {
46+
rolename: "edit",
47+
rolebindingname: "rb1",
48+
users: []string{
49+
"foo",
50+
},
51+
accessor: NewLocalRoleBindingAccessor(metav1.NamespaceDefault, fake),
52+
startPolicyBinding: &authorizationapi.PolicyBinding {
53+
ObjectMeta: metav1.ObjectMeta{
54+
Namespace: "namespace",
55+
Name: "policybinding",
56+
},
57+
PolicyRef: kapi.ObjectReference{
58+
Namespace: "namespace",
59+
},
60+
RoleBindings: map[string]*authorizationapi.RoleBinding{},
61+
},
62+
},
63+
"update-default-binding": {
64+
rolename: "edit",
65+
rolebindingname: "",
66+
users: []string{
67+
"foo",
68+
"bar",
69+
},
70+
accessor: NewLocalRoleBindingAccessor(metav1.NamespaceDefault, fake),
71+
startPolicyBinding: &authorizationapi.PolicyBinding {
72+
ObjectMeta: metav1.ObjectMeta{
73+
Namespace: "namespace",
74+
Name: "policybinding",
75+
},
76+
PolicyRef: kapi.ObjectReference{
77+
Namespace: "namespace",
78+
},
79+
RoleBindings: map[string]*authorizationapi.RoleBinding{
80+
"edit": {
81+
ObjectMeta: metav1.ObjectMeta{
82+
Name: "edit",
83+
Namespace: metav1.NamespaceDefault,
84+
},
85+
Subjects: []kapi.ObjectReference{{
86+
Name: "foo",
87+
Kind: authorizationapi.UserKind,
88+
}},
89+
RoleRef: kapi.ObjectReference{
90+
Name: "edit",
91+
Namespace: metav1.NamespaceDefault,
92+
},
93+
},
94+
"custom": {
95+
ObjectMeta: metav1.ObjectMeta{
96+
Name: "custom",
97+
Namespace: metav1.NamespaceDefault,
98+
},
99+
Subjects: []kapi.ObjectReference{{
100+
Name: "baz",
101+
Kind: authorizationapi.UserKind,
102+
}},
103+
RoleRef: kapi.ObjectReference{
104+
Name: "edit",
105+
Namespace: metav1.NamespaceDefault,
106+
},
107+
},
108+
},
109+
},
110+
},
111+
"update-named-binding": {
112+
rolename: "edit",
113+
rolebindingname: "custom",
114+
users: []string{
115+
"bar",
116+
"baz",
117+
},
118+
accessor: NewLocalRoleBindingAccessor(metav1.NamespaceDefault, fake),
119+
startPolicyBinding: &authorizationapi.PolicyBinding {
120+
ObjectMeta: metav1.ObjectMeta{
121+
Namespace: "namespace",
122+
Name: "policybinding",
123+
},
124+
PolicyRef: kapi.ObjectReference{
125+
Namespace: "namespace",
126+
},
127+
RoleBindings: map[string]*authorizationapi.RoleBinding{
128+
"edit": {
129+
ObjectMeta: metav1.ObjectMeta{
130+
Name: "edit",
131+
Namespace: metav1.NamespaceDefault,
132+
},
133+
Subjects: []kapi.ObjectReference{{
134+
Name: "foo",
135+
Kind: authorizationapi.UserKind,
136+
}},
137+
RoleRef: kapi.ObjectReference{
138+
Name: "edit",
139+
Namespace: metav1.NamespaceDefault,
140+
},
141+
},
142+
"custom": {
143+
ObjectMeta: metav1.ObjectMeta{
144+
Name: "custom",
145+
Namespace: metav1.NamespaceDefault,
146+
},
147+
Subjects: []kapi.ObjectReference{{
148+
Name: "bar",
149+
Kind: authorizationapi.UserKind,
150+
}},
151+
RoleRef: kapi.ObjectReference{
152+
Name: "edit",
153+
Namespace: metav1.NamespaceDefault,
154+
},
155+
},
156+
},
157+
},
158+
},
159+
160+
}
161+
162+
for tcName, tc := range tests {
163+
164+
fake.PrependReactor("get", "policybindings", func(action clientgotesting.Action)(handled bool, ret runtime.Object, err error) {
165+
return true, tc.startPolicyBinding, nil
166+
})
167+
168+
pbList := &authorizationapi.PolicyBindingList{}
169+
fake.PrependReactor("list", "policybindings", func(action clientgotesting.Action)(handled bool, ret runtime.Object, err error) {
170+
pbList.Items = []authorizationapi.PolicyBinding{*tc.startPolicyBinding}
171+
return true, pbList, nil
172+
})
173+
174+
var actualRoleBinding *authorizationapi.RoleBinding
175+
fake.PrependReactor("update","rolebindings", func(action clientgotesting.Action)(handled bool, ret runtime.Object, err error) {
176+
actualRoleBinding = action.(clientgotesting.UpdateAction).GetObject().(*authorizationapi.RoleBinding)
177+
return true, actualRoleBinding, nil
178+
})
179+
fake.PrependReactor("create","rolebindings", func(action clientgotesting.Action)(handled bool, ret runtime.Object, err error) {
180+
actualRoleBinding = action.(clientgotesting.CreateAction).GetObject().(*authorizationapi.RoleBinding)
181+
return true, actualRoleBinding, nil
182+
})
183+
184+
o := &RoleModificationOptions{
185+
RoleName: tc.rolename,
186+
RoleBindingName: tc.rolebindingname,
187+
Users: tc.users,
188+
RoleNamespace: "default",
189+
RoleBindingAccessor: tc.accessor,
190+
}
191+
192+
var err error
193+
194+
err = o.AddRole()
195+
if err != nil {
196+
t.Errorf("%s: unexpected err %v", tcName, err)
197+
}
198+
199+
if len(tc.rolebindingname) < 1 {
200+
// Check default case for rolebindingname which is the role name.
201+
tc.rolebindingname = tc.rolename
202+
}
203+
204+
if tc.rolebindingname != actualRoleBinding.Name {
205+
t.Errorf("%s: wrong rolebinding, expected: %v, actual: %v", tcName, tc.rolebindingname, actualRoleBinding.Name)
206+
}
207+
208+
if tc.rolename != actualRoleBinding.RoleRef.Name {
209+
t.Errorf("%s: wrong role, expected: %v, actual: %v", tcName, tc.rolename, actualRoleBinding.RoleRef.Name)
210+
}
211+
212+
subs, _ := authorizationapi.StringSubjectsFor(actualRoleBinding.Namespace, actualRoleBinding.Subjects)
213+
if !reflect.DeepEqual(tc.users, subs) {
214+
t.Errorf("%s: err expected users: %v, actual: %v", tcName, tc.users, subs)
215+
}
216+
}
217+
}

0 commit comments

Comments
 (0)