Skip to content

Commit

Permalink
Add --rolebinding-name to policy commands
Browse files Browse the repository at this point in the history
Add the --rolebinding-name option to the rolebinding and
clusterrolebinding add commands for specifying the name of the rolebinding
to modify.

Fixes #13035
  • Loading branch information
Matt Rogers committed Jul 3, 2017
1 parent 9fae408 commit 98ec082
Show file tree
Hide file tree
Showing 3 changed files with 252 additions and 23 deletions.
19 changes: 16 additions & 3 deletions pkg/cmd/admin/policy/modify_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ var (
type RoleModificationOptions struct {
RoleNamespace string
RoleName string
RoleBindingName string
RoleBindingAccessor RoleBindingAccessor

Targets []string
Expand Down Expand Up @@ -71,6 +72,7 @@ func NewCmdAddRoleToGroup(name, fullName string, f *clientcmd.Factory, out io.Wr
},
}

cmd.Flags().StringVar(&options.RoleBindingName, "rolebinding-name", "", "Name of the rolebinding to modify or create. If left empty, appends to the first rolebinding found for the given role")
cmd.Flags().StringVar(&options.RoleNamespace, "role-namespace", "", "namespace where the role is located: empty means a role defined in cluster policy")

return cmd
Expand Down Expand Up @@ -99,6 +101,7 @@ func NewCmdAddRoleToUser(name, fullName string, f *clientcmd.Factory, out io.Wri
},
}

cmd.Flags().StringVar(&options.RoleBindingName, "rolebinding-name", "", "Name of the rolebinding to modify or create. If left empty, appends to the first rolebinding found for the given role")
cmd.Flags().StringVar(&options.RoleNamespace, "role-namespace", "", "namespace where the role is located: empty means a role defined in cluster policy")
cmd.Flags().StringSliceVarP(&saNames, "serviceaccount", "z", saNames, "service account in the current namespace to use as a user")

Expand Down Expand Up @@ -180,6 +183,7 @@ func NewCmdAddClusterRoleToGroup(name, fullName string, f *clientcmd.Factory, ou
},
}

cmd.Flags().StringVar(&options.RoleBindingName, "rolebinding-name", "", "Name of the rolebinding to modify or create. If left empty, appends to the first rolebinding found for the given role")
return cmd
}

Expand All @@ -205,6 +209,7 @@ func NewCmdAddClusterRoleToUser(name, fullName string, f *clientcmd.Factory, out
},
}

cmd.Flags().StringVar(&options.RoleBindingName, "rolebinding-name", "", "Name of the rolebinding to modify or create. If left empty, appends to the first rolebinding found for the given role")
cmd.Flags().StringSliceVarP(&saNames, "serviceaccount", "z", saNames, "service account in the current namespace to use as a user")

return cmd
Expand Down Expand Up @@ -267,6 +272,7 @@ func (o *RoleModificationOptions) CompleteUserWithSA(f *clientcmd.Factory, args
}

o.RoleName = args[0]

if len(args) > 1 {
o.Users = append(o.Users, args[1:]...)
}
Expand Down Expand Up @@ -332,7 +338,7 @@ func (o *RoleModificationOptions) Complete(f *clientcmd.Factory, args []string,
}

func (o *RoleModificationOptions) AddRole() error {
roleBindings, err := o.RoleBindingAccessor.GetExistingRoleBindingsForRole(o.RoleNamespace, o.RoleName)
roleBindings, err := o.RoleBindingAccessor.GetExistingRoleBindingsForRole(o.RoleNamespace, o.RoleName, o.RoleBindingName)
if err != nil {
return err
}
Expand Down Expand Up @@ -371,9 +377,16 @@ subjectCheck:
}

if isUpdate {
if len(o.RoleBindingName) > 0 {
roleBinding.Name = o.RoleBindingName
}
err = o.RoleBindingAccessor.UpdateRoleBinding(roleBinding)
} else {
roleBinding.Name = getUniqueName(o.RoleName, roleBindingNames)
if len(o.RoleBindingName) > 0 {
roleBinding.Name = o.RoleBindingName
} else {
roleBinding.Name = getUniqueName(o.RoleName, roleBindingNames)
}
err = o.RoleBindingAccessor.CreateRoleBinding(roleBinding)
// If the rolebinding was created in the meantime, rerun
if kapierrors.IsAlreadyExists(err) {
Expand All @@ -388,7 +401,7 @@ subjectCheck:
}

func (o *RoleModificationOptions) RemoveRole() error {
roleBindings, err := o.RoleBindingAccessor.GetExistingRoleBindingsForRole(o.RoleNamespace, o.RoleName)
roleBindings, err := o.RoleBindingAccessor.GetExistingRoleBindingsForRole(o.RoleNamespace, o.RoleName, "")
if err != nil {
return err
}
Expand Down
217 changes: 217 additions & 0 deletions pkg/cmd/admin/policy/modify_roles_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
package policy

import (
"testing"
"reflect"

"k8s.io/apimachinery/pkg/runtime"
"github.com/openshift/origin/pkg/client/testclient"

authorizationapi "github.com/openshift/origin/pkg/authorization/api"
clientgotesting "k8s.io/client-go/testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kapi "k8s.io/kubernetes/pkg/api"
)

func TestModifyRoleBindingName(t *testing.T) {
var fake *testclient.Fake
fake = testclient.NewSimpleFake()

tests := map[string]struct {
rolename string
rolebindingname string
users []string
accessor LocalRoleBindingAccessor
startPolicyBinding *authorizationapi.PolicyBinding
}{
"create-default-binding": {
rolename: "edit",
rolebindingname: "",
users: []string{
"foo",
},
accessor: NewLocalRoleBindingAccessor(metav1.NamespaceDefault, fake),
startPolicyBinding: &authorizationapi.PolicyBinding {
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
Name: "policybinding",
},
PolicyRef: kapi.ObjectReference{
Namespace: "namespace",
},
RoleBindings: map[string]*authorizationapi.RoleBinding{},
},
},
"create-named-binding": {
rolename: "edit",
rolebindingname: "rb1",
users: []string{
"foo",
},
accessor: NewLocalRoleBindingAccessor(metav1.NamespaceDefault, fake),
startPolicyBinding: &authorizationapi.PolicyBinding {
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
Name: "policybinding",
},
PolicyRef: kapi.ObjectReference{
Namespace: "namespace",
},
RoleBindings: map[string]*authorizationapi.RoleBinding{},
},
},
"update-default-binding": {
rolename: "edit",
rolebindingname: "",
users: []string{
"foo",
"bar",
},
accessor: NewLocalRoleBindingAccessor(metav1.NamespaceDefault, fake),
startPolicyBinding: &authorizationapi.PolicyBinding {
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
Name: "policybinding",
},
PolicyRef: kapi.ObjectReference{
Namespace: "namespace",
},
RoleBindings: map[string]*authorizationapi.RoleBinding{
"edit": {
ObjectMeta: metav1.ObjectMeta{
Name: "edit",
Namespace: metav1.NamespaceDefault,
},
Subjects: []kapi.ObjectReference{{
Name: "foo",
Kind: authorizationapi.UserKind,
}},
RoleRef: kapi.ObjectReference{
Name: "edit",
Namespace: metav1.NamespaceDefault,
},
},
"custom": {
ObjectMeta: metav1.ObjectMeta{
Name: "custom",
Namespace: metav1.NamespaceDefault,
},
Subjects: []kapi.ObjectReference{{
Name: "baz",
Kind: authorizationapi.UserKind,
}},
RoleRef: kapi.ObjectReference{
Name: "edit",
Namespace: metav1.NamespaceDefault,
},
},
},
},
},
"update-named-binding": {
rolename: "edit",
rolebindingname: "rb1",
users: []string{
"bar",
"baz",
},
accessor: NewLocalRoleBindingAccessor(metav1.NamespaceDefault, fake),
startPolicyBinding: &authorizationapi.PolicyBinding {
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
Name: "policybinding",
},
PolicyRef: kapi.ObjectReference{
Namespace: "namespace",
},
RoleBindings: map[string]*authorizationapi.RoleBinding{
"edit": {
ObjectMeta: metav1.ObjectMeta{
Name: "edit",
Namespace: metav1.NamespaceDefault,
},
Subjects: []kapi.ObjectReference{{
Name: "foo",
Kind: authorizationapi.UserKind,
}},
RoleRef: kapi.ObjectReference{
Name: "edit",
Namespace: metav1.NamespaceDefault,
},
},
"custom": {
ObjectMeta: metav1.ObjectMeta{
Name: "custom",
Namespace: metav1.NamespaceDefault,
},
Subjects: []kapi.ObjectReference{{
Name: "bar",
Kind: authorizationapi.UserKind,
}},
RoleRef: kapi.ObjectReference{
Name: "edit",
Namespace: metav1.NamespaceDefault,
},
},
},
},
},

}

for tcName, tc := range tests {

fake.PrependReactor("get", "policybindings", func(action clientgotesting.Action)(handled bool, ret runtime.Object, err error) {
return true, tc.startPolicyBinding, nil
})

pbList := &authorizationapi.PolicyBindingList{}
fake.PrependReactor("list", "policybindings", func(action clientgotesting.Action)(handled bool, ret runtime.Object, err error) {
pbList.Items = []authorizationapi.PolicyBinding{*tc.startPolicyBinding}
return true, pbList, nil
})

var actualRoleBinding *authorizationapi.RoleBinding
fake.PrependReactor("update","rolebindings", func(action clientgotesting.Action)(handled bool, ret runtime.Object, err error) {
actualRoleBinding = action.(clientgotesting.UpdateAction).GetObject().(*authorizationapi.RoleBinding)
return true, actualRoleBinding, nil
})
fake.PrependReactor("create","rolebindings", func(action clientgotesting.Action)(handled bool, ret runtime.Object, err error) {
actualRoleBinding = action.(clientgotesting.CreateAction).GetObject().(*authorizationapi.RoleBinding)
return true, actualRoleBinding, nil
})

o := &RoleModificationOptions{
RoleName: tc.rolename,
RoleBindingName: tc.rolebindingname,
Users: tc.users,
RoleNamespace: "default",
RoleBindingAccessor: tc.accessor,
}

var err error

err = o.AddRole()
if err != nil {
t.Errorf("%s: unexpected err %v", tcName, err)
}

if len(tc.rolebindingname) < 1 {
// Check default case for rolebindingname which is the role name.
tc.rolebindingname = tc.rolename
}

if tc.rolebindingname != actualRoleBinding.Name {
t.Errorf("%s: wrong rolebinding, expected: %v, actual: %v", tcName, tc.rolebindingname, actualRoleBinding.Name)
}

if tc.rolename != actualRoleBinding.RoleRef.Name {
t.Errorf("%s: wrong role, expected: %v, actual: %v", tcName, tc.rolename, actualRoleBinding.RoleRef.Name)
}

subs, _ := authorizationapi.StringSubjectsFor(actualRoleBinding.Namespace, actualRoleBinding.Subjects)
if !reflect.DeepEqual(tc.users, subs) {
t.Errorf("%s: err expected users: %v, actual: %v", tcName, tc.users, subs)
}
}
}
39 changes: 19 additions & 20 deletions pkg/cmd/admin/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func getUniqueName(basename string, existingNames *sets.String) string {

// RoleBindingAccessor is used by role modification commands to access and modify roles
type RoleBindingAccessor interface {
GetExistingRoleBindingsForRole(roleNamespace, role string) ([]*authorizationapi.RoleBinding, error)
GetExistingRoleBindingsForRole(roleNamespace, role, rb string) ([]*authorizationapi.RoleBinding, error)
GetExistingRoleBindingNames() (*sets.String, error)
UpdateRoleBinding(binding *authorizationapi.RoleBinding) error
CreateRoleBinding(binding *authorizationapi.RoleBinding) error
Expand All @@ -131,21 +131,13 @@ func NewLocalRoleBindingAccessor(bindingNamespace string, client client.Interfac
return LocalRoleBindingAccessor{bindingNamespace, client}
}

func (a LocalRoleBindingAccessor) GetExistingRoleBindingsForRole(roleNamespace, role string) ([]*authorizationapi.RoleBinding, error) {
func (a LocalRoleBindingAccessor) GetExistingRoleBindingsForRole(roleNamespace, role, rb string) ([]*authorizationapi.RoleBinding, error) {
existingBindings, err := a.Client.PolicyBindings(a.BindingNamespace).Get(authorizationapi.GetPolicyBindingName(roleNamespace), metav1.GetOptions{})
if err != nil && !kapierrors.IsNotFound(err) {
return nil, err
}

ret := make([]*authorizationapi.RoleBinding, 0)
// see if we can find an existing binding that points to the role in question.
for _, currBinding := range existingBindings.RoleBindings {
if currBinding.RoleRef.Name == role {
t := currBinding
ret = append(ret, t)
}
}

ret := findBindingsByRoleNames(existingBindings.RoleBindings, role, rb)
return ret, nil
}

Expand Down Expand Up @@ -186,21 +178,28 @@ func NewClusterRoleBindingAccessor(client client.Interface) ClusterRoleBindingAc
return ClusterRoleBindingAccessor{client}
}

func (a ClusterRoleBindingAccessor) GetExistingRoleBindingsForRole(roleNamespace, role string) ([]*authorizationapi.RoleBinding, error) {
// Filter out bindings by role or both role and rb if rb is a non-empty string
func findBindingsByRoleNames(bindings authorizationapi.RoleBindingsByName, role string, rb string) []*authorizationapi.RoleBinding {
ret := make([]*authorizationapi.RoleBinding, 0)

for _, currBinding := range bindings {
if currBinding.RoleRef.Name == role && (len(rb) == 0 || rb == currBinding.Name) {
t := currBinding
ret = append(ret, t)
}
}

return ret
}

func (a ClusterRoleBindingAccessor) GetExistingRoleBindingsForRole(roleNamespace, role, rb string) ([]*authorizationapi.RoleBinding, error) {
uncast, err := a.Client.ClusterPolicyBindings().Get(authorizationapi.GetPolicyBindingName(roleNamespace), metav1.GetOptions{})
if err != nil && !kapierrors.IsNotFound(err) {
return nil, err
}
existingBindings := authorizationapi.ToPolicyBinding(uncast)

ret := make([]*authorizationapi.RoleBinding, 0)
// see if we can find an existing binding that points to the role in question.
for _, currBinding := range existingBindings.RoleBindings {
if currBinding.RoleRef.Name == role {
t := currBinding
ret = append(ret, t)
}
}
ret := findBindingsByRoleNames(existingBindings.RoleBindings, role, rb)

return ret, nil
}
Expand Down

0 comments on commit 98ec082

Please sign in to comment.