Skip to content

Commit 775b51d

Browse files
enjenj
committedJul 14, 2017
Remove use of policy API from CLI
This change removes the use of the policy API in CLI commands that interact with roles and bindings. The policy API is deprecated and will be removed in the 3.7 release. Thus this is required to make sure that a 3.6 oc binary continues to work with a 3.7 master. Signed-off-by: Monis Khan <[email protected]>
·
v4.1.0v3.6.0
1 parent 05b9ea8 commit 775b51d

File tree

4 files changed

+64
-84
lines changed

4 files changed

+64
-84
lines changed
 

‎pkg/authorization/apis/authorization/helpers.go

+1-21
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package authorization
22

33
import (
44
"fmt"
5-
"sort"
65
"strings"
76
"unicode"
87

@@ -105,25 +104,6 @@ func (r PolicyRule) CompactString() string {
105104
return fmt.Sprintf(formatString, formatArgs...)
106105
}
107106

108-
func getRoleBindingValues(roleBindingMap map[string]*RoleBinding) []*RoleBinding {
109-
ret := []*RoleBinding{}
110-
for _, currBinding := range roleBindingMap {
111-
ret = append(ret, currBinding)
112-
}
113-
114-
return ret
115-
}
116-
func SortRoleBindings(roleBindingMap map[string]*RoleBinding, reverse bool) []*RoleBinding {
117-
roleBindings := getRoleBindingValues(roleBindingMap)
118-
if reverse {
119-
sort.Sort(sort.Reverse(RoleBindingSorter(roleBindings)))
120-
} else {
121-
sort.Sort(RoleBindingSorter(roleBindings))
122-
}
123-
124-
return roleBindings
125-
}
126-
127107
type PolicyBindingSorter []PolicyBinding
128108

129109
func (s PolicyBindingSorter) Len() int {
@@ -136,7 +116,7 @@ func (s PolicyBindingSorter) Swap(i, j int) {
136116
s[i], s[j] = s[j], s[i]
137117
}
138118

139-
type RoleBindingSorter []*RoleBinding
119+
type RoleBindingSorter []RoleBinding
140120

141121
func (s RoleBindingSorter) Len() int {
142122
return len(s)

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

+24-23
Original file line numberDiff line numberDiff line change
@@ -132,34 +132,33 @@ func NewLocalRoleBindingAccessor(bindingNamespace string, client client.Interfac
132132
}
133133

134134
func (a LocalRoleBindingAccessor) GetExistingRoleBindingsForRole(roleNamespace, role string) ([]*authorizationapi.RoleBinding, error) {
135-
existingBindings, err := a.Client.PolicyBindings(a.BindingNamespace).Get(authorizationapi.GetPolicyBindingName(roleNamespace), metav1.GetOptions{})
135+
existingBindings, err := a.Client.RoleBindings(a.BindingNamespace).List(metav1.ListOptions{})
136136
if err != nil && !kapierrors.IsNotFound(err) {
137137
return nil, err
138138
}
139139

140140
ret := make([]*authorizationapi.RoleBinding, 0)
141141
// see if we can find an existing binding that points to the role in question.
142-
for _, currBinding := range existingBindings.RoleBindings {
143-
if currBinding.RoleRef.Name == role {
144-
t := currBinding
145-
ret = append(ret, t)
142+
for i := range existingBindings.Items {
143+
// shallow copy outside of the loop so that we can take its address
144+
currBinding := existingBindings.Items[i]
145+
if currBinding.RoleRef.Name == role && currBinding.RoleRef.Namespace == roleNamespace {
146+
ret = append(ret, &currBinding)
146147
}
147148
}
148149

149150
return ret, nil
150151
}
151152

152153
func (a LocalRoleBindingAccessor) GetExistingRoleBindingNames() (*sets.String, error) {
153-
policyBindings, err := a.Client.PolicyBindings(a.BindingNamespace).List(metav1.ListOptions{})
154+
roleBindings, err := a.Client.RoleBindings(a.BindingNamespace).List(metav1.ListOptions{})
154155
if err != nil {
155156
return nil, err
156157
}
157158

158159
ret := &sets.String{}
159-
for _, existingBindings := range policyBindings.Items {
160-
for _, currBinding := range existingBindings.RoleBindings {
161-
ret.Insert(currBinding.Name)
162-
}
160+
for _, currBinding := range roleBindings.Items {
161+
ret.Insert(currBinding.Name)
163162
}
164163

165164
return ret, nil
@@ -187,36 +186,38 @@ func NewClusterRoleBindingAccessor(client client.Interface) ClusterRoleBindingAc
187186
}
188187

189188
func (a ClusterRoleBindingAccessor) GetExistingRoleBindingsForRole(roleNamespace, role string) ([]*authorizationapi.RoleBinding, error) {
190-
uncast, err := a.Client.ClusterPolicyBindings().Get(authorizationapi.GetPolicyBindingName(roleNamespace), metav1.GetOptions{})
191-
if err != nil && !kapierrors.IsNotFound(err) {
189+
// cluster role bindings can only reference cluster roles
190+
if roleNamespace != "" {
191+
return nil, nil
192+
}
193+
194+
existingBindings, err := a.Client.ClusterRoleBindings().List(metav1.ListOptions{})
195+
if err != nil {
192196
return nil, err
193197
}
194-
existingBindings := authorizationapi.ToPolicyBinding(uncast)
195198

196199
ret := make([]*authorizationapi.RoleBinding, 0)
197200
// see if we can find an existing binding that points to the role in question.
198-
for _, currBinding := range existingBindings.RoleBindings {
199-
if currBinding.RoleRef.Name == role {
200-
t := currBinding
201-
ret = append(ret, t)
201+
for i := range existingBindings.Items {
202+
// shallow copy outside of the loop so that we can take its address
203+
currBinding := existingBindings.Items[i]
204+
if currBinding.RoleRef.Name == role && currBinding.RoleRef.Namespace == "" {
205+
ret = append(ret, authorizationapi.ToRoleBinding(&currBinding))
202206
}
203207
}
204208

205209
return ret, nil
206210
}
207211

208212
func (a ClusterRoleBindingAccessor) GetExistingRoleBindingNames() (*sets.String, error) {
209-
uncast, err := a.Client.ClusterPolicyBindings().List(metav1.ListOptions{})
213+
existingBindings, err := a.Client.ClusterRoleBindings().List(metav1.ListOptions{})
210214
if err != nil {
211215
return nil, err
212216
}
213-
policyBindings := authorizationapi.ToPolicyBindingList(uncast)
214217

215218
ret := &sets.String{}
216-
for _, existingBindings := range policyBindings.Items {
217-
for _, currBinding := range existingBindings.RoleBindings {
218-
ret.Insert(currBinding.Name)
219-
}
219+
for _, currBinding := range existingBindings.Items {
220+
ret.Insert(currBinding.Name)
220221
}
221222

222223
return ret, nil

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

+37-38
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,12 @@ func (o *RemoveFromProjectOptions) Complete(f *clientcmd.Factory, args []string,
9696
}
9797

9898
func (o *RemoveFromProjectOptions) Run() error {
99-
bindingList, err := o.Client.PolicyBindings(o.BindingNamespace).List(metav1.ListOptions{})
99+
roleBindings, err := o.Client.RoleBindings(o.BindingNamespace).List(metav1.ListOptions{})
100100
if err != nil {
101101
return err
102102
}
103-
sort.Sort(authorizationapi.PolicyBindingSorter(bindingList.Items))
103+
// maintain David's hack from #1973 (see #1975, #1976 and https://bugzilla.redhat.com/show_bug.cgi?id=1215969)
104+
sort.Sort(sort.Reverse(authorizationapi.RoleBindingSorter(roleBindings.Items)))
104105

105106
usersRemoved := sets.String{}
106107
groupsRemoved := sets.String{}
@@ -109,47 +110,45 @@ func (o *RemoveFromProjectOptions) Run() error {
109110

110111
subjectsToRemove := authorizationapi.BuildSubjects(o.Users, o.Groups, uservalidation.ValidateUserName, uservalidation.ValidateGroupName)
111112

112-
for _, currPolicyBinding := range bindingList.Items {
113-
for _, currBinding := range authorizationapi.SortRoleBindings(currPolicyBinding.RoleBindings, true) {
114-
originalSubjects := make([]kapi.ObjectReference, len(currBinding.Subjects))
115-
copy(originalSubjects, currBinding.Subjects)
116-
oldUsers, oldGroups, oldSAs, oldOthers := authorizationapi.SubjectsStrings(currBinding.Namespace, originalSubjects)
117-
oldUsersSet, oldGroupsSet, oldSAsSet, oldOtherSet := sets.NewString(oldUsers...), sets.NewString(oldGroups...), sets.NewString(oldSAs...), sets.NewString(oldOthers...)
113+
for _, currBinding := range roleBindings.Items {
114+
originalSubjects := make([]kapi.ObjectReference, len(currBinding.Subjects))
115+
copy(originalSubjects, currBinding.Subjects)
116+
oldUsers, oldGroups, oldSAs, oldOthers := authorizationapi.SubjectsStrings(currBinding.Namespace, originalSubjects)
117+
oldUsersSet, oldGroupsSet, oldSAsSet, oldOtherSet := sets.NewString(oldUsers...), sets.NewString(oldGroups...), sets.NewString(oldSAs...), sets.NewString(oldOthers...)
118118

119-
currBinding.Subjects = removeSubjects(currBinding.Subjects, subjectsToRemove)
120-
newUsers, newGroups, newSAs, newOthers := authorizationapi.SubjectsStrings(currBinding.Namespace, currBinding.Subjects)
121-
newUsersSet, newGroupsSet, newSAsSet, newOtherSet := sets.NewString(newUsers...), sets.NewString(newGroups...), sets.NewString(newSAs...), sets.NewString(newOthers...)
119+
currBinding.Subjects = removeSubjects(currBinding.Subjects, subjectsToRemove)
120+
newUsers, newGroups, newSAs, newOthers := authorizationapi.SubjectsStrings(currBinding.Namespace, currBinding.Subjects)
121+
newUsersSet, newGroupsSet, newSAsSet, newOtherSet := sets.NewString(newUsers...), sets.NewString(newGroups...), sets.NewString(newSAs...), sets.NewString(newOthers...)
122122

123-
if len(currBinding.Subjects) == len(originalSubjects) {
124-
continue
125-
}
123+
if len(currBinding.Subjects) == len(originalSubjects) {
124+
continue
125+
}
126126

127-
_, err = o.Client.RoleBindings(o.BindingNamespace).Update(currBinding)
128-
if err != nil {
129-
return err
130-
}
127+
_, err = o.Client.RoleBindings(o.BindingNamespace).Update(&currBinding)
128+
if err != nil {
129+
return err
130+
}
131131

132-
roleDisplayName := fmt.Sprintf("%s/%s", currBinding.RoleRef.Namespace, currBinding.RoleRef.Name)
133-
if len(currBinding.RoleRef.Namespace) == 0 {
134-
roleDisplayName = currBinding.RoleRef.Name
135-
}
132+
roleDisplayName := fmt.Sprintf("%s/%s", currBinding.RoleRef.Namespace, currBinding.RoleRef.Name)
133+
if len(currBinding.RoleRef.Namespace) == 0 {
134+
roleDisplayName = currBinding.RoleRef.Name
135+
}
136136

137-
if diff := oldUsersSet.Difference(newUsersSet); len(diff) != 0 {
138-
fmt.Fprintf(o.Out, "Removing %s from users %v in project %s.\n", roleDisplayName, diff.List(), o.BindingNamespace)
139-
usersRemoved.Insert(diff.List()...)
140-
}
141-
if diff := oldGroupsSet.Difference(newGroupsSet); len(diff) != 0 {
142-
fmt.Fprintf(o.Out, "Removing %s from groups %v in project %s.\n", roleDisplayName, diff.List(), o.BindingNamespace)
143-
groupsRemoved.Insert(diff.List()...)
144-
}
145-
if diff := oldSAsSet.Difference(newSAsSet); len(diff) != 0 {
146-
fmt.Fprintf(o.Out, "Removing %s from serviceaccounts %v in project %s.\n", roleDisplayName, diff.List(), o.BindingNamespace)
147-
sasRemoved.Insert(diff.List()...)
148-
}
149-
if diff := oldOtherSet.Difference(newOtherSet); len(diff) != 0 {
150-
fmt.Fprintf(o.Out, "Removing %s from subjects %v in project %s.\n", roleDisplayName, diff.List(), o.BindingNamespace)
151-
othersRemoved.Insert(diff.List()...)
152-
}
137+
if diff := oldUsersSet.Difference(newUsersSet); len(diff) != 0 {
138+
fmt.Fprintf(o.Out, "Removing %s from users %v in project %s.\n", roleDisplayName, diff.List(), o.BindingNamespace)
139+
usersRemoved.Insert(diff.List()...)
140+
}
141+
if diff := oldGroupsSet.Difference(newGroupsSet); len(diff) != 0 {
142+
fmt.Fprintf(o.Out, "Removing %s from groups %v in project %s.\n", roleDisplayName, diff.List(), o.BindingNamespace)
143+
groupsRemoved.Insert(diff.List()...)
144+
}
145+
if diff := oldSAsSet.Difference(newSAsSet); len(diff) != 0 {
146+
fmt.Fprintf(o.Out, "Removing %s from serviceaccounts %v in project %s.\n", roleDisplayName, diff.List(), o.BindingNamespace)
147+
sasRemoved.Insert(diff.List()...)
148+
}
149+
if diff := oldOtherSet.Difference(newOtherSet); len(diff) != 0 {
150+
fmt.Fprintf(o.Out, "Removing %s from subjects %v in project %s.\n", roleDisplayName, diff.List(), o.BindingNamespace)
151+
othersRemoved.Insert(diff.List()...)
153152
}
154153
}
155154

‎test/cmd/policy-storage-admin.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ os::cmd::expect_success_and_text 'oc policy can-i create pv' 'yes'
3131
os::cmd::expect_success_and_text 'oc policy can-i create storageclass' 'yes'
3232

3333
# Test failure to change policy on users for storage-admin
34-
os::cmd::expect_failure_and_text 'oc policy add-role-to-user admin storage-adm' 'cannot get policybindings'
35-
os::cmd::expect_failure_and_text 'oc policy remove-user screeley' 'cannot list policybindings'
34+
os::cmd::expect_failure_and_text 'oc policy add-role-to-user admin storage-adm' 'cannot list rolebindings'
35+
os::cmd::expect_failure_and_text 'oc policy remove-user screeley' 'cannot list rolebindings'
3636
os::cmd::expect_success 'oc logout'
3737

3838
# Test that scoped storage-admin now an admin in project foo

0 commit comments

Comments
 (0)
Please sign in to comment.