From f552ffd72b168e472e43b9a983d7978cd694c0f5 Mon Sep 17 00:00:00 2001 From: Dan Ramich Date: Tue, 15 Nov 2022 23:11:30 -0700 Subject: [PATCH] Add policy view to serviceAccounts (#1840) Add the same policy view to ServiceAccounts as users/groups. This also works under the xray view. This commit also fixes the issue with role/rolebindings not having the same name not showing up in Policy view. --- internal/dao/rbac_policy.go | 25 +++++++++-- internal/dao/rbac_policy_test.go | 76 ++++++++++++++++++++++++++++++++ internal/view/sa.go | 21 ++++++++- 3 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 internal/dao/rbac_policy_test.go diff --git a/internal/dao/rbac_policy.go b/internal/dao/rbac_policy.go index e319376b..c88da803 100644 --- a/internal/dao/rbac_policy.go +++ b/internal/dao/rbac_policy.go @@ -61,10 +61,12 @@ func (p *Policy) loadClusterRoleBinding(kind, name string) (render.Policies, err return nil, err } + ns, n := client.Namespaced(name) var nn []string for _, crb := range crbs { for _, s := range crb.Subjects { - if s.Kind == kind && s.Name == name { + s := s + if isSameSubject(kind, ns, n, &s) { nn = append(nn, crb.RoleRef.Name) } } @@ -159,11 +161,14 @@ func (p *Policy) fetchRoleBindingSubjects(kind, name string) ([]string, error) { if err != nil { return nil, err } + + ns, n := client.Namespaced(name) ss := make([]string, 0, len(rbs)) for _, rb := range rbs { for _, s := range rb.Subjects { - if s.Kind == kind && s.Name == name { - ss = append(ss, rb.RoleRef.Kind+":"+rb.Name) + s := s + if isSameSubject(kind, ns, n, &s) { + ss = append(ss, rb.RoleRef.Kind+":"+rb.RoleRef.Name) } } } @@ -171,6 +176,20 @@ func (p *Policy) fetchRoleBindingSubjects(kind, name string) ([]string, error) { return ss, nil } +// isSameSubject verifies if the incoming type name and namespace match a subject from a +// cluster/roleBinding. A ServiceAccount will always have a namespace and needs to be validated to ensure +// we don't display permissions for a ServiceAccount with the same name in a different namespace +func isSameSubject(kind, namespace, name string, subject *rbacv1.Subject) bool { + if subject.Kind != kind || subject.Name != name { + return false + } + if kind == rbacv1.ServiceAccountKind { + // Kind and name were checked above, check the namespace + return subject.Namespace == namespace + } + return true +} + func (p *Policy) fetchClusterRoles() ([]rbacv1.ClusterRole, error) { const gvr = "rbac.authorization.k8s.io/v1/clusterroles" diff --git a/internal/dao/rbac_policy_test.go b/internal/dao/rbac_policy_test.go new file mode 100644 index 00000000..af49a108 --- /dev/null +++ b/internal/dao/rbac_policy_test.go @@ -0,0 +1,76 @@ +package dao + +import ( + "testing" + + "github.com/stretchr/testify/assert" + rbacv1 "k8s.io/api/rbac/v1" +) + +func TestIsSameSubject(t *testing.T) { + uu := map[string]struct { + kind string + namespace string + name string + subject rbacv1.Subject + want bool + }{ + "kind-name-match": { + kind: rbacv1.UserKind, + name: "foo", + subject: rbacv1.Subject{ + Kind: rbacv1.UserKind, + Name: "foo", + }, + want: true, + }, + "name-does-not-match": { + kind: rbacv1.UserKind, + name: "foo", + subject: rbacv1.Subject{ + Kind: rbacv1.UserKind, + Name: "bar", + }, + want: false, + }, + "kind-does-not-match": { + kind: rbacv1.GroupKind, + name: "foo", + subject: rbacv1.Subject{ + Kind: rbacv1.UserKind, + Name: "foo", + }, + want: false, + }, + "serviceAccount-all-match": { + kind: rbacv1.ServiceAccountKind, + name: "foo", + namespace: "bar", + subject: rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: "foo", + Namespace: "bar", + }, + want: true, + }, + "serviceAccount-namespace-no-match": { + kind: rbacv1.ServiceAccountKind, + name: "foo", + namespace: "bar", + subject: rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: "foo", + Namespace: "bazz", + }, + want: false, + }, + } + + for k := range uu { + u := uu[k] + t.Run(k, func(t *testing.T) { + same := isSameSubject(u.kind, u.namespace, u.name, &u.subject) + assert.Equal(t, u.want, same) + }) + } +} diff --git a/internal/view/sa.go b/internal/view/sa.go index f9778a39..4bd5237c 100644 --- a/internal/view/sa.go +++ b/internal/view/sa.go @@ -3,6 +3,7 @@ package view import ( "context" + "github.com/derailed/k9s/internal" "github.com/derailed/k9s/internal/client" "github.com/derailed/k9s/internal/dao" "github.com/derailed/k9s/internal/ui" @@ -20,20 +21,38 @@ func NewServiceAccount(gvr client.GVR) ResourceViewer { ResourceViewer: NewBrowser(gvr), } s.AddBindKeysFn(s.bindKeys) + s.SetContextFn(s.subjectCtx) return &s } func (s *ServiceAccount) bindKeys(aa ui.KeyActions) { aa.Add(ui.KeyActions{ - ui.KeyU: ui.NewKeyAction("UsedBy", s.refCmd, true), + ui.KeyU: ui.NewKeyAction("UsedBy", s.refCmd, true), + tcell.KeyEnter: ui.NewKeyAction("Rules", s.policyCmd, true), }) } +func (s *ServiceAccount) subjectCtx(ctx context.Context) context.Context { + return context.WithValue(ctx, internal.KeySubjectKind, sa) +} + func (s *ServiceAccount) refCmd(evt *tcell.EventKey) *tcell.EventKey { return scanSARefs(evt, s.App(), s.GetTable(), "v1/serviceaccounts") } +func (s *ServiceAccount) policyCmd(evt *tcell.EventKey) *tcell.EventKey { + path := s.GetTable().GetSelectedItem() + if path == "" { + return evt + } + if err := s.App().inject(NewPolicy(s.App(), sa, path)); err != nil { + s.App().Flash().Err(err) + } + + return nil +} + func scanSARefs(evt *tcell.EventKey, a *App, t *Table, gvr string) *tcell.EventKey { path := t.GetSelectedItem() if path == "" {