Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

Commit

Permalink
Use namespace scoped informers under restricted rbac (#2923)
Browse files Browse the repository at this point in the history
Signed-off-by: Sam Foo <[email protected]>
  • Loading branch information
Sam Foo committed Oct 6, 2021
1 parent 19e4190 commit 3c89689
Show file tree
Hide file tree
Showing 13 changed files with 168 additions and 68 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/2900-GuessWhoSamFoo
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Check resource access for namespaces before starting informers
2 changes: 1 addition & 1 deletion internal/api/content_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const (
type ContentManagerOption func(manager *ContentManager)

// ContentGenerateFunc is a function that generates content. It returns `rerun=true`
// if the action should be be immediately rerun.
// if the action should be immediately rerun.
type ContentGenerateFunc func(ctx context.Context, state octant.State) (Content, bool, error)

// Content is a content to be sent to clients.
Expand Down
6 changes: 3 additions & 3 deletions internal/api/namespaces_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (n *NamespacesManager) runUpdate(state octant.State, client api.OctantClien
}

// NamespacesGenerator generates a list of namespaces.
func NamespacesGenerator(_ context.Context, config NamespaceManagerConfig) ([]string, error) {
func NamespacesGenerator(ctx context.Context, config NamespaceManagerConfig) ([]string, error) {
if config == nil {
return nil, errors.New("namespaces manager config is nil")
}
Expand All @@ -126,9 +126,9 @@ func NamespacesGenerator(_ context.Context, config NamespaceManagerConfig) ([]st
return nil, errors.Wrap(err, "retrieve namespaces client")
}

providedNamespaces := namespaceClient.ProvidedNamespaces()
providedNamespaces := namespaceClient.ProvidedNamespaces(ctx)

names, err := namespaceClient.Names()
names, err := namespaceClient.Names(ctx)
if err != nil {
initialNamespace := namespaceClient.InitialNamespace()
names = []string{initialNamespace}
Expand Down
6 changes: 4 additions & 2 deletions internal/api/namespaces_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ func TestNamespacesManager_GenerateNamespaces(t *testing.T) {
}

func TestNamespacesGenerator(t *testing.T) {
ctx := context.Background()

tests := []struct {
name string
setup func(controller *gomock.Controller) *configFake.MockDash
Expand All @@ -57,8 +59,8 @@ func TestNamespacesGenerator(t *testing.T) {
namespaces := []string{"ns-1"}

namespaceClient := clusterFake.NewMockNamespaceInterface(controller)
namespaceClient.EXPECT().Names().Return(namespaces, nil)
namespaceClient.EXPECT().ProvidedNamespaces().Return([]string{})
namespaceClient.EXPECT().Names(ctx).Return(namespaces, nil)
namespaceClient.EXPECT().ProvidedNamespaces(ctx).Return([]string{})

clusterClient := clusterFake.NewMockClientInterface(controller)
clusterClient.EXPECT().NamespaceClient().Return(namespaceClient, nil)
Expand Down
2 changes: 1 addition & 1 deletion internal/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func (c *Cluster) NamespaceClient() (clusterTypes.NamespaceInterface, error) {
if err != nil {
return nil, errors.Wrap(err, "resolving initial namespace")
}
return newNamespaceClient(dc, rc, ns, c.providedNamespaces), nil
return newNamespaceClient(dc, rc, c.kubernetesClient, ns, c.providedNamespaces), nil
}

// DynamicClient returns a dynamic client.
Expand Down
25 changes: 13 additions & 12 deletions internal/cluster/fake/mock_namespace_interface.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 57 additions & 16 deletions internal/cluster/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ package cluster

import (
"context"
"fmt"

authv1 "k8s.io/api/authorization/v1"
"k8s.io/client-go/kubernetes"
authClient "k8s.io/client-go/kubernetes/typed/authorization/v1"

internalLog "github.com/vmware-tanzu/octant/internal/log"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
Expand All @@ -22,23 +29,29 @@ import (
type namespaceClient struct {
restClient rest.Interface
dynamicClient dynamic.Interface
authClient authClient.AuthorizationV1Interface
initialNamespace string
providedNamespaces []string
}

var _ clusterTypes.NamespaceInterface = (*namespaceClient)(nil)

func newNamespaceClient(dynamicClient dynamic.Interface, restClient rest.Interface, initialNamespace string, providedNamespaces []string) *namespaceClient {
func newNamespaceClient(dynamicClient dynamic.Interface, restClient rest.Interface, kubernetesClient kubernetes.Interface, initialNamespace string, providedNamespaces []string) *namespaceClient {
var authClient authClient.AuthorizationV1Interface
if kubernetesClient != nil {
authClient = kubernetesClient.AuthorizationV1()
}
return &namespaceClient{
restClient: restClient,
dynamicClient: dynamicClient,
authClient: authClient,
initialNamespace: initialNamespace,
providedNamespaces: providedNamespaces,
}
}

func (n *namespaceClient) Names() ([]string, error) {
namespaces, err := namespaces(n.dynamicClient)
func (n *namespaceClient) Names(ctx context.Context) ([]string, error) {
namespaces, err := n.namespaces(ctx, n.dynamicClient)
if err != nil {
return nil, err
}
Expand All @@ -51,33 +64,61 @@ func (n *namespaceClient) Names() ([]string, error) {
return names, nil
}

func (n *namespaceClient) HasNamespace(namespace string) bool {
func (n *namespaceClient) HasNamespace(ctx context.Context, namespace string) bool {
ns := &corev1.Namespace{}
err := n.restClient.Get().Resource("namespaces").Name(namespace).Do(context.TODO()).Into(ns)
err := n.restClient.Get().Resource("namespaces").Name(namespace).Do(ctx).Into(ns)
if err != nil {
return false
}
return true
}

// Namespaces returns available namespaces.
func namespaces(dc dynamic.Interface) ([]corev1.Namespace, error) {
func (n *namespaceClient) namespaces(ctx context.Context, dc dynamic.Interface) ([]corev1.Namespace, error) {
logger := internalLog.From(ctx)

res := schema.GroupVersionResource{
Version: "v1",
Resource: "namespaces",
}

nri := dc.Resource(res)
var nsList corev1.NamespaceList

list, err := nri.List(context.TODO(), metav1.ListOptions{})
if err != nil {
return nil, errors.Wrap(err, "list namespaces")
ssar := &authv1.SelfSubjectAccessReview{
Spec: authv1.SelfSubjectAccessReviewSpec{
ResourceAttributes: &authv1.ResourceAttributes{
Verb: "list",
Version: "v1",
Resource: "namespaces",
},
},
}

var nsList corev1.NamespaceList
err = runtime.DefaultUnstructuredConverter.FromUnstructured(list.UnstructuredContent(), &nsList)
if err != nil {
return nil, errors.Wrap(err, "convert object to namespace list")
var resp *authv1.SelfSubjectAccessReview
var err error
if n.authClient != nil {
resp, err = n.authClient.SelfSubjectAccessReviews().Create(ctx, ssar, metav1.CreateOptions{})
if err != nil {
return nil, fmt.Errorf("auth client: %w", err)
}
}

if resp == nil || resp.Status.Allowed {
nri := dc.Resource(res)

list, err := nri.List(ctx, metav1.ListOptions{})
if err != nil {
return nil, errors.Wrap(err, "list namespaces")
}

err = runtime.DefaultUnstructuredConverter.FromUnstructured(list.UnstructuredContent(), &nsList)
if err != nil {
return nil, errors.Wrap(err, "convert object to namespace list")
}
}

if len(nsList.Items) == 0 {
logger.Debugf("no namespaces found")
}

return nsList.Items, nil
Expand All @@ -89,8 +130,8 @@ func (n *namespaceClient) InitialNamespace() string {
}

// ProvidedNamespaces returns the list of namespaces provided.
// If no namespaces are provided, it will default to returning the InitialNamespace
func (n *namespaceClient) ProvidedNamespaces() []string {
// If no namespaces are provided, it will default to returning the InitialNamespace
func (n *namespaceClient) ProvidedNamespaces(ctx context.Context) []string {
if len(n.providedNamespaces) == 0 {
n.providedNamespaces = []string{n.initialNamespace}
}
Expand Down
19 changes: 11 additions & 8 deletions internal/cluster/namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ SPDX-License-Identifier: Apache-2.0
package cluster

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -17,6 +18,7 @@ import (
)

func Test_namespaceClient_Names(t *testing.T) {
ctx := context.Background()
scheme := runtime.NewScheme()

dc := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme,
Expand All @@ -27,31 +29,32 @@ func Test_namespaceClient_Names(t *testing.T) {
newUnstructured("v1", "Namespace", "app-1", "app-1"),
)

nc := newNamespaceClient(dc, nil, "default", []string{})
nc := newNamespaceClient(dc, nil, nil, "default", []string{})

got, err := nc.Names()
got, err := nc.Names(ctx)
require.NoError(t, err)

expected := []string{"app-1", "default"}
assert.Equal(t, expected, got)
}

func Test_namespaceClient_providedNamespaces(t *testing.T) {
ctx := context.Background()
providedNamespaces := []string{"default", "user-1"}

scheme := runtime.NewScheme()
dc := dynamicfake.NewSimpleDynamicClient(scheme)
nc := newNamespaceClient(dc, nil, "default", providedNamespaces)
nc := newNamespaceClient(dc, nil, nil, "default", providedNamespaces)

assert.Equal(t, providedNamespaces, nc.ProvidedNamespaces())
assert.Equal(t, providedNamespaces, nc.ProvidedNamespaces(ctx))

nc = newNamespaceClient(dc, nil, "default", []string{})
assert.Equal(t, nc.ProvidedNamespaces(), []string{"default"})
nc = newNamespaceClient(dc, nil, nil, "default", []string{})
assert.Equal(t, nc.ProvidedNamespaces(ctx), []string{"default"})
}

func Test_namespaceClient_InitialNamespace(t *testing.T) {
expected := "inital-namespace"
nc := newNamespaceClient(nil, nil, expected, []string{})
expected := "initial-namespace"
nc := newNamespaceClient(nil, nil, nil, expected, []string{})
assert.Equal(t, expected, nc.InitialNamespace())
}

Expand Down
Loading

0 comments on commit 3c89689

Please sign in to comment.