Remove default token from all service accounts

CHANGELOG.md

@@ -7,8 +7,9 @@
   * Delete spec.priority in pod restore action (#879, @mwieczorek)
   * Added brew reference (#1051, @omerlh)
   * Update to go 1.11 (#1069, @gliptak)
-  * Initialize empty schedule metrics on server init (#1054, @cbeneke) 
+  * Initialize empty schedule metrics on server init (#1054, @cbeneke)
   * Update CHANGELOGs (#1063, @wwitzel3)
+  * Remove default token from all service accounts (#1048, @ncdc)
 
 ## Current release:
   * [CHANGELOG-0.10.md][8]

pkg/cmd/server/plugin/plugin.go

@@ -54,6 +54,7 @@ func NewCommand(f client.Factory) *cobra.Command {
 				RegisterRestoreItemAction("pod", newPodRestoreItemAction).
 				RegisterRestoreItemAction("restic", newResticRestoreItemAction).
 				RegisterRestoreItemAction("service", newServiceRestoreItemAction).
+				RegisterRestoreItemAction("serviceaccount", newServiceAccountRestoreItemAction).
 				Serve()
 		},
 	}
@@ -133,3 +134,7 @@ func newResticRestoreItemAction(logger logrus.FieldLogger) (interface{}, error)
 func newServiceRestoreItemAction(logger logrus.FieldLogger) (interface{}, error) {
 	return restore.NewServiceAction(logger), nil
 }
+
+func newServiceAccountRestoreItemAction(logger logrus.FieldLogger) (interface{}, error) {
+	return restore.NewServiceAccountAction(logger), nil
+}

pkg/restore/merge_service_account.go

@@ -17,7 +17,6 @@ package restore
 
 import (
 	"encoding/json"
-	"strings"
 
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/pkg/errors"
@@ -30,7 +29,6 @@ import (
 )
 
 // mergeServiceAccount takes a backed up serviceaccount and merges attributes into the current in-cluster service account.
-// The default token secret from the backed up serviceaccount will be ignored in favor of the one already present.
 // Labels and Annotations on the backed up version but not on the in-cluster version will be merged. If a key is specified in both, the in-cluster version is retained.
 func mergeServiceAccounts(fromCluster, fromBackup *unstructured.Unstructured) (*unstructured.Unstructured, error) {
 	desired := new(corev1api.ServiceAccount)
@@ -44,15 +42,6 @@ func mergeServiceAccounts(fromCluster, fromBackup *unstructured.Unstructured) (*
 		return nil, errors.Wrap(err, "unable to convert from backed up service account unstructured to serviceaccount")
 	}
 
-	for i := len(backupSA.Secrets) - 1; i >= 0; i-- {
-		secret := &backupSA.Secrets[i]
-		if strings.HasPrefix(secret.Name, backupSA.Name+"-token-") {
-			// Copy all secrets *except* -token-
-			backupSA.Secrets = append(backupSA.Secrets[:i], backupSA.Secrets[i+1:]...)
-			break
-		}
-	}
-
 	desired.Secrets = mergeObjectReferenceSlices(desired.Secrets, backupSA.Secrets)
 
 	desired.ImagePullSecrets = mergeLocalObjectReferenceSlices(desired.ImagePullSecrets, backupSA.ImagePullSecrets)

pkg/restore/merge_service_account_test.go

@@ -21,6 +21,7 @@ import (
 	"unicode"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	corev1api "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
@@ -503,7 +504,7 @@ func TestMergeServiceAccountBasic(t *testing.T) {
 		expectedErr bool
 	}{
 		{
-			name: "only default tokens present",
+			name: "only default token",
 			fromCluster: arktest.UnstructuredOrDie(
 				`{
 					"apiVersion": "v1",
@@ -517,6 +518,8 @@ func TestMergeServiceAccountBasic(t *testing.T) {
 					]
 				}`,
 			),
+			// fromBackup doesn't have the default token because it is expected to already have been removed
+			// by the service account action
 			fromBackup: arktest.UnstructuredOrDie(
 				`{
 					"kind": "ServiceAccount",
@@ -525,9 +528,7 @@ func TestMergeServiceAccountBasic(t *testing.T) {
 						"namespace": "ns1",
 						"name": "default"
 					},
-					"secrets": [
-						{ "name": "default-token-xzy12" }
-					]
+					"secrets": []
 				}`,
 			),
 			expectedRes: arktest.UnstructuredOrDie(
@@ -561,7 +562,8 @@ func TestMergeServiceAccountBasic(t *testing.T) {
 					]
 				}`,
 			),
-
+			// fromBackup doesn't have the default token because it is expected to already have been removed
+			// by the service account action
 			fromBackup: arktest.UnstructuredOrDie(
 				`{
 					"kind": "ServiceAccount",
@@ -571,7 +573,6 @@ func TestMergeServiceAccountBasic(t *testing.T) {
 						"name": "default"
 					},
 					"secrets": [
-						{ "name": "default-token-xzy12" },
 						{ "name": "my-old-secret" },
 						{ "name": "secrete"}
 					]
@@ -621,7 +622,8 @@ func TestMergeServiceAccountBasic(t *testing.T) {
 					]
 				}`,
 			),
-
+			// fromBackup doesn't have the default token because it is expected to already have been removed
+			// by the service account action
 			fromBackup: arktest.UnstructuredOrDie(
 				`{
 					"kind": "ServiceAccount",
@@ -645,9 +647,7 @@ func TestMergeServiceAccountBasic(t *testing.T) {
 							"a6": "v6"
 						}
 					},
-					"secrets": [
-						{ "name": "default-token-xzy12" }
-					]
+					"secrets": []
 				}`,
 			),
 			expectedRes: arktest.UnstructuredOrDie(
@@ -682,11 +682,10 @@ func TestMergeServiceAccountBasic(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		t.Run(test.name, func(b *testing.T) {
+		t.Run(test.name, func(t *testing.T) {
 			result, err := mergeServiceAccounts(test.fromCluster, test.fromBackup)
-			if err != nil {
-				assert.Equal(t, test.expectedRes, result)
-			}
+			require.NoError(t, err)
+			assert.Equal(t, test.expectedRes, result)
 		})
 	}
 }

pkg/restore/service_account_action.go

@@ -0,0 +1,79 @@
+/*
+Copyright 2018 the Heptio Ark contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package restore
+
+import (
+	"strings"
+
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	api "github.com/heptio/ark/pkg/apis/ark/v1"
+	"github.com/heptio/ark/pkg/util/kube"
+)
+
+type serviceAccountAction struct {
+	logger logrus.FieldLogger
+}
+
+func NewServiceAccountAction(logger logrus.FieldLogger) ItemAction {
+	return &serviceAccountAction{logger: logger}
+}
+
+func (a *serviceAccountAction) AppliesTo() (ResourceSelector, error) {
+	return ResourceSelector{
+		IncludedResources: []string{"serviceaccounts"},
+	}, nil
+}
+
+func (a *serviceAccountAction) Execute(obj runtime.Unstructured, restore *api.Restore) (runtime.Unstructured, error, error) {
+	a.logger.Info("Executing serviceAccountAction")
+	defer a.logger.Info("Done executing serviceAccountAction")
+
+	var serviceAccount corev1.ServiceAccount
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &serviceAccount); err != nil {
+		return nil, nil, errors.Wrap(err, "unable to convert serviceaccount from runtime.Unstructured")
+	}
+
+	log := a.logger.WithField("serviceaccount", kube.NamespaceAndName(&serviceAccount))
+
+	log.Debug("Checking secrets")
+	check := serviceAccount.Name + "-token-"
+	for i := len(serviceAccount.Secrets) - 1; i >= 0; i-- {
+		secret := &serviceAccount.Secrets[i]
+		log.Debugf("Checking if secret %s matches %s", secret.Name, check)
+
+		if strings.HasPrefix(secret.Name, check) {
+			// Copy all secrets *except* -token-
+			log.Debug("Match found - excluding this secret")
+			serviceAccount.Secrets = append(serviceAccount.Secrets[:i], serviceAccount.Secrets[i+1:]...)
+			break
+		} else {
+			log.Debug("No match found - including this secret")
+		}
+	}
+
+	res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&serviceAccount)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "unable to convert serviceaccount to runtime.Unstructured")
+	}
+
+	return &unstructured.Unstructured{Object: res}, nil, nil
+}

pkg/restore/service_account_action_test.go

@@ -0,0 +1,111 @@
+/*
+Copyright 2018 the Heptio Ark contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package restore
+
+import (
+	"sort"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	"github.com/heptio/ark/pkg/util/test"
+)
+
+func TestServiceAccountActionAppliesTo(t *testing.T) {
+	action := NewServiceAccountAction(test.NewLogger())
+	actual, err := action.AppliesTo()
+	require.NoError(t, err)
+	assert.Equal(t, ResourceSelector{IncludedResources: []string{"serviceaccounts"}}, actual)
+}
+
+func TestServiceAccountActionExecute(t *testing.T) {
+	tests := []struct {
+		name     string
+		secrets  []string
+		expected []string
+	}{
+		{
+			name:     "no secrets",
+			secrets:  []string{},
+			expected: []string{},
+		},
+		{
+			name:     "no match",
+			secrets:  []string{"a", "bar-TOKN-nomatch", "baz"},
+			expected: []string{"a", "bar-TOKN-nomatch", "baz"},
+		},
+		{
+			name:     "match - first",
+			secrets:  []string{"bar-token-a1b2c", "a", "baz"},
+			expected: []string{"a", "baz"},
+		},
+		{
+			name:     "match - middle",
+			secrets:  []string{"a", "bar-token-a1b2c", "baz"},
+			expected: []string{"a", "baz"},
+		},
+		{
+			name:     "match - end",
+			secrets:  []string{"a", "baz", "bar-token-a1b2c"},
+			expected: []string{"a", "baz"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			sa := corev1.ServiceAccount{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "foo",
+					Name:      "bar",
+				},
+			}
+
+			for _, secret := range tc.secrets {
+				sa.Secrets = append(sa.Secrets, corev1.ObjectReference{
+					Name: secret,
+				})
+			}
+
+			saUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&sa)
+			require.NoError(t, err)
+
+			action := NewServiceAccountAction(test.NewLogger())
+			res, warning, err := action.Execute(&unstructured.Unstructured{Object: saUnstructured}, nil)
+			require.NoError(t, warning)
+			require.NoError(t, err)
+
+			var resSA *corev1.ServiceAccount
+			err = runtime.DefaultUnstructuredConverter.FromUnstructured(res.UnstructuredContent(), &resSA)
+			require.NoError(t, err)
+
+			actual := []string{}
+			for _, secret := range resSA.Secrets {
+				actual = append(actual, secret.Name)
+			}
+
+			sort.Strings(tc.expected)
+			sort.Strings(actual)
+
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}