Skip to content

Commit

Permalink
Remove default token from all service accounts
Browse files Browse the repository at this point in the history
Instead of only removing the default token from a service account when
it already exists in the cluster, always remove it. If the service
account already exists, continue to do the merging logic.

Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>
  • Loading branch information
ncdc committed Nov 9, 2018
1 parent 449cac5 commit b557ea1
Show file tree
Hide file tree
Showing 5 changed files with 207 additions and 25 deletions.
5 changes: 5 additions & 0 deletions pkg/cmd/server/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func NewCommand(f client.Factory) *cobra.Command {
RegisterRestoreItemAction("pod", newPodRestoreItemAction).
RegisterRestoreItemAction("restic", newResticRestoreItemAction).
RegisterRestoreItemAction("service", newServiceRestoreItemAction).
RegisterRestoreItemAction("serviceaccount", newServiceAccountRestoreItemAction).
Serve()
},
}
Expand Down Expand Up @@ -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
}
11 changes: 0 additions & 11 deletions pkg/restore/merge_service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package restore

import (
"encoding/json"
"strings"

jsonpatch "github.com/evanphx/json-patch"
"github.com/pkg/errors"
Expand All @@ -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)
Expand All @@ -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)
Expand Down
27 changes: 13 additions & 14 deletions pkg/restore/merge_service_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -525,9 +528,7 @@ func TestMergeServiceAccountBasic(t *testing.T) {
"namespace": "ns1",
"name": "default"
},
"secrets": [
{ "name": "default-token-xzy12" }
]
"secrets": []
}`,
),
expectedRes: arktest.UnstructuredOrDie(
Expand Down Expand Up @@ -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",
Expand All @@ -571,7 +573,6 @@ func TestMergeServiceAccountBasic(t *testing.T) {
"name": "default"
},
"secrets": [
{ "name": "default-token-xzy12" },
{ "name": "my-old-secret" },
{ "name": "secrete"}
]
Expand Down Expand Up @@ -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",
Expand All @@ -645,9 +647,7 @@ func TestMergeServiceAccountBasic(t *testing.T) {
"a6": "v6"
}
},
"secrets": [
{ "name": "default-token-xzy12" }
]
"secrets": []
}`,
),
expectedRes: arktest.UnstructuredOrDie(
Expand Down Expand Up @@ -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)
})
}
}
79 changes: 79 additions & 0 deletions pkg/restore/service_account_action.go
Original file line number Diff line number Diff line change
@@ -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
}
110 changes: 110 additions & 0 deletions pkg/restore/service_account_action_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
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/heptio/ark/pkg/util/test"
"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"
)

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)
})
}
}

0 comments on commit b557ea1

Please sign in to comment.