Repository: vmware-tanzu/octant
Pull request #905 (2020-05-02 13:40:34)

Show service account secrets
**What this PR does / why we need it**:
This change shows service account secrets. Additionally, it shows the policy table for a service account.

**Release note**:
```
Show secrets and policies associated with a service account
```



Patch:
diff --git a/internal/printer/clusterrole.go b/internal/printer/clusterrole.go
index 5b703b18..11d99309 100644
--- a/internal/printer/clusterrole.go
+++ b/internal/printer/clusterrole.go
@@ -115,7 +115,7 @@ func createClusterRolePolicyRulesView(clusterRole *rbacv1.ClusterRole) (*compone
 }
 
 func printPolicyRules(rules []rbacv1.PolicyRule) (*component.Table, error) {
-	breakdownRules := []rbacv1.PolicyRule{}
+	var breakdownRules []rbacv1.PolicyRule
 	for _, rule := range rules {
 		breakdownRules = append(breakdownRules, BreakdownRule(rule)...)
 	}
diff --git a/internal/printer/serviceaccount.go b/internal/printer/serviceaccount.go
index 64b7a353..68f69e87 100644
--- a/internal/printer/serviceaccount.go
+++ b/internal/printer/serviceaccount.go
@@ -12,6 +12,7 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
 
@@ -49,17 +50,19 @@ func ServiceAccountListHandler(_ context.Context, list *corev1.ServiceAccountLis
 
 type serviceAccountObject interface {
 	Config(ctx context.Context, options Options) error
-	PolicyRules(ctx context.Context, serviceAccount *corev1.ServiceAccount, options Options) error
+	PolicyRules(ctx context.Context, options Options) error
+	Secrets(ctx context.Context, options Options) error
 }
 
 type serviceAccountHandler struct {
 	serviceAccount  *corev1.ServiceAccount
 	configFunc      func(context.Context, *corev1.ServiceAccount, Options) (*component.Summary, error)
 	policyRulesFunc func(context.Context, *corev1.ServiceAccount, Options) (*component.Table, error)
+	secretsFunc     func(context.Context, *corev1.ServiceAccount, Options) (*component.Table, error)
 	object          *Object
 }
 
-var _serviceAccountObject = (*serviceAccountHandler)(nil)
+var _ serviceAccountObject = (*serviceAccountHandler)(nil)
 
 func newServiceAccountHandler(serviceAccount *corev1.ServiceAccount, object *Object) (*serviceAccountHandler, error) {
 	if serviceAccount == nil {
@@ -74,6 +77,7 @@ func newServiceAccountHandler(serviceAccount *corev1.ServiceAccount, object *Obj
 		serviceAccount:  serviceAccount,
 		configFunc:      defaultServiceAccountConfig,
 		policyRulesFunc: defaultServiceAccountPolicyRules,
+		secretsFunc:     ServiceAccountSecrets,
 		object:          object,
 	}
 	return s, nil
@@ -88,6 +92,80 @@ func (s *serviceAccountHandler) Config(ctx context.Context, options Options) err
 	return nil
 }
 
+func (s *serviceAccountHandler) Secrets(ctx context.Context, options Options) error {
+	table, err := s.secretsFunc(ctx, s.serviceAccount, options)
+	if err != nil {
+		return err
+	}
+
+	if table == nil {
+		return nil
+	}
+
+	s.object.RegisterItems(ItemDescriptor{
+		Func: func() (component.Component, error) {
+			return table, nil
+		},
+		Width: component.WidthFull,
+	})
+
+	return nil
+}
+
+// ServiceAccountSecretCols are columns for a service account secrets table.
+var ServiceAccountSecretCols = component.NewTableCols("Name", "Type")
+
+// ServiceAccountSecretPlaceholder is the table placeholder for service account secrets.
+const ServiceAccountSecretPlaceholder = "No secrets"
+
+// ServiceAccountSecrets generates a service account secrets table. It will return a nil
+// table if there are no secrets.
+func ServiceAccountSecrets(
+	ctx context.Context,
+	account *corev1.ServiceAccount,
+	options Options) (*component.Table, error) {
+	if account == nil {
+		return nil, fmt.Errorf("service account is nil")
+	}
+
+	if len(account.Secrets) == 0 {
+		return nil, nil
+	}
+
+	table := component.NewTable("Secrets", ServiceAccountSecretPlaceholder, ServiceAccountSecretCols)
+
+	for _, ref := range account.Secrets {
+		key := store.Key{
+			Namespace:  account.Namespace,
+			APIVersion: "v1",
+			Kind:       "Secret",
+			Name:       ref.Name,
+			Selector:   nil,
+		}
+		secret, err := options.DashConfig.ObjectStore().Get(ctx, key)
+		if err != nil {
+			return nil, fmt.Errorf("unable to get secret %s: %w", key, err)
+		}
+		secretType, _, err := unstructured.NestedString(secret.Object, "type")
+		if err != nil {
+			return nil, fmt.Errorf("get secret type: %w", err)
+		}
+
+		secretLink, err := options.Link.ForObject(secret, secret.GetName())
+		if err != nil {
+			return nil, fmt.Errorf("generate link for object ref: %w", err)
+		}
+
+		row := component.TableRow{
+			"Name": secretLink,
+			"Type": component.NewText(secretType),
+		}
+		table.Add(row)
+	}
+
+	return table, nil
+}
+
 // ServiceAccountHandler is a printFunc that prints ServiceAccounts
 func ServiceAccountHandler(ctx context.Context, serviceAccount *corev1.ServiceAccount, options Options) (component.Component, error) {
 	o := NewObject(serviceAccount)
@@ -102,6 +180,14 @@ func ServiceAccountHandler(ctx context.Context, serviceAccount *corev1.ServiceAc
 		return nil, errors.Wrap(err, "print service account configuration")
 	}
 
+	if err := s.Secrets(ctx, options); err != nil {
+		return nil, fmt.Errorf("print service account secrets")
+	}
+
+	if err := s.PolicyRules(ctx, options); err != nil {
+		return nil, fmt.Errorf("print policy results")
+	}
+
 	return o.ToComponent(ctx, options)
 }
 
@@ -146,19 +232,6 @@ func (s *ServiceAccountConfiguration) Create(options Options) (*component.Summar
 		sections.Add("Image Pull Secrets", view)
 	}
 
-	var mountSecrets []string
-	for _, s := range serviceAccount.Secrets {
-		mountSecrets = append(mountSecrets, s.Name)
-	}
-
-	if len(mountSecrets) > 0 {
-		view, err := generateServiceAccountSecretsList(serviceAccount.Namespace, mountSecrets, options)
-		if err != nil {
-			return nil, err
-		}
-		sections.Add("Mountable Secrets", view)
-	}
-
 	tokens, err := serviceAccountTokens(s.context, *serviceAccount, s.objectStore)
 
 	if err != nil {
@@ -410,11 +483,11 @@ func defaultServiceAccountConfig(ctx context.Context, serviceAccount *corev1.Ser
 	return NewServiceAccountConfiguration(ctx, serviceAccount, options).Create(options)
 }
 
-func (s *serviceAccountHandler) PolicyRules(ctx context.Context, serviceAccount *corev1.ServiceAccount, options Options) error {
+func (s *serviceAccountHandler) PolicyRules(ctx context.Context, options Options) error {
 	s.object.RegisterItems(ItemDescriptor{
 		Width: component.WidthFull,
 		Func: func() (component.Component, error) {
-			return s.policyRulesFunc(ctx, serviceAccount, options)
+			return s.policyRulesFunc(ctx, s.serviceAccount, options)
 		},
 	})
 


Test patch:
diff --git a/internal/printer/serviceaccount_test.go b/internal/printer/serviceaccount_test.go
index c4676a32..08d64303 100644
--- a/internal/printer/serviceaccount_test.go
+++ b/internal/printer/serviceaccount_test.go
@@ -1,7 +1,7 @@
 /*
-Copyright (c) 2019 the Octant contributors. All Rights Reserved.
-SPDX-License-Identifier: Apache-2.0
-*/
+ * Copyright (c) 2020 the Octant contributors. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 package printer
 
@@ -15,8 +15,11 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	configFake "github.com/vmware-tanzu/octant/internal/config/fake"
+	linkFake "github.com/vmware-tanzu/octant/internal/link/fake"
 	"github.com/vmware-tanzu/octant/internal/testutil"
 	"github.com/vmware-tanzu/octant/pkg/store"
+	storeFake "github.com/vmware-tanzu/octant/pkg/store/fake"
 	"github.com/vmware-tanzu/octant/pkg/view/component"
 )
 
@@ -79,18 +82,18 @@ func Test_ServiceAccountConfiguration(t *testing.T) {
 		Kind:       "Secret",
 	}
 
-	cases := []struct {
+	tests := []struct {
 		name           string
 		namespace      string
-		serviceaccount *corev1.ServiceAccount
+		serviceAccount *corev1.ServiceAccount
 		secret         *corev1.Secret
 		isErr          bool
 		expected       *component.Summary
 	}{
 		{
-			name:           "serviceaccount",
+			name:           "in general",
 			namespace:      serviceAccount.Namespace,
-			serviceaccount: serviceAccount,
+			serviceAccount: serviceAccount,
 			expected: component.NewSummary("Configuration", []component.SummarySection{
 				{
 					Header: "Image Pull Secrets",
@@ -98,12 +101,6 @@ func Test_ServiceAccountConfiguration(t *testing.T) {
 						component.NewLink("", "secret", "/secret"),
 					}),
 				},
-				{
-					Header: "Mountable Secrets",
-					Content: component.NewList([]component.TitleComponent{}, []component.Component{
-						component.NewLink("", "secret", "/secret"),
-					}),
-				},
 				{
 					Header: "Tokens",
 					Content: component.NewList([]component.TitleComponent{}, []component.Component{
@@ -113,38 +110,38 @@ func Test_ServiceAccountConfiguration(t *testing.T) {
 			}...),
 		},
 		{
-			name:           "serviceaccount is nil",
-			serviceaccount: nil,
+			name:           "service account is nil",
+			serviceAccount: nil,
 			isErr:          true,
 		},
 	}
 
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
 			controller := gomock.NewController(t)
 			defer controller.Finish()
 
 			ctx := context.Background()
 			tpo := newTestPrinterOptions(controller)
 
-			if !tc.isErr {
+			if !test.isErr {
 				tpo.objectStore.EXPECT().List(gomock.Any(), gomock.Eq(key)).
 					Return(testutil.ToUnstructuredList(t, testutil.ToUnstructured(t, secret)), false, nil)
 			}
 
 			printOptions := tpo.ToOptions()
 
-			tpo.PathForGVK(tc.namespace, "v1", "Secret", "secret", "secret", "/secret")
+			tpo.PathForGVK(test.namespace, "v1", "Secret", "secret", "secret", "/secret")
 
-			sac := NewServiceAccountConfiguration(ctx, tc.serviceaccount, printOptions)
+			sac := NewServiceAccountConfiguration(ctx, test.serviceAccount, printOptions)
 			summary, err := sac.Create(printOptions)
-			if tc.isErr {
+			if test.isErr {
 				require.Error(t, err)
 				return
 			}
 			require.NoError(t, err)
 
-			component.AssertEqual(t, tc.expected, summary)
+			component.AssertEqual(t, test.expected, summary)
 		})
 	}
 }
@@ -219,8 +216,8 @@ func Test_ServiceAccountPolicyRules(t *testing.T) {
 		Get(gomock.Any(), role2Key).
 		Return(testutil.ToUnstructured(t, role2), nil)
 
-	saph := NewServiceAccountPolicyRules(ctx, serviceAccount, printOptions)
-	got, err := saph.Create()
+	policyRules := NewServiceAccountPolicyRules(ctx, serviceAccount, printOptions)
+	got, err := policyRules.Create()
 	require.NoError(t, err)
 
 	cols := component.NewTableCols("Resources", "Non-Resource URLs", "Resource Names", "Verbs")
@@ -242,3 +239,97 @@ func Test_ServiceAccountPolicyRules(t *testing.T) {
 
 	component.AssertEqual(t, expected, got)
 }
+
+func TestServiceAccountSecrets(t *testing.T) {
+	tests := []struct {
+		name        string
+		account     *corev1.ServiceAccount
+		initOptions func(controller *gomock.Controller) Options
+		want        *component.Table
+		wantError   bool
+	}{
+		{
+			name:    "service account with no secrets",
+			account: testutil.CreateServiceAccount("sa"),
+			initOptions: func(controller *gomock.Controller) Options {
+				return Options{}
+			},
+			want: nil,
+		},
+		{
+			name: "service account with secret",
+			account: testutil.CreateServiceAccount("sa", func(account *corev1.ServiceAccount) {
+				account.Secrets = []corev1.ObjectReference{
+					{
+						Name: "secret",
+					},
+				}
+			}),
+			initOptions: func(ctrl *gomock.Controller) Options {
+				secret := testutil.ToUnstructured(t, testutil.CreateSecret("secret", func(secret *corev1.Secret) {
+					secret.Type = corev1.SecretTypeOpaque
+				}))
+				secretKey, err := store.KeyFromObject(secret)
+				require.NoError(t, err)
+
+				objectStore := storeFake.NewMockStore(ctrl)
+				objectStore.EXPECT().
+					Get(gomock.Any(), secretKey).
+					Return(secret, nil)
+
+				secretLink := component.NewLink("", "secret", "/secret")
+				link := linkFake.NewMockInterface(ctrl)
+				link.EXPECT().
+					ForObject(secret, secret.GetName()).
+					Return(secretLink, nil)
+
+				dashConfig := configFake.NewMockDash(ctrl)
+				dashConfig.EXPECT().ObjectStore().Return(objectStore).AnyTimes()
+
+				options := Options{
+					DashConfig: dashConfig,
+					Link:       link,
+				}
+
+				return options
+			},
+			want: component.NewTableWithRows(
+				"Secrets",
+				ServiceAccountSecretPlaceholder,
+				ServiceAccountSecretCols,
+				[]component.TableRow{
+					{
+						"Name": component.NewLink("", "secret", "/secret"),
+						"Type": component.NewText(string(corev1.SecretTypeOpaque)),
+					},
+				}),
+		},
+		{
+			name:      "with nil service account",
+			account:   nil,
+			wantError: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			ctx := context.Background()
+
+			var options Options
+			if test.initOptions != nil {
+				options = test.initOptions(ctrl)
+			}
+
+			actual, err := ServiceAccountSecrets(ctx, test.account, options)
+			if test.wantError {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+
+			testutil.AssertJSONEqual(t, test.want, actual)
+		})
+	}
+}
diff --git a/internal/testutil/generator.go b/internal/testutil/generator.go
index 3e802d79..c724b5e0 100644
--- a/internal/testutil/generator.go
+++ b/internal/testutil/generator.go
@@ -282,11 +282,17 @@ func CreateExtReplicaSet(name string) *extv1beta1.ReplicaSet {
 }
 
 // CreateSecret creates a secret
-func CreateSecret(name string) *corev1.Secret {
-	return &corev1.Secret{
+func CreateSecret(name string, options ...func(*corev1.Secret)) *corev1.Secret {
+	s := &corev1.Secret{
 		TypeMeta:   genTypeMeta(gvk.Secret),
 		ObjectMeta: genObjectMeta(name, true),
 	}
+
+	for _, option := range options {
+		option(s)
+	}
+
+	return s
 }
 
 // CreateService creates a service
@@ -298,11 +304,17 @@ func CreateService(name string) *corev1.Service {
 }
 
 // CreateServiceAccount creates a service account
-func CreateServiceAccount(name string) *corev1.ServiceAccount {
-	return &corev1.ServiceAccount{
+func CreateServiceAccount(name string, options ...func(*corev1.ServiceAccount)) *corev1.ServiceAccount {
+	sa := &corev1.ServiceAccount{
 		TypeMeta:   genTypeMeta(gvk.ServiceAccount),
 		ObjectMeta: genObjectMeta(name, true),
 	}
+
+	for _, option := range options {
+		option(sa)
+	}
+
+	return sa
 }
 
 // CreateStatefulSet creates a stateful set
diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go
index 4c20260f..9058a184 100644
--- a/internal/testutil/testutil.go
+++ b/internal/testutil/testutil.go
@@ -131,3 +131,4 @@ func RequireErrorOrNot(t *testing.T, wantErr bool, err error) {
 	}
 	require.NoError(t, err)
 }
+