Add merge support for serviceaccounts

All properties from a backup will be merged into the ServiceAccount
except for the default token secret.

Signed-off-by: Nolan Brubaker <nolan@heptio.com>

pkg/client/dynamic.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/dynamic"
 )
@@ -82,12 +83,20 @@ type Getter interface {
 	Get(name string, opts metav1.GetOptions) (*unstructured.Unstructured, error)
 }
 
+// Patcher patches an object.
+type Patcher interface {
+	//Patch patches the named object using the provided patch bytes, which are expected to be in JSON merge patch format. The patched object is returned.
+
+	Patch(name string, data []byte) (*unstructured.Unstructured, error)
+}
+
 // Dynamic contains client methods that Ark needs for backing up and restoring resources.
 type Dynamic interface {
 	Creator
 	Lister
 	Watcher
 	Getter
+	Patcher
 }
 
 // dynamicResourceClient implements Dynamic.
@@ -112,3 +121,7 @@ func (d *dynamicResourceClient) Watch(options metav1.ListOptions) (watch.Interfa
 func (d *dynamicResourceClient) Get(name string, opts metav1.GetOptions) (*unstructured.Unstructured, error) {
 	return d.resourceClient.Get(name, opts)
 }
+
+func (d *dynamicResourceClient) Patch(name string, data []byte) (*unstructured.Unstructured, error) {
+	return d.resourceClient.Patch(name, types.MergePatchType, data)
+}

pkg/kuberesource/kuberesource.go

@@ -28,4 +28,5 @@ var (
 	PersistentVolumeClaims = schema.GroupResource{Group: "", Resource: "persistentvolumeclaims"}
 	PersistentVolumes      = schema.GroupResource{Group: "", Resource: "persistentvolumes"}
 	Pods                   = schema.GroupResource{Group: "", Resource: "pods"}
+	ServiceAccounts        = schema.GroupResource{Group: "", Resource: "serviceaccounts"}
 )

pkg/restore/merge_service_account.go

@@ -0,0 +1,134 @@
+/*
+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 (
+	"encoding/json"
+	"strings"
+
+	jsonpatch "github.com/evanphx/json-patch"
+	"github.com/pkg/errors"
+
+	corev1api "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	"github.com/heptio/ark/pkg/util/collections"
+)
+
+// 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)
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(fromCluster.UnstructuredContent(), desired); err != nil {
+		return nil, errors.Wrap(err, "unable to convert from-cluster service account from unstructured to serviceaccount")
+
+	}
+
+	backupSA := new(corev1api.ServiceAccount)
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(fromBackup.UnstructuredContent(), backupSA); err != nil {
+		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, "default-token-") {
+			// Copy all secrets *except* default-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)
+
+	collections.MergeMaps(desired.Labels, backupSA.Labels)
+
+	collections.MergeMaps(desired.Annotations, backupSA.Annotations)
+
+	desiredUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(desired)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to convert desired service account to unstructured")
+	}
+	// The DefaultUnstructuredConverter.ToUnstructured function will populate the creation timestamp with the nil value
+	// However, we remove this on both the backup and cluster objects before comparison, and we don't want it in any patches.
+	delete(desiredUnstructured["metadata"].(map[string]interface{}), "creationTimestamp")
+
+	return &unstructured.Unstructured{Object: desiredUnstructured}, nil
+}
+
+func mergeObjectReferenceSlices(first, second []corev1api.ObjectReference) []corev1api.ObjectReference {
+	for _, s := range second {
+		var exists bool
+		for _, f := range first {
+			if s.Name == f.Name {
+				exists = true
+				break
+			}
+		}
+		if !exists {
+			first = append(first, s)
+		}
+	}
+
+	return first
+}
+
+func mergeLocalObjectReferenceSlices(first, second []corev1api.LocalObjectReference) []corev1api.LocalObjectReference {
+	for _, s := range second {
+		var exists bool
+		for _, f := range first {
+			if s.Name == f.Name {
+				exists = true
+				break
+			}
+		}
+		if !exists {
+			first = append(first, s)
+		}
+	}
+
+	return first
+}
+
+// generatePatch will calculate a JSON merge patch for an object's desired state.
+// If the passed in objects are already equal, nil is returned.
+func generatePatch(fromCluster, desired *unstructured.Unstructured) ([]byte, error) {
+	// If the objects are already equal, there's no need to generate a patch.
+	if equality.Semantic.DeepEqual(fromCluster, desired) {
+		return nil, nil
+	}
+
+	desiredBytes, err := json.Marshal(desired.Object)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to marshal desired object")
+	}
+
+	fromClusterBytes, err := json.Marshal(fromCluster.Object)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to marshal in-cluster object")
+	}
+
+	patchBytes, err := jsonpatch.CreateMergePatch(fromClusterBytes, desiredBytes)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to create merge patch")
+	}
+
+	return patchBytes, nil
+}

pkg/restore/merge_service_account_test.go

@@ -0,0 +1,693 @@
+/*
+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"
+	"testing"
+	"unicode"
+
+	"github.com/stretchr/testify/assert"
+
+	corev1api "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	arktest "github.com/heptio/ark/pkg/util/test"
+)
+
+var mergedServiceAccountsBenchmarkResult *unstructured.Unstructured
+
+func BenchmarkMergeServiceAccountBasic(b *testing.B) {
+	tests := []struct {
+		name        string
+		fromCluster *unstructured.Unstructured
+		fromBackup  *unstructured.Unstructured
+	}{
+		{
+			name: "only default tokens present",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			fromBackup: arktest.UnstructuredOrDie(
+				`{
+					"kind": "ServiceAccount",
+					"apiVersion": "v1",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-xzy12" }
+					]
+				}`,
+			),
+		},
+		{
+			name: "service accounts with multiple secrets",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" },
+						{ "name": "my-secret" },
+						{ "name": "sekrit" }
+					]
+				}`,
+			),
+
+			fromBackup: arktest.UnstructuredOrDie(
+				`{
+					"kind": "ServiceAccount",
+					"apiVersion": "v1",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-xzy12" },
+						{ "name": "my-old-secret" },
+						{ "name": "secrete"}
+					]
+				}`,
+			),
+		},
+		{
+			name: "service accounts with labels and annotations",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default",
+						"labels": {
+							"l1": "v1",
+							"l2": "v2",
+							"l3": "v3"
+						},
+						"annotations": {
+							"a1": "v1",
+							"a2": "v2",
+							"a3": "v3",
+							"a4": "v4"
+						}
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+
+			fromBackup: arktest.UnstructuredOrDie(
+				`{
+					"kind": "ServiceAccount",
+					"apiVersion": "v1",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default",
+						"labels": {
+							"l1": "v1",
+							"l2": "v2",
+							"l3": "v3",
+							"l4": "v4",
+							"l5": "v5"
+						},
+						"annotations": {
+							"a1": "v1",
+							"a2": "v2",
+							"a3": "v3",
+							"a4": "v4",
+							"a5": "v5",
+							"a6": "v6"
+						}
+					},
+					"secrets": [
+						{ "name": "default-token-xzy12" }
+					]
+				}`,
+			),
+		},
+	}
+
+	var desired *unstructured.Unstructured
+
+	for _, test := range tests {
+		b.Run(test.name, func(b *testing.B) {
+			for n := 0; n < b.N; n++ {
+				desired, _ = mergeServiceAccounts(test.fromCluster, test.fromBackup)
+			}
+
+			mergedServiceAccountsBenchmarkResult = desired
+		})
+	}
+}
+
+func TestMergeLocalObjectReferenceSlices(t *testing.T) {
+	tests := []struct {
+		name     string
+		first    []corev1api.LocalObjectReference
+		second   []corev1api.LocalObjectReference
+		expected []corev1api.LocalObjectReference
+	}{
+		{
+			name: "two slices without overlapping elements",
+			first: []corev1api.LocalObjectReference{
+				{Name: "lor1"},
+				{Name: "lor2"},
+			},
+			second: []corev1api.LocalObjectReference{
+				{Name: "lor3"},
+				{Name: "lor4"},
+			},
+			expected: []corev1api.LocalObjectReference{
+				{Name: "lor1"},
+				{Name: "lor2"},
+				{Name: "lor3"},
+				{Name: "lor4"},
+			},
+		},
+		{
+			name: "two slices with an overlapping element",
+			first: []corev1api.LocalObjectReference{
+				{Name: "lor1"},
+				{Name: "lor2"},
+			},
+			second: []corev1api.LocalObjectReference{
+				{Name: "lor3"},
+				{Name: "lor2"},
+			},
+			expected: []corev1api.LocalObjectReference{
+				{Name: "lor1"},
+				{Name: "lor2"},
+				{Name: "lor3"},
+			},
+		},
+		{
+			name: "merging always adds elements to the end",
+			first: []corev1api.LocalObjectReference{
+				{Name: "lor3"},
+				{Name: "lor4"},
+			},
+			second: []corev1api.LocalObjectReference{
+				{Name: "lor1"},
+				{Name: "lor2"},
+			},
+			expected: []corev1api.LocalObjectReference{
+				{Name: "lor3"},
+				{Name: "lor4"},
+				{Name: "lor1"},
+				{Name: "lor2"},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			result := mergeLocalObjectReferenceSlices(test.first, test.second)
+			assert.Equal(t, test.expected, result)
+		})
+	}
+}
+
+func TestMergeObjectReferenceSlices(t *testing.T) {
+	tests := []struct {
+		name     string
+		first    []corev1api.ObjectReference
+		second   []corev1api.ObjectReference
+		expected []corev1api.ObjectReference
+	}{
+		{
+			name: "two slices without overlapping elements",
+			first: []corev1api.ObjectReference{
+				{Name: "or1"},
+				{Name: "or2"},
+			},
+			second: []corev1api.ObjectReference{
+				{Name: "or3"},
+				{Name: "or4"},
+			},
+			expected: []corev1api.ObjectReference{
+				{Name: "or1"},
+				{Name: "or2"},
+				{Name: "or3"},
+				{Name: "or4"},
+			},
+		},
+		{
+			name: "two slices with an overlapping element",
+			first: []corev1api.ObjectReference{
+				{Name: "or1"},
+				{Name: "or2"},
+			},
+			second: []corev1api.ObjectReference{
+				{Name: "or3"},
+				{Name: "or2"},
+			},
+			expected: []corev1api.ObjectReference{
+				{Name: "or1"},
+				{Name: "or2"},
+				{Name: "or3"},
+			},
+		},
+		{
+			name: "merging always adds elements to the end",
+			first: []corev1api.ObjectReference{
+				{Name: "or3"},
+				{Name: "or4"},
+			},
+			second: []corev1api.ObjectReference{
+				{Name: "or1"},
+				{Name: "or2"},
+			},
+			expected: []corev1api.ObjectReference{
+				{Name: "or3"},
+				{Name: "or4"},
+				{Name: "or1"},
+				{Name: "or2"},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			result := mergeObjectReferenceSlices(test.first, test.second)
+			assert.Equal(t, test.expected, result)
+		})
+	}
+}
+
+// stripWhitespace removes any Unicode whitespace from a string.
+// Useful for cleaning up formatting on expected JSON strings before comparison
+func stripWhitespace(s string) string {
+	return strings.Map(func(r rune) rune {
+		if unicode.IsSpace(r) {
+			return -1
+		}
+		return r
+	}, s)
+}
+
+func TestGeneratePatch(t *testing.T) {
+	tests := []struct {
+		name           string
+		fromCluster    *unstructured.Unstructured
+		desired        *unstructured.Unstructured
+		expectedString string
+		expectedErr    bool
+	}{
+		{
+			name: "objects are equal, no patch needed",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			desired: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			expectedString: "",
+			expectedErr:    false,
+		},
+		{
+			name: "patch is required when labels are present",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			desired: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default",
+						"labels": {
+							"label1": "value1",
+							"label2": "value2"
+						}
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			expectedString: stripWhitespace(
+				`{
+					"metadata": {
+						"labels": {
+							"label1":"value1",
+							"label2":"value2"
+						}
+					}
+				}`,
+			),
+			expectedErr: false,
+		},
+		{
+			name: "patch is required when annotations are present",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			desired: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default",
+						"annotations" :{
+							"a1": "v1",
+							"a2": "v2"
+						}
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			expectedString: stripWhitespace(
+				`{
+					"metadata": {
+						"annotations": {
+							"a1":"v1",
+							"a2":"v2"
+						}
+					}
+				}`,
+			),
+			expectedErr: false,
+		},
+		{
+			name: "patch is required many secrets are present",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			desired: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" },
+						{ "name": "sekrit" },
+						{ "name": "secrete" }
+					]
+				}`,
+			),
+			expectedString: stripWhitespace(
+				`{
+					"secrets": [
+						{"name": "default-token-abcde"},
+						{"name": "sekrit"},
+						{"name": "secrete"}
+					]
+				}`,
+			),
+			expectedErr: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			result, err := generatePatch(test.fromCluster, test.desired)
+			if assert.Equal(t, test.expectedErr, err != nil) {
+				assert.Equal(t, test.expectedString, string(result))
+			}
+		})
+	}
+}
+
+func TestMergeServiceAccountBasic(t *testing.T) {
+	tests := []struct {
+		name        string
+		fromCluster *unstructured.Unstructured
+		fromBackup  *unstructured.Unstructured
+		expectedRes *unstructured.Unstructured
+		expectedErr bool
+	}{
+		{
+			name: "only default tokens present",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+			fromBackup: arktest.UnstructuredOrDie(
+				`{
+					"kind": "ServiceAccount",
+					"apiVersion": "v1",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-xzy12" }
+					]
+				}`,
+			),
+			expectedRes: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+		},
+		{
+			name: "service accounts with multiple secrets",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" },
+						{ "name": "my-secret" },
+						{ "name": "sekrit" }
+					]
+				}`,
+			),
+
+			fromBackup: arktest.UnstructuredOrDie(
+				`{
+					"kind": "ServiceAccount",
+					"apiVersion": "v1",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-xzy12" },
+						{ "name": "my-old-secret" },
+						{ "name": "secrete"}
+					]
+				}`,
+			),
+			expectedRes: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default"
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" },
+						{ "name": "my-secret" },
+						{ "name": "sekrit" },
+						{ "name": "my-old-secret" },
+						{ "name": "secrete"}
+					]
+				}`,
+			),
+		},
+		{
+			name: "service accounts with labels and annotations",
+			fromCluster: arktest.UnstructuredOrDie(
+				`{
+					"apiVersion": "v1",
+					"kind": "ServiceAccount",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default",
+						"labels": {
+							"l1": "v1",
+							"l2": "v2",
+							"l3": "v3"
+						},
+						"annotations": {
+							"a1": "v1",
+							"a2": "v2",
+							"a3": "v3",
+							"a4": "v4"
+						}
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+
+			fromBackup: arktest.UnstructuredOrDie(
+				`{
+					"kind": "ServiceAccount",
+					"apiVersion": "v1",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default",
+						"labels": {
+							"l1": "v1",
+							"l2": "v2",
+							"l3": "v3",
+							"l4": "v4",
+							"l5": "v5"
+						},
+						"annotations": {
+							"a1": "v1",
+							"a2": "v2",
+							"a3": "v3",
+							"a4": "v4",
+							"a5": "v5",
+							"a6": "v6"
+						}
+					},
+					"secrets": [
+						{ "name": "default-token-xzy12" }
+					]
+				}`,
+			),
+			expectedRes: arktest.UnstructuredOrDie(
+				`{
+					"kind": "ServiceAccount",
+					"apiVersion": "v1",
+					"metadata": {
+						"namespace": "ns1",
+						"name": "default",
+						"labels": {
+							"l1": "v1",
+							"l2": "v2",
+							"l3": "v3",
+							"l4": "v4",
+							"l5": "v5"
+						},
+						"annotations": {
+							"a1": "v1",
+							"a2": "v2",
+							"a3": "v3",
+							"a4": "v4",
+							"a5": "v5",
+							"a6": "v6"
+						}
+					},
+					"secrets": [
+						{ "name": "default-token-abcde" }
+					]
+				}`,
+			),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(b *testing.T) {
+			result, err := mergeServiceAccounts(test.fromCluster, test.fromBackup)
+			if err != nil {
+				assert.Equal(t, test.expectedRes, result)
+			}
+		})
+	}
+}

pkg/restore/restore.go

@@ -732,19 +732,57 @@ func (ctx *context) restoreResource(resource, namespace, resourcePath string) (a
 		ctx.infof("Restoring %s: %v", obj.GroupVersionKind().Kind, obj.GetName())
 		createdObj, restoreErr := resourceClient.Create(obj)
 		if apierrors.IsAlreadyExists(restoreErr) {
-			equal := false
-			if fromCluster, err := resourceClient.Get(obj.GetName(), metav1.GetOptions{}); err == nil {
-				equal, err = objectsAreEqual(fromCluster, obj)
-				// Log any errors trying to check equality
-				if err != nil {
-					ctx.infof("error checking %s against cluster: %v", obj.GetName(), err)
-				}
-			} else {
-				ctx.infof("Error retrieving cluster version of %s: %v", obj.GetName(), err)
+			fromCluster, err := resourceClient.Get(obj.GetName(), metav1.GetOptions{})
+			if err != nil {
+				ctx.infof("Error retrieving cluster version of %s: %v", kube.NamespaceAndName(obj), err)
+				addToResult(&warnings, namespace, err)
+				continue
 			}
-			if !equal {
-				e := errors.Errorf("not restored: %s and is different from backed up version.", restoreErr)
-				addToResult(&warnings, namespace, e)
+			// Remove insubstantial metadata
+			fromCluster, err = resetMetadataAndStatus(fromCluster)
+			if err != nil {
+				ctx.infof("Error trying to reset metadata for %s: %v", kube.NamespaceAndName(obj), err)
+				addToResult(&warnings, namespace, err)
+				continue
+			}
+
+			// We know the cluster won't have the restore name label, so
+			// copy it over from the backup
+			restoreName := obj.GetLabels()[api.RestoreLabelKey]
+			addLabel(fromCluster, api.RestoreLabelKey, restoreName)
+
+			if !equality.Semantic.DeepEqual(fromCluster, obj) {
+				switch groupResource {
+				case kuberesource.ServiceAccounts:
+					desired, err := mergeServiceAccounts(fromCluster, obj)
+					if err != nil {
+						ctx.infof("error merging secrets for ServiceAccount %s: %v", kube.NamespaceAndName(obj), err)
+						addToResult(&warnings, namespace, err)
+						continue
+					}
+
+					patchBytes, err := generatePatch(fromCluster, desired)
+					if err != nil {
+						ctx.infof("error generating patch for ServiceAccount %s: %v", kube.NamespaceAndName(obj), err)
+						addToResult(&warnings, namespace, err)
+						continue
+					}
+
+					if patchBytes == nil {
+						// In-cluster and desired state are the same, so move on to the next item
+						continue
+					}
+
+					_, err = resourceClient.Patch(obj.GetName(), patchBytes)
+					if err != nil {
+						addToResult(&warnings, namespace, err)
+					} else {
+						ctx.infof("ServiceAccount %s successfully updated", kube.NamespaceAndName(obj))
+					}
+				default:
+					e := errors.Errorf("not restored: %s and is different from backed up version.", restoreErr)
+					addToResult(&warnings, namespace, e)
+				}
 			}
 			continue
 		}
@@ -877,25 +915,6 @@ func (ctx *context) executePVAction(obj *unstructured.Unstructured) (*unstructur
 	return updated2, nil
 }
 
-// objectsAreEqual takes two unstructured objects and checks for equality.
-// The fromCluster object is mutated to remove any insubstantial runtime
-// information that won't match
-func objectsAreEqual(fromCluster, fromBackup *unstructured.Unstructured) (bool, error) {
-	// Remove insubstantial metadata
-	fromCluster, err := resetMetadataAndStatus(fromCluster)
-	if err != nil {
-		return false, err
-	}
-
-	// We know the cluster won't have the restore name label, so
-	// copy it over from the backup
-	restoreName := fromBackup.GetLabels()[api.RestoreLabelKey]
-	addLabel(fromCluster, api.RestoreLabelKey, restoreName)
-
-	// If there are no specific actions needed based on the type, simply check for equality.
-	return equality.Semantic.DeepEqual(fromBackup, fromCluster), nil
-}
-
 func isPVReady(obj runtime.Unstructured) bool {
 	phase, err := collections.GetString(obj.UnstructuredContent(), "status.phase")
 	if err != nil {

pkg/restore/restore_test.go

@@ -19,12 +19,14 @@ package restore
 import (
 	"encoding/json"
 	"testing"
+	"time"
 
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
@@ -35,6 +37,7 @@ import (
 
 	api "github.com/heptio/ark/pkg/apis/ark/v1"
 	"github.com/heptio/ark/pkg/cloudprovider"
+	"github.com/heptio/ark/pkg/kuberesource"
 	"github.com/heptio/ark/pkg/util/boolptr"
 	"github.com/heptio/ark/pkg/util/collections"
 	arktest "github.com/heptio/ark/pkg/util/test"
@@ -528,6 +531,15 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			fileSystem:              arktest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()),
 			expectedObjs:            toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap),
 		},
+		{
+			name:                    "serviceaccounts are restored",
+			namespace:               "ns-1",
+			resourcePath:            "serviceaccounts",
+			labelSelector:           labels.NewSelector(),
+			includeClusterResources: nil,
+			fileSystem:              arktest.NewFakeFileSystem().WithFile("serviceaccounts/sa-1.json", newTestServiceAccount().ToJSON()),
+			expectedObjs:            toUnstructured(newTestServiceAccount().WithArkLabel("my-restore").ServiceAccount),
+		},
 	}
 
 	for _, test := range tests {
@@ -547,6 +559,9 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 			dynamicFactory.On("ClientForGroupVersionResource", gv, pvResource, test.namespace).Return(resourceClient, nil)
 			resourceClient.On("Watch", metav1.ListOptions{}).Return(&fakeWatch{}, nil)
 
+			saResource := metav1.APIResource{Name: "serviceaccounts", Namespaced: true}
+			dynamicFactory.On("ClientForGroupVersionResource", gv, saResource, test.namespace).Return(resourceClient, nil)
+
 			ctx := &context{
 				dynamicFactory: dynamicFactory,
 				actions:        test.actions,
@@ -575,6 +590,90 @@ func TestRestoreResourceForNamespace(t *testing.T) {
 	}
 }
 
+func TestRestoringExistingServiceAccount(t *testing.T) {
+	fromCluster := newTestServiceAccount()
+	fromClusterUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(fromCluster.ServiceAccount)
+	require.NoError(t, err)
+
+	different := newTestServiceAccount().WithImagePullSecret("image-secret").WithSecret("secret")
+	differentUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(different.ServiceAccount)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name          string
+		expectedPatch []byte
+		fromBackup    *unstructured.Unstructured
+	}{
+		{
+			name:       "fromCluster and fromBackup are exactly the same",
+			fromBackup: &unstructured.Unstructured{Object: fromClusterUnstructured},
+		},
+		{
+			name:          "fromCluster and fromBackup are different",
+			fromBackup:    &unstructured.Unstructured{Object: differentUnstructured},
+			expectedPatch: []byte(`{"imagePullSecrets":[{"name":"image-secret"}],"secrets":[{"name":"secret"}]}`),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			resourceClient := &arktest.FakeDynamicClient{}
+			defer resourceClient.AssertExpectations(t)
+			name := fromCluster.GetName()
+
+			// restoreResource will add the restore label to object provided to create, so we need to make a copy to provide to our expected call
+			m := make(map[string]interface{})
+			for k, v := range test.fromBackup.Object {
+				m[k] = v
+			}
+			fromBackupWithLabel := &unstructured.Unstructured{Object: m}
+			l := map[string]string{api.RestoreLabelKey: "my-restore"}
+			fromBackupWithLabel.SetLabels(l)
+			// resetMetadataAndStatus will strip the creationTimestamp before calling Create
+			fromBackupWithLabel.SetCreationTimestamp(metav1.Time{Time: time.Time{}})
+
+			resourceClient.On("Create", fromBackupWithLabel).Return(new(unstructured.Unstructured), k8serrors.NewAlreadyExists(kuberesource.ServiceAccounts, name))
+			resourceClient.On("Get", name, metav1.GetOptions{}).Return(&unstructured.Unstructured{Object: fromClusterUnstructured}, nil)
+
+			if len(test.expectedPatch) > 0 {
+				resourceClient.On("Patch", name, test.expectedPatch).Return(test.fromBackup, nil)
+			}
+
+			dynamicFactory := &arktest.FakeDynamicFactory{}
+			gv := schema.GroupVersion{Group: "", Version: "v1"}
+
+			resource := metav1.APIResource{Name: "serviceaccounts", Namespaced: true}
+			dynamicFactory.On("ClientForGroupVersionResource", gv, resource, "ns-1").Return(resourceClient, nil)
+			fromBackupJSON, err := json.Marshal(test.fromBackup)
+			require.NoError(t, err)
+			ctx := &context{
+				dynamicFactory: dynamicFactory,
+				actions:        []resolvedAction{},
+				fileSystem: arktest.NewFakeFileSystem().
+					WithFile("foo/resources/serviceaccounts/namespaces/ns-1/sa-1.json", fromBackupJSON),
+				selector: labels.NewSelector(),
+				restore: &api.Restore{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: api.DefaultNamespace,
+						Name:      "my-restore",
+					},
+					Spec: api.RestoreSpec{
+						IncludeClusterResources: nil,
+					},
+				},
+				backup: &api.Backup{},
+				logger: arktest.NewLogger(),
+			}
+			warnings, errors := ctx.restoreResource("serviceaccounts", "ns-1", "foo/resources/serviceaccounts/namespaces/ns-1/")
+
+			assert.Empty(t, warnings.Ark)
+			assert.Empty(t, warnings.Cluster)
+			assert.Empty(t, warnings.Namespaces)
+			assert.Equal(t, api.RestoreResult{}, errors)
+		})
+	}
+}
+
 type fakeWatch struct{}
 
 func (w *fakeWatch) Stop() {}
@@ -752,61 +851,6 @@ func TestIsCompleted(t *testing.T) {
 	}
 }
 
-func TestObjectsAreEqual(t *testing.T) {
-	tests := []struct {
-		name        string
-		backupObj   *unstructured.Unstructured
-		clusterObj  *unstructured.Unstructured
-		expectedErr bool
-		expectedRes bool
-	}{
-		{
-			name:        "objects are already equal",
-			backupObj:   NewTestUnstructured().WithName("obj").WithArkLabel("test").Unstructured,
-			clusterObj:  NewTestUnstructured().WithName("obj").Unstructured,
-			expectedErr: false,
-			expectedRes: true,
-		},
-		{
-			name:        "objects reset correctly",
-			backupObj:   NewTestUnstructured().WithName("obj").WithArkLabel("test").Unstructured,
-			clusterObj:  NewTestUnstructured().WithMetadata("blah", "foo").WithName("obj").Unstructured,
-			expectedErr: false,
-			expectedRes: true,
-		},
-		{
-			name:        "cluster object has no metadata to reset",
-			backupObj:   NewTestUnstructured().WithName("obj").WithArkLabel("test").Unstructured,
-			clusterObj:  NewTestUnstructured().Unstructured,
-			expectedErr: true,
-			expectedRes: false,
-		},
-		{
-			name:        "Test JSON objects",
-			backupObj:   arktest.UnstructuredOrDie(`{"apiVersion":"v1","kind":"ServiceAccount","metadata":{"name":"default","namespace":"nginx-example", "labels": {"ark-restore": "test"}},"secrets":[{"name":"default-token-xhjjc"}]}`),
-			clusterObj:  arktest.UnstructuredOrDie(`{"apiVersion":"v1","kind":"ServiceAccount","metadata":{"creationTimestamp":"2018-04-05T20:12:21Z","name":"default","namespace":"nginx-example","resourceVersion":"650","selfLink":"/api/v1/namespaces/nginx-example/serviceaccounts/default","uid":"a5a3d2a2-390d-11e8-9644-42010a960002"},"secrets":[{"name":"default-token-xhjjc"}]}`),
-			expectedErr: false,
-			expectedRes: true,
-		},
-		{
-			name:        "Test ServiceAccount secrets mismatch",
-			backupObj:   arktest.UnstructuredOrDie(`{"apiVersion":"v1","kind":"ServiceAccount","metadata":{"name":"default","namespace":"nginx-example", "labels": {"ark-restore": "test"}},"secrets":[{"name":"default-token-abcde"}]}`),
-			clusterObj:  arktest.UnstructuredOrDie(`{"apiVersion":"v1","kind":"ServiceAccount","metadata":{"creationTimestamp":"2018-04-05T20:12:21Z","name":"default","namespace":"nginx-example","resourceVersion":"650","selfLink":"/api/v1/namespaces/nginx-example/serviceaccounts/default","uid":"a5a3d2a2-390d-11e8-9644-42010a960002"},"secrets":[{"name":"default-token-xhjjc"}]}`),
-			expectedErr: false,
-			expectedRes: false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			res, err := objectsAreEqual(test.clusterObj, test.backupObj)
-			if assert.Equal(t, test.expectedErr, err != nil) {
-				assert.Equal(t, test.expectedRes, res)
-			}
-		})
-	}
-}
-
 func TestExecutePVAction(t *testing.T) {
 	iops := int64(1000)
 
@@ -1092,6 +1136,51 @@ func toUnstructured(objs ...runtime.Object) []unstructured.Unstructured {
 	return res
 }
 
+type testServiceAccount struct {
+	*v1.ServiceAccount
+}
+
+func newTestServiceAccount() *testServiceAccount {
+	return &testServiceAccount{
+		ServiceAccount: &v1.ServiceAccount{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "v1",
+				Kind:       "ServiceAccount",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace:         "ns-1",
+				Name:              "test-sa",
+				CreationTimestamp: metav1.Time{Time: time.Now()},
+			},
+		},
+	}
+}
+
+func (sa *testServiceAccount) WithArkLabel(restoreName string) *testServiceAccount {
+	if sa.Labels == nil {
+		sa.Labels = make(map[string]string)
+	}
+	sa.Labels[api.RestoreLabelKey] = restoreName
+	return sa
+}
+
+func (sa *testServiceAccount) WithImagePullSecret(name string) *testServiceAccount {
+	secret := v1.LocalObjectReference{Name: name}
+	sa.ImagePullSecrets = append(sa.ImagePullSecrets, secret)
+	return sa
+}
+
+func (sa *testServiceAccount) WithSecret(name string) *testServiceAccount {
+	secret := v1.ObjectReference{Name: name}
+	sa.Secrets = append(sa.Secrets, secret)
+	return sa
+}
+
+func (sa *testServiceAccount) ToJSON() []byte {
+	bytes, _ := json.Marshal(sa.ServiceAccount)
+	return bytes
+}
+
 type testPersistentVolume struct {
 	*v1.PersistentVolume
 }

pkg/util/collections/map_utils.go

@@ -122,3 +122,14 @@ func Exists(root map[string]interface{}, path string) bool {
 	_, err := GetValue(root, path)
 	return err == nil
 }
+
+// MergeMaps takes two map[string]string and merges missing keys from the second into the first.
+// If a key already exists, its value is not overwritten.
+func MergeMaps(first, second map[string]string) {
+	for k, v := range second {
+		_, ok := first[k]
+		if !ok {
+			first[k] = v
+		}
+	}
+}

pkg/util/collections/map_utils_test.go

@@ -16,7 +16,9 @@ limitations under the License.
 
 package collections
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestGetString(t *testing.T) {
 	var testCases = []struct {

pkg/util/test/fake_dynamic.go

@@ -64,3 +64,8 @@ func (c *FakeDynamicClient) Get(name string, opts metav1.GetOptions) (*unstructu
 	args := c.Called(name, opts)
 	return args.Get(0).(*unstructured.Unstructured), args.Error(1)
 }
+
+func (c *FakeDynamicClient) Patch(name string, data []byte) (*unstructured.Unstructured, error) {
+	args := c.Called(name, data)
+	return args.Get(0).(*unstructured.Unstructured), args.Error(1)
+}