Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add affirmative output to oc policy / oadm policy #12324

Merged
merged 1 commit into from
Jan 16, 2017
Merged

add affirmative output to oc policy / oadm policy #12324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions pkg/cmd/admin/policy/modify_roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type RoleModificationOptions struct {
RoleName string
RoleBindingAccessor RoleBindingAccessor

Targets []string
Users []string
Groups []string
Subjects []kapi.ObjectReference
Expand All @@ -63,7 +64,10 @@ func NewCmdAddRoleToGroup(name, fullName string, f *clientcmd.Factory, out io.Wr

if err := options.AddRole(); err != nil {
kcmdutil.CheckErr(err)
return
}
printSuccessForCommand(options.RoleName, true, "group", options.Targets, true, out)

},
}

Expand All @@ -89,7 +93,9 @@ func NewCmdAddRoleToUser(name, fullName string, f *clientcmd.Factory, out io.Wri

if err := options.AddRole(); err != nil {
kcmdutil.CheckErr(err)
return
}
printSuccessForCommand(options.RoleName, true, "user", options.Targets, true, out)
},
}

Expand All @@ -114,7 +120,9 @@ func NewCmdRemoveRoleFromGroup(name, fullName string, f *clientcmd.Factory, out

if err := options.RemoveRole(); err != nil {
kcmdutil.CheckErr(err)
return
}
printSuccessForCommand(options.RoleName, false, "group", options.Targets, true, out)
},
}

Expand All @@ -139,7 +147,9 @@ func NewCmdRemoveRoleFromUser(name, fullName string, f *clientcmd.Factory, out i

if err := options.RemoveRole(); err != nil {
kcmdutil.CheckErr(err)
return
}
printSuccessForCommand(options.RoleName, false, "user", options.Targets, true, out)
},
}

Expand All @@ -164,7 +174,9 @@ func NewCmdAddClusterRoleToGroup(name, fullName string, f *clientcmd.Factory, ou

if err := options.AddRole(); err != nil {
kcmdutil.CheckErr(err)
return
}
printSuccessForCommand(options.RoleName, true, "group", options.Targets, false, out)
},
}

Expand All @@ -186,7 +198,9 @@ func NewCmdAddClusterRoleToUser(name, fullName string, f *clientcmd.Factory, out

if err := options.AddRole(); err != nil {
kcmdutil.CheckErr(err)
return
}
printSuccessForCommand(options.RoleName, true, "user", options.Targets, false, out)
},
}

Expand All @@ -208,7 +222,9 @@ func NewCmdRemoveClusterRoleFromGroup(name, fullName string, f *clientcmd.Factor

if err := options.RemoveRole(); err != nil {
kcmdutil.CheckErr(err)
return
}
printSuccessForCommand(options.RoleName, false, "group", options.Targets, false, out)
},
}

Expand All @@ -230,7 +246,9 @@ func NewCmdRemoveClusterRoleFromUser(name, fullName string, f *clientcmd.Factory

if err := options.RemoveRole(); err != nil {
kcmdutil.CheckErr(err)
return
}
printSuccessForCommand(options.RoleName, false, "user", options.Targets, false, out)
},
}

Expand All @@ -247,6 +265,8 @@ func (o *RoleModificationOptions) CompleteUserWithSA(f *clientcmd.Factory, args
o.Users = append(o.Users, args[1:]...)
}

o.Targets = o.Users

if (len(o.Users) == 0) && (len(saNames) == 0) {
return errors.New("you must specify at least one user or service account")
}
Expand All @@ -263,6 +283,7 @@ func (o *RoleModificationOptions) CompleteUserWithSA(f *clientcmd.Factory, args
o.RoleBindingAccessor = NewLocalRoleBindingAccessor(roleBindingNamespace, osClient)

for _, sa := range saNames {
o.Targets = append(o.Targets, sa)
o.Subjects = append(o.Subjects, kapi.ObjectReference{Namespace: roleBindingNamespace, Name: sa, Kind: "ServiceAccount"})
}

Expand All @@ -277,6 +298,8 @@ func (o *RoleModificationOptions) Complete(f *clientcmd.Factory, args []string,
o.RoleName = args[0]
*target = append(*target, args[1:]...)

o.Targets = *target

osClient, _, err := f.Clients()
if err != nil {
return err
Expand Down Expand Up @@ -396,3 +419,23 @@ existingLoop:

return newSubjects
}

// prints affirmative output for role modification commands
func printSuccessForCommand(role string, didAdd bool, targetName string, targets []string, isNamespaced bool, out io.Writer) {
verb := "removed"
clusterScope := "cluster "
allTargets := fmt.Sprintf("%q", targets)
if isNamespaced {
clusterScope = ""
}
if len(targets) > 1 {
targetName = fmt.Sprintf("%ss", targetName)
} else if len(targets) == 1 {
allTargets = fmt.Sprintf("%q", targets[0])
}
if didAdd {
verb = "added"
}

fmt.Fprintf(out, "%srole %q %s: %s\n", clusterScope, role, verb, allTargets)
}
22 changes: 11 additions & 11 deletions test/cmd/policy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,28 @@ os::cmd::expect_failure_and_text 'oc policy add-role-to-user' 'you must specify
os::cmd::expect_failure_and_text 'oc policy add-role-to-user -z NamespaceWithoutRole' 'you must specify a role'
os::cmd::expect_failure_and_text 'oc policy add-role-to-user view' 'you must specify at least one user or service account'

os::cmd::expect_success 'oc policy add-role-to-group cluster-admin system:unauthenticated'
os::cmd::expect_success 'oc policy add-role-to-user cluster-admin system:no-user'
os::cmd::expect_success_and_text 'oc policy add-role-to-group cluster-admin system:unauthenticated' 'role "cluster-admin" added: "system:unauthenticated"'
os::cmd::expect_success_and_text 'oc policy add-role-to-user cluster-admin system:no-user' 'role "cluster-admin" added: "system:no-user"'
os::cmd::expect_success 'oc get rolebinding/cluster-admin --no-headers'
os::cmd::expect_success_and_text 'oc get rolebinding/cluster-admin --no-headers' 'system:no-user'

os::cmd::expect_success 'oc policy add-role-to-user cluster-admin -z=one,two --serviceaccount=three,four'
os::cmd::expect_success_and_text 'oc policy add-role-to-user cluster-admin -z=one,two --serviceaccount=three,four' 'role "cluster-admin" added: \["one" "two" "three" "four"\]'
os::cmd::expect_success 'oc get rolebinding/cluster-admin --no-headers'
os::cmd::expect_success_and_text 'oc get rolebinding/cluster-admin --no-headers' 'one'
os::cmd::expect_success_and_text 'oc get rolebinding/cluster-admin --no-headers' 'four'

os::cmd::expect_success 'oc policy remove-role-from-group cluster-admin system:unauthenticated'
os::cmd::expect_success_and_text 'oc policy remove-role-from-group cluster-admin system:unauthenticated' 'role "cluster-admin" removed: "system:unauthenticated"'

os::cmd::expect_success 'oc policy remove-role-from-user cluster-admin system:no-user'
os::cmd::expect_success 'oc policy remove-role-from-user cluster-admin -z=one,two --serviceaccount=three,four'
os::cmd::expect_success_and_text 'oc policy remove-role-from-user cluster-admin system:no-user' 'role "cluster-admin" removed: "system:no-user"'
os::cmd::expect_success_and_text 'oc policy remove-role-from-user cluster-admin -z=one,two --serviceaccount=three,four' 'role "cluster-admin" removed: \["one" "two" "three" "four"\]'
os::cmd::expect_success 'oc get rolebinding/cluster-admin --no-headers'
os::cmd::expect_success_and_not_text 'oc get rolebinding/cluster-admin --no-headers' 'four'

os::cmd::expect_success 'oc policy remove-group system:unauthenticated'
os::cmd::expect_success 'oc policy remove-user system:no-user'


os::cmd::expect_success 'oc policy add-role-to-user admin namespaced-user'
os::cmd::expect_success_and_text 'oc policy add-role-to-user admin namespaced-user' 'role "admin" added: "namespaced-user"'
# Ensure the user has create permissions on builds, but that build strategy permissions are granted through the authenticated users group
os::cmd::try_until_text 'oadm policy who-can create builds' 'namespaced-user'
os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/docker' 'namespaced-user'
Expand All @@ -62,9 +62,9 @@ os::cmd::expect_success_and_text 'oadm policy who-can create builds/docker'
os::cmd::expect_success_and_text 'oadm policy who-can create builds/source' 'system:authenticated'
os::cmd::expect_success_and_text 'oadm policy who-can create builds/jenkinspipeline' 'system:authenticated'
# if this method for removing access to docker/custom/source/jenkinspipeline builds changes, docs need to be updated as well
os::cmd::expect_success 'oadm policy remove-cluster-role-from-group system:build-strategy-docker system:authenticated'
os::cmd::expect_success 'oadm policy remove-cluster-role-from-group system:build-strategy-source system:authenticated'
os::cmd::expect_success 'oadm policy remove-cluster-role-from-group system:build-strategy-jenkinspipeline system:authenticated'
os::cmd::expect_success_and_text 'oadm policy remove-cluster-role-from-group system:build-strategy-docker system:authenticated' 'cluster role "system:build-strategy-docker" removed: "system:authenticated"'
os::cmd::expect_success_and_text 'oadm policy remove-cluster-role-from-group system:build-strategy-source system:authenticated' 'cluster role "system:build-strategy-source" removed: "system:authenticated"'
os::cmd::expect_success_and_text 'oadm policy remove-cluster-role-from-group system:build-strategy-jenkinspipeline system:authenticated' 'cluster role "system:build-strategy-jenkinspipeline" removed: "system:authenticated"'
# ensure build strategy permissions no longer exist
os::cmd::try_until_failure 'oadm policy who-can create builds/source | grep system:authenticated'
os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/docker' 'system:authenticated'
Expand All @@ -73,7 +73,7 @@ os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/jenkinsp

# ensure system:authenticated users can not create custom builds by default, but can if explicitly granted access
os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/custom' 'system:authenticated'
os::cmd::expect_success 'oadm policy add-cluster-role-to-group system:build-strategy-custom system:authenticated'
os::cmd::expect_success_and_text 'oadm policy add-cluster-role-to-group system:build-strategy-custom system:authenticated' 'cluster role "system:build-strategy-custom" added: "system:authenticated"'
os::cmd::expect_success_and_text 'oadm policy who-can create builds/custom' 'system:authenticated'

os::cmd::expect_success 'oadm policy reconcile-cluster-role-bindings --confirm'
Expand Down