Skip to content

Commit

Permalink
Merge pull request #1048 from ncdc/remove-default-token-from-all-serv…
Browse files Browse the repository at this point in the history
…ice-accounts

Remove default token from all service accounts
  • Loading branch information
skriss authored Dec 4, 2018
2 parents a4a09f0 + 62d8c64 commit 555f73c
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 26 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
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
}
111 changes: 111 additions & 0 deletions pkg/restore/service_account_action_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}

0 comments on commit 555f73c

Please sign in to comment.