Skip to content

Commit

Permalink
add affirmative output to oc policy / oadm policy
Browse files Browse the repository at this point in the history 
This patch adds affirmative output to commands that modify cluster and
local roles for users and groups.

**Examples**
```
$ oadm policy remove-cluster-role-from-group self-provisioner
system:authenticated:oauth system:authenticated
Successfully removed the "self-provisioner" cluster role from groups
["system:authenticated:oauth" "system:authenticated"].

$ oadm policy add-cluster-role-to-group self-provisioner
system:authenticated:oauth
Successfully added the "self-provisioner" cluster role to group
"system:authenticated:oauth".

$ oc policy add-role-to-user cluster-admin testuser
Successfully added the "cluster-admin" role to user "testuser".

$ oc policy remove-role-from-user cluster-admin testuser
Successfully removed the "cluster-admin" role from user "testuser".
```
  • Loading branch information
@juanvallejo
juanvallejo committed Dec 21, 2016
1 parent 1180f16 commit 37b8a80
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 11 deletions.
45 changes: 45 additions & 0 deletions pkg/cmd/admin/policy/modify_roles.go
View file
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,25 @@ 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) {
preposition := "from"
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"
preposition = "to"
}

fmt.Fprintf(out, "Successfully %s the %q%srole %s %s %s.\n", verb, role, clusterScope, preposition, targetName, allTargets)
}
22 changes: 11 additions & 11 deletions test/cmd/policy.sh
View file
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' 'Successfully added the "cluster-admin" role to group "system:unauthenticated"'
os::cmd::expect_success_and_text 'oc policy add-role-to-user cluster-admin system:no-user' 'Successfully added the "cluster-admin" role to user "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' 'Successfully added the "cluster-admin" role to users \["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' 'Successfully removed the "cluster-admin" role from group "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' 'Successfully removed the "cluster-admin" role from user "system:no-user"'
os::cmd::expect_success_and_text 'oc policy remove-role-from-user cluster-admin -z=one,two --serviceaccount=three,four' 'Successfully removed the "cluster-admin" role from users \["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' 'Successfully added the "admin" role to user "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' 'Successfully removed the "system:build-strategy-docker" cluster role from group "system:authenticated"'
os::cmd::expect_success_and_text 'oadm policy remove-cluster-role-from-group system:build-strategy-source system:authenticated' 'Successfully removed the "system:build-strategy-source" cluster role from group "system:authenticated"'
os::cmd::expect_success_and_text 'oadm policy remove-cluster-role-from-group system:build-strategy-jenkinspipeline system:authenticated' 'Successfully removed the "system:build-strategy-jenkinspipeline" cluster role from group "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' 'Successfully added the "system:build-strategy-custom" cluster role to group "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

0 comments on commit 37b8a80

Please sign in to comment.