From b66a25e5f870829873b659ff1c6e973f8ed44bc7 Mon Sep 17 00:00:00 2001 From: derailed Date: Tue, 4 Jun 2019 09:47:17 -0600 Subject: [PATCH] Fix issue #211 + ns access msg --- internal/config/mock_connection_test.go | 10 +++++--- internal/k8s/api.go | 27 ++++++++++++++-------- internal/k8s/config.go | 13 ++++++++--- internal/resource/mock_clustermeta_test.go | 10 +++++--- internal/resource/mock_connection_test.go | 10 +++++--- internal/views/mock_clustermeta.go | 10 +++++--- internal/views/ns.go | 6 +++-- internal/views/resource.go | 2 +- internal/watch/informer.go | 23 ++++++++++-------- internal/watch/informer_test.go | 6 ++--- internal/watch/mock_connection_test.go | 10 +++++--- 11 files changed, 83 insertions(+), 44 deletions(-) diff --git a/internal/config/mock_connection_test.go b/internal/config/mock_connection_test.go index a7ebf61b..71c22759 100644 --- a/internal/config/mock_connection_test.go +++ b/internal/config/mock_connection_test.go @@ -24,19 +24,23 @@ func NewMockConnection() *MockConnection { return &MockConnection{fail: pegomock.GlobalFailHandler} } -func (mock *MockConnection) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) bool { +func (mock *MockConnection) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) (bool, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockConnection().") } params := []pegomock.Param{_param0, _param1, _param2, _param3} - result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 bool + var ret1 error if len(result) != 0 { if result[0] != nil { ret0 = result[0].(bool) } + if result[1] != nil { + ret1 = result[1].(error) + } } - return ret0 + return ret0, ret1 } func (mock *MockConnection) Config() *k8s.Config { diff --git a/internal/k8s/api.go b/internal/k8s/api.go index 38c59e78..e0d6b9e0 100644 --- a/internal/k8s/api.go +++ b/internal/k8s/api.go @@ -64,7 +64,7 @@ type ( ServerVersion() (*version.Info, error) FetchNodes() (*v1.NodeList, error) CurrentNamespaceName() (string, error) - CanIAccess(ns, name, resURL string, verbs []string) bool + CanIAccess(ns, name, resURL string, verbs []string) (bool, error) } // APIClient represents a Kubernetes api client. @@ -90,7 +90,7 @@ func InitConnectionOrDie(config *Config, logger zerolog.Logger) *APIClient { } // CanIAccess checks if user has access to a certain resource. -func (a *APIClient) CanIAccess(ns, name, resURL string, verbs []string) bool { +func (a *APIClient) CanIAccess(ns, name, resURL string, verbs []string) (bool, error) { _, gr := schema.ParseResourceArg(strings.ToLower(resURL)) sar := &authorizationv1.SelfSubjectAccessReview{ Spec: authorizationv1.SelfSubjectAccessReviewSpec{ @@ -104,24 +104,31 @@ func (a *APIClient) CanIAccess(ns, name, resURL string, verbs []string) bool { }, } - var resp *authorizationv1.SelfSubjectAccessReview - var err error - var allow bool + user, _ := a.Config().CurrentUserName() + groups, _ := a.Config().CurrentGroupNames() + log.Debug().Msgf("AuthInfo user/groups: %s:%v", user, groups) + + var ( + resp *authorizationv1.SelfSubjectAccessReview + err error + allow bool + ) for _, v := range verbs { sar.Spec.ResourceAttributes.Verb = v resp, err = a.DialOrDie().AuthorizationV1().SelfSubjectAccessReviews().Create(sar) if err != nil { log.Warn().Err(err).Msgf("CanIAccess") - return false + return false, err } - log.Debug().Msgf("CHECKING ACCESS for %s/%s/ in NS %q verb: %s -> %t, %s", resURL, name, ns, v, resp.Status.Allowed, resp.Status.Reason) + log.Debug().Msgf("CHECKING ACCESS res:%s-%q for NS: %q Verb: %s -> %t, %s", resURL, name, ns, v, resp.Status.Allowed, resp.Status.Reason) if !resp.Status.Allowed { - return false + return false, err } allow = true } - log.Debug().Msgf("GRANT ACCESS:%t", allow) - return allow + log.Debug().Msgf("GRANT ACCESS? >>> %t", allow) + + return allow, nil } // CurrentNamespaceName return namespace name set via either cli arg or cluster config. diff --git a/internal/k8s/config.go b/internal/k8s/config.go index c91405da..ac4a2bc4 100644 --- a/internal/k8s/config.go +++ b/internal/k8s/config.go @@ -163,6 +163,15 @@ func (c *Config) ClusterNames() ([]string, error) { return cc, nil } +// CurrentGroupNames retrieves the active group names. +func (c *Config) CurrentGroupNames() ([]string, error) { + if c.flags.ImpersonateGroup != nil && len(*c.flags.ImpersonateGroup) != 0 { + return *c.flags.ImpersonateGroup, nil + } + + return []string{}, errors.New("unable to locate current group") +} + // CurrentUserName retrieves the active user name. func (c *Config) CurrentUserName() (string, error) { if isSet(c.flags.Impersonate) { @@ -186,7 +195,7 @@ func (c *Config) CurrentUserName() (string, error) { return ctx.AuthInfo, nil } - return "", errors.New("unable to locate current cluster") + return "", errors.New("unable to locate current user") } // CurrentNamespaceName retrieves the active namespace. @@ -199,12 +208,10 @@ func (c *Config) CurrentNamespaceName() (string, error) { if err != nil { return "", err } - ctx, err := c.CurrentContextName() if err != nil { return "", err } - if ctx, ok := cfg.Contexts[ctx]; ok { if isSet(&ctx.Namespace) { return ctx.Namespace, nil diff --git a/internal/resource/mock_clustermeta_test.go b/internal/resource/mock_clustermeta_test.go index ae681a7e..1ac19356 100644 --- a/internal/resource/mock_clustermeta_test.go +++ b/internal/resource/mock_clustermeta_test.go @@ -24,19 +24,23 @@ func NewMockClusterMeta() *MockClusterMeta { return &MockClusterMeta{fail: pegomock.GlobalFailHandler} } -func (mock *MockClusterMeta) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) bool { +func (mock *MockClusterMeta) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) (bool, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClusterMeta().") } params := []pegomock.Param{_param0, _param1, _param2, _param3} - result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 bool + var ret1 error if len(result) != 0 { if result[0] != nil { ret0 = result[0].(bool) } + if result[1] != nil { + ret1 = result[1].(error) + } } - return ret0 + return ret0, ret1 } func (mock *MockClusterMeta) ClusterName() string { diff --git a/internal/resource/mock_connection_test.go b/internal/resource/mock_connection_test.go index 486048a9..30650424 100644 --- a/internal/resource/mock_connection_test.go +++ b/internal/resource/mock_connection_test.go @@ -24,19 +24,23 @@ func NewMockConnection() *MockConnection { return &MockConnection{fail: pegomock.GlobalFailHandler} } -func (mock *MockConnection) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) bool { +func (mock *MockConnection) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) (bool, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockConnection().") } params := []pegomock.Param{_param0, _param1, _param2, _param3} - result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 bool + var ret1 error if len(result) != 0 { if result[0] != nil { ret0 = result[0].(bool) } + if result[1] != nil { + ret1 = result[1].(error) + } } - return ret0 + return ret0, ret1 } func (mock *MockConnection) Config() *k8s.Config { diff --git a/internal/views/mock_clustermeta.go b/internal/views/mock_clustermeta.go index 890dcd3b..8647f28e 100644 --- a/internal/views/mock_clustermeta.go +++ b/internal/views/mock_clustermeta.go @@ -24,19 +24,23 @@ func NewMockClusterMeta() *MockClusterMeta { return &MockClusterMeta{fail: pegomock.GlobalFailHandler} } -func (mock *MockClusterMeta) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) bool { +func (mock *MockClusterMeta) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) (bool, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClusterMeta().") } params := []pegomock.Param{_param0, _param1, _param2, _param3} - result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 bool + var ret1 error if len(result) != 0 { if result[0] != nil { ret0 = result[0].(bool) } + if result[1] != nil { + ret1 = result[1].(error) + } } - return ret0 + return ret0, ret1 } func (mock *MockClusterMeta) ClusterName() string { diff --git a/internal/views/ns.go b/internal/views/ns.go index 5525d48f..124e6bc2 100644 --- a/internal/views/ns.go +++ b/internal/views/ns.go @@ -6,6 +6,7 @@ import ( "github.com/derailed/k9s/internal/config" "github.com/derailed/k9s/internal/resource" "github.com/gdamore/tcell" + "github.com/rs/zerolog/log" ) const ( @@ -36,7 +37,7 @@ func (v *namespaceView) extraActions(aa keyActions) { } func (v *namespaceView) switchNs(app *appView, _, res, sel string) { - v.useNamespace(sel) + v.useNamespace(v.cleanser(sel)) app.gotoResource("po", true) } @@ -63,12 +64,13 @@ func (v *namespaceView) getSelectedItem() string { } func (*namespaceView) cleanser(s string) string { + log.Debug().Msgf("SWITCHING: %s-%s", s, nsCleanser.ReplaceAllString(s, `$1`)) return nsCleanser.ReplaceAllString(s, `$1`) } func (v *namespaceView) decorate(data resource.TableData) resource.TableData { if _, ok := data.Rows[resource.AllNamespaces]; !ok { - if v.app.conn().CanIAccess("", "namespaces", "namespace.v1", []string{"list"}) { + if acc, err := v.app.conn().CanIAccess("", "namespaces", "namespace.v1", []string{"list"}); acc && err != nil { data.Rows[resource.AllNamespace] = &resource.RowEvent{ Action: resource.Unchanged, Fields: resource.Row{resource.AllNamespace, "Active", "0"}, diff --git a/internal/views/resource.go b/internal/views/resource.go index eed0809c..1c128a74 100644 --- a/internal/views/resource.go +++ b/internal/views/resource.go @@ -97,7 +97,7 @@ func (v *resourceView) init(ctx context.Context, ns string) { } v.getTV().setColorer(colorer) - v.nsListAccess = v.app.conn().CanIAccess("", "namespaces", "namespace.v1", []string{"list"}) + v.nsListAccess, _ = v.app.conn().CanIAccess("", "namespaces", "namespace.v1", []string{"list"}) if v.nsListAccess { nn, err := k8s.NewNamespace(v.app.conn()).List(resource.AllNamespaces) if err != nil { diff --git a/internal/watch/informer.go b/internal/watch/informer.go index aeabffb6..5a86c711 100644 --- a/internal/watch/informer.go +++ b/internal/watch/informer.go @@ -62,17 +62,20 @@ func NewInformer(client k8s.Connection, ns string) *Informer { log.Debug().Msgf(">> Starting Informer") i := Informer{client: client, informers: map[string]StoreInformer{}} - nsAccess := i.client.CanIAccess("", "", "namespaces", []string{"list", "watch"}) - ns, err := client.Config().CurrentNamespaceName() - // User did not lock NS. Check all ns access if not bail - if err != nil && !nsAccess { - log.Panic().Msg("Unauthorized: Unable to list ALL namespaces. Missing verbs ['list', 'watch']. Please specify a namespace or correct RBAC") + _, err := client.CanIAccess("", "", "namespaces", []string{"list", "watch"}) + if err != nil && ns == AllNamespaces { + log.Panic().Msg("Unauthorized: All namespaces. Missing verbs ['list', 'watch']. Please specify a namespace or correct RBAC") } // Namespace is locked in. check if user has auth for this ns access. - if ns != AllNamespaces && !nsAccess { - if !i.client.CanIAccess("", ns, "namespaces", []string{"get", "watch"}) { - log.Panic().Msgf("Unauthorized: Access to namespace %q is missing verbs ['get', 'watch']", ns) + if ns != AllNamespaces { + acc, err := client.CanIAccess("", ns, "namespaces", []string{"get", "watch"}) + if err != nil { + log.Panic().Err(err).Msgf("Failed access %s", ns) + } + if !acc { + user, _ := client.Config().CurrentUserName() + log.Panic().Msgf("Unauthorized: %s access to namespace %q is missing verbs ['get', 'watch']", user, ns) } i.init(ns) } else { @@ -90,7 +93,7 @@ func (i *Informer) init(ns string) { ContainerIndex: NewContainer(po), } - if i.client.CanIAccess("", "", "nodes", []string{"list", "watch"}) { + if acc, err := i.client.CanIAccess("", "", "nodes", []string{"list", "watch"}); acc && err != nil { i.informers[NodeIndex] = NewNode(i.client) } @@ -98,7 +101,7 @@ func (i *Informer) init(ns string) { return } - if i.client.CanIAccess("", ns, "metrics.k8s.io", []string{"list", "watch"}) { + if acc, err := i.client.CanIAccess("", ns, "metrics.k8s.io", []string{"list", "watch"}); acc && err != nil { i.informers[NodeMXIndex] = NewNodeMetrics(i.client) i.informers[PodMXIndex] = NewPodMetrics(i.client, ns) } diff --git a/internal/watch/informer_test.go b/internal/watch/informer_test.go index a73cb1b3..7740467e 100644 --- a/internal/watch/informer_test.go +++ b/internal/watch/informer_test.go @@ -19,9 +19,9 @@ func TestInformerInitWithNS(t *testing.T) { cmo := NewMockConnection() m.When(cmo.Config()).ThenReturn(k8s.NewConfig(f)) m.When(cmo.HasMetrics()).ThenReturn(true) - m.When(cmo.CanIAccess("", "", "namespaces", []string{"list", "watch"})).ThenReturn(false) - m.When(cmo.CanIAccess("", ns, "namespaces", []string{"get", "watch"})).ThenReturn(true) - m.When(cmo.CanIAccess("", ns, "metrics.k8s.io", []string{"list", "watch"})).ThenReturn(true) + m.When(cmo.CanIAccess("", "", "namespaces", []string{"list", "watch"})).ThenReturn(false, nil) + m.When(cmo.CanIAccess("", ns, "namespaces", []string{"get", "watch"})).ThenReturn(true, nil) + m.When(cmo.CanIAccess("", ns, "metrics.k8s.io", []string{"list", "watch"})).ThenReturn(true, nil) i := NewInformer(cmo, ns) o, err := i.List(PodIndex, "fred", metav1.ListOptions{}) diff --git a/internal/watch/mock_connection_test.go b/internal/watch/mock_connection_test.go index 4026efd5..cdce32ca 100644 --- a/internal/watch/mock_connection_test.go +++ b/internal/watch/mock_connection_test.go @@ -24,19 +24,23 @@ func NewMockConnection() *MockConnection { return &MockConnection{fail: pegomock.GlobalFailHandler} } -func (mock *MockConnection) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) bool { +func (mock *MockConnection) CanIAccess(_param0 string, _param1 string, _param2 string, _param3 []string) (bool, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockConnection().") } params := []pegomock.Param{_param0, _param1, _param2, _param3} - result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + result := pegomock.GetGenericMockFrom(mock).Invoke("CanIAccess", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 bool + var ret1 error if len(result) != 0 { if result[0] != nil { ret0 = result[0].(bool) } + if result[1] != nil { + ret1 = result[1].(error) + } } - return ret0 + return ret0, ret1 } func (mock *MockConnection) Config() *k8s.Config {