Skip to content

Commit 67f7fef

Browse files
simo5luksa
authored andcommitted
Fix oc policy remove-user to remove rolebindings too
Followup to openshift#18102 Signed-off-by: Simo Sorce <[email protected]>
1 parent 7401aa8 commit 67f7fef

File tree

3 files changed

+15
-2
lines changed

3 files changed

+15
-2
lines changed

pkg/oc/admin/policy/modify_roles.go

+6
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,12 @@ func (o *RoleModificationOptions) RemoveRole() error {
513513
if err != nil {
514514
return err
515515
}
516+
// Check that we update the rolebinding for the intended role.
517+
if existingRoleBinding.RoleRef.Name != o.RoleName || existingRoleBinding.RoleRef.Namespace != o.RoleNamespace {
518+
return fmt.Errorf("rolebinding %s contains role %s in namespace %s, instead of role %s in namespace %s",
519+
o.RoleBindingName, existingRoleBinding.RoleRef.Name, existingRoleBinding.RoleRef.Namespace, o.RoleName, o.RoleNamespace)
520+
}
521+
516522
roleBindings = make([]*authorizationapi.RoleBinding, 1)
517523
roleBindings[0] = existingRoleBinding
518524
} else {

pkg/oc/admin/policy/remove_from_project.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,11 @@ func (o *RemoveFromProjectOptions) Run() error {
176176
}
177177

178178
if !o.DryRun {
179-
_, err = o.Client.RoleBindings(o.BindingNamespace).Update(&currBinding)
179+
if len(currBinding.Subjects) > 0 {
180+
_, err = o.Client.RoleBindings(o.BindingNamespace).Update(&currBinding)
181+
} else {
182+
err = o.Client.RoleBindings(o.BindingNamespace).Delete(currBinding.Name, &metav1.DeleteOptions{})
183+
}
180184
if err != nil {
181185
return err
182186
}

test/integration/policy_commands_test.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"io/ioutil"
55
"testing"
66

7+
"k8s.io/apimachinery/pkg/api/errors"
78
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
89

910
authorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset"
@@ -89,8 +90,10 @@ func TestPolicyCommands(t *testing.T) {
8990
t.Fatalf("unexpected error: %v", err)
9091
}
9192

93+
// Check that the removal of the last subject caused the rolebinding to be
94+
// removed as well
9295
viewers, err = haroldAuthorizationClient.RoleBindings(projectName).Get("view", metav1.GetOptions{})
93-
if err != nil {
96+
if !errors.IsNotFound(err) {
9497
t.Fatalf("unexpected error: %v", err)
9598
}
9699
binding = authorizationinterfaces.NewLocalRoleBindingAdapter(viewers)

0 commit comments

Comments
 (0)