diff --git a/pkg/client/dynamic.go b/pkg/client/dynamic.go index eddd500fa..50b720efa 100644 --- a/pkg/client/dynamic.go +++ b/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) +} diff --git a/pkg/kuberesource/kuberesource.go b/pkg/kuberesource/kuberesource.go index f03cd377d..37e80355d 100644 --- a/pkg/kuberesource/kuberesource.go +++ b/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"} ) diff --git a/pkg/restore/merge_service_account.go b/pkg/restore/merge_service_account.go new file mode 100644 index 000000000..641586ff0 --- /dev/null +++ b/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 +} diff --git a/pkg/restore/merge_service_account_test.go b/pkg/restore/merge_service_account_test.go new file mode 100644 index 000000000..cb58c93c6 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index ee68fc3c7..0edb634a4 100644 --- a/pkg/restore/restore.go +++ b/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 { diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 0174c98d9..4fd6fb6a7 100644 --- a/pkg/restore/restore_test.go +++ b/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 } diff --git a/pkg/util/collections/map_utils.go b/pkg/util/collections/map_utils.go index 5dfdaa9b4..f0b8fb934 100644 --- a/pkg/util/collections/map_utils.go +++ b/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 + } + } +} diff --git a/pkg/util/collections/map_utils_test.go b/pkg/util/collections/map_utils_test.go index 2651d4ad6..e717e9438 100644 --- a/pkg/util/collections/map_utils_test.go +++ b/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 { diff --git a/pkg/util/test/fake_dynamic.go b/pkg/util/test/fake_dynamic.go index 3ab1e33e2..30cce4ae9 100644 --- a/pkg/util/test/fake_dynamic.go +++ b/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) +}