Skip to content

Commit

Permalink
Merge pull request #12324 from juanvallejo/jvallejo/add-affirmative-o…
Browse files Browse the repository at this point in the history 
…utput-policy-cmds

Merged by openshift-bot
  • Loading branch information
OpenShift Bot authored Jan 16, 2017
2 parents 17ede32 + 6009e22 commit 54becc6
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 11 deletions.
43 changes: 43 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,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
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

0 comments on commit 54becc6

Please sign in to comment.