diff --git a/pkg/backup/pod_action.go b/pkg/backup/pod_action.go index d779883e8..4268611dc 100644 --- a/pkg/backup/pod_action.go +++ b/pkg/backup/pod_action.go @@ -19,12 +19,11 @@ package backup import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/api/meta" + corev1api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" v1 "github.com/heptio/velero/pkg/apis/velero/v1" "github.com/heptio/velero/pkg/kuberesource" - "github.com/heptio/velero/pkg/util/collections" ) // podAction implements ItemAction. @@ -51,48 +50,27 @@ func (a *podAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runti a.log.Info("Executing podAction") defer a.log.Info("Done executing podAction") - pod := item.UnstructuredContent() - if !collections.Exists(pod, "spec.volumes") { + pod := new(corev1api.Pod) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(item.UnstructuredContent(), pod); err != nil { + return nil, nil, errors.WithStack(err) + } + + if len(pod.Spec.Volumes) == 0 { a.log.Info("pod has no volumes") return item, nil, nil } - metadata, err := meta.Accessor(item) - if err != nil { - return nil, nil, errors.Wrap(err, "unable to access pod metadata") - } - - volumes, err := collections.GetSlice(pod, "spec.volumes") - if err != nil { - return nil, nil, errors.WithMessage(err, "error getting spec.volumes") - } - - var errs []error var additionalItems []ResourceIdentifier + for _, volume := range pod.Spec.Volumes { + if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.ClaimName != "" { + a.log.Infof("Adding pvc %s to additionalItems", volume.PersistentVolumeClaim.ClaimName) - for i := range volumes { - volume, ok := volumes[i].(map[string]interface{}) - if !ok { - errs = append(errs, errors.Errorf("unexpected type %T", volumes[i])) - continue + additionalItems = append(additionalItems, ResourceIdentifier{ + GroupResource: kuberesource.PersistentVolumeClaims, + Namespace: pod.Namespace, + Name: volume.PersistentVolumeClaim.ClaimName, + }) } - if !collections.Exists(volume, "persistentVolumeClaim.claimName") { - continue - } - - claimName, err := collections.GetString(volume, "persistentVolumeClaim.claimName") - if err != nil { - errs = append(errs, err) - continue - } - - a.log.Infof("Adding pvc %s to additionalItems", claimName) - - additionalItems = append(additionalItems, ResourceIdentifier{ - GroupResource: kuberesource.PersistentVolumeClaims, - Namespace: metadata.GetNamespace(), - Name: claimName, - }) } return item, additionalItems, nil diff --git a/pkg/cloudprovider/aws/block_store.go b/pkg/cloudprovider/aws/block_store.go index 1b0e53a30..5c3c58df2 100644 --- a/pkg/cloudprovider/aws/block_store.go +++ b/pkg/cloudprovider/aws/block_store.go @@ -27,11 +27,12 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" "github.com/sirupsen/logrus" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "github.com/heptio/velero/pkg/cloudprovider" - "github.com/heptio/velero/pkg/util/collections" ) const regionKey = "region" @@ -254,26 +255,39 @@ func (b *blockStore) DeleteSnapshot(snapshotID string) error { var ebsVolumeIDRegex = regexp.MustCompile("vol-.*") -func (b *blockStore) GetVolumeID(pv runtime.Unstructured) (string, error) { - if !collections.Exists(pv.UnstructuredContent(), "spec.awsElasticBlockStore") { +func (b *blockStore) GetVolumeID(unstructuredPV runtime.Unstructured) (string, error) { + pv := new(v1.PersistentVolume) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPV.UnstructuredContent(), pv); err != nil { + return "", errors.WithStack(err) + } + + if pv.Spec.AWSElasticBlockStore == nil { return "", nil } - volumeID, err := collections.GetString(pv.UnstructuredContent(), "spec.awsElasticBlockStore.volumeID") - if err != nil { - return "", err + if pv.Spec.AWSElasticBlockStore.VolumeID == "" { + return "", errors.New("spec.awsElasticBlockStore.volumeID not found") } - return ebsVolumeIDRegex.FindString(volumeID), nil + return ebsVolumeIDRegex.FindString(pv.Spec.AWSElasticBlockStore.VolumeID), nil } -func (b *blockStore) SetVolumeID(pv runtime.Unstructured, volumeID string) (runtime.Unstructured, error) { - aws, err := collections.GetMap(pv.UnstructuredContent(), "spec.awsElasticBlockStore") - if err != nil { - return nil, err +func (b *blockStore) SetVolumeID(unstructuredPV runtime.Unstructured, volumeID string) (runtime.Unstructured, error) { + pv := new(v1.PersistentVolume) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPV.UnstructuredContent(), pv); err != nil { + return nil, errors.WithStack(err) } - aws["volumeID"] = volumeID + if pv.Spec.AWSElasticBlockStore == nil { + return nil, errors.New("spec.awsElasticBlockStore not found") + } - return pv, nil + pv.Spec.AWSElasticBlockStore.VolumeID = volumeID + + res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pv) + if err != nil { + return nil, errors.WithStack(err) + } + + return &unstructured.Unstructured{Object: res}, nil } diff --git a/pkg/cloudprovider/aws/block_store_test.go b/pkg/cloudprovider/aws/block_store_test.go index 42b6850a4..ecfd03120 100644 --- a/pkg/cloudprovider/aws/block_store_test.go +++ b/pkg/cloudprovider/aws/block_store_test.go @@ -24,9 +24,9 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - - "github.com/heptio/velero/pkg/util/collections" + "k8s.io/apimachinery/pkg/runtime" ) func TestGetVolumeID(t *testing.T) { @@ -87,9 +87,11 @@ func TestSetVolumeID(t *testing.T) { } updatedPV, err = b.SetVolumeID(pv, "vol-updated") require.NoError(t, err) - actual, err := collections.GetString(updatedPV.UnstructuredContent(), "spec.awsElasticBlockStore.volumeID") - require.NoError(t, err) - assert.Equal(t, "vol-updated", actual) + + res := new(v1.PersistentVolume) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(updatedPV.UnstructuredContent(), res)) + require.NotNil(t, res.Spec.AWSElasticBlockStore) + assert.Equal(t, "vol-updated", res.Spec.AWSElasticBlockStore.VolumeID) } func TestGetTagsForCluster(t *testing.T) { diff --git a/pkg/cloudprovider/azure/block_store.go b/pkg/cloudprovider/azure/block_store.go index 4aa47e34f..77fb10a2a 100644 --- a/pkg/cloudprovider/azure/block_store.go +++ b/pkg/cloudprovider/azure/block_store.go @@ -31,10 +31,11 @@ import ( "github.com/pkg/errors" "github.com/satori/uuid" "github.com/sirupsen/logrus" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "github.com/heptio/velero/pkg/cloudprovider" - "github.com/heptio/velero/pkg/util/collections" ) const ( @@ -335,27 +336,40 @@ func parseFullSnapshotName(name string) (*snapshotIdentifier, error) { return snapshotID, nil } -func (b *blockStore) GetVolumeID(pv runtime.Unstructured) (string, error) { - if !collections.Exists(pv.UnstructuredContent(), "spec.azureDisk") { +func (b *blockStore) GetVolumeID(unstructuredPV runtime.Unstructured) (string, error) { + pv := new(v1.PersistentVolume) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPV.UnstructuredContent(), pv); err != nil { + return "", errors.WithStack(err) + } + + if pv.Spec.AzureDisk == nil { return "", nil } - volumeID, err := collections.GetString(pv.UnstructuredContent(), "spec.azureDisk.diskName") - if err != nil { - return "", err + if pv.Spec.AzureDisk.DiskName == "" { + return "", errors.New("spec.azureDisk.diskName not found") } - return volumeID, nil + return pv.Spec.AzureDisk.DiskName, nil } -func (b *blockStore) SetVolumeID(pv runtime.Unstructured, volumeID string) (runtime.Unstructured, error) { - azure, err := collections.GetMap(pv.UnstructuredContent(), "spec.azureDisk") - if err != nil { - return nil, err +func (b *blockStore) SetVolumeID(unstructuredPV runtime.Unstructured, volumeID string) (runtime.Unstructured, error) { + pv := new(v1.PersistentVolume) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPV.UnstructuredContent(), pv); err != nil { + return nil, errors.WithStack(err) } - azure["diskName"] = volumeID - azure["diskURI"] = getComputeResourceName(b.subscription, b.disksResourceGroup, disksResource, volumeID) + if pv.Spec.AzureDisk == nil { + return nil, errors.New("spec.azureDisk not found") + } - return pv, nil + pv.Spec.AzureDisk.DiskName = volumeID + pv.Spec.AzureDisk.DataDiskURI = getComputeResourceName(b.subscription, b.disksResourceGroup, disksResource, volumeID) + + res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pv) + if err != nil { + return nil, errors.WithStack(err) + } + + return &unstructured.Unstructured{Object: res}, nil } diff --git a/pkg/cloudprovider/azure/block_store_test.go b/pkg/cloudprovider/azure/block_store_test.go index e1c986d49..cdaa8cb59 100644 --- a/pkg/cloudprovider/azure/block_store_test.go +++ b/pkg/cloudprovider/azure/block_store_test.go @@ -21,9 +21,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - - "github.com/heptio/velero/pkg/util/collections" + "k8s.io/apimachinery/pkg/runtime" ) func TestGetVolumeID(t *testing.T) { @@ -75,23 +75,23 @@ func TestSetVolumeID(t *testing.T) { } updatedPV, err = b.SetVolumeID(pv, "updated") require.NoError(t, err) - actual, err := collections.GetString(updatedPV.UnstructuredContent(), "spec.azureDisk.diskName") - require.NoError(t, err) - assert.Equal(t, "updated", actual) - actual, err = collections.GetString(updatedPV.UnstructuredContent(), "spec.azureDisk.diskURI") - require.NoError(t, err) - assert.Equal(t, "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/disks/updated", actual) + + res := new(v1.PersistentVolume) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(updatedPV.UnstructuredContent(), res)) + require.NotNil(t, res.Spec.AzureDisk) + assert.Equal(t, "updated", res.Spec.AzureDisk.DiskName) + assert.Equal(t, "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/disks/updated", res.Spec.AzureDisk.DataDiskURI) // with diskURI azure["diskURI"] = "/foo/bar/updated/blarg" updatedPV, err = b.SetVolumeID(pv, "revised") require.NoError(t, err) - actual, err = collections.GetString(updatedPV.UnstructuredContent(), "spec.azureDisk.diskName") - require.NoError(t, err) - assert.Equal(t, "revised", actual) - actual, err = collections.GetString(updatedPV.UnstructuredContent(), "spec.azureDisk.diskURI") - require.NoError(t, err) - assert.Equal(t, "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/disks/revised", actual) + + res = new(v1.PersistentVolume) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(updatedPV.UnstructuredContent(), res)) + require.NotNil(t, res.Spec.AzureDisk) + assert.Equal(t, "revised", res.Spec.AzureDisk.DiskName) + assert.Equal(t, "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/disks/revised", res.Spec.AzureDisk.DataDiskURI) } // TODO(1.0) rename to TestParseFullSnapshotName, switch to testing diff --git a/pkg/cloudprovider/gcp/block_store.go b/pkg/cloudprovider/gcp/block_store.go index ff0567a99..d0e89072f 100644 --- a/pkg/cloudprovider/gcp/block_store.go +++ b/pkg/cloudprovider/gcp/block_store.go @@ -30,10 +30,11 @@ import ( "golang.org/x/oauth2/google" "google.golang.org/api/compute/v1" "google.golang.org/api/googleapi" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "github.com/heptio/velero/pkg/cloudprovider" - "github.com/heptio/velero/pkg/util/collections" ) const ( @@ -320,26 +321,39 @@ func (b *blockStore) DeleteSnapshot(snapshotID string) error { return nil } -func (b *blockStore) GetVolumeID(pv runtime.Unstructured) (string, error) { - if !collections.Exists(pv.UnstructuredContent(), "spec.gcePersistentDisk") { +func (b *blockStore) GetVolumeID(unstructuredPV runtime.Unstructured) (string, error) { + pv := new(v1.PersistentVolume) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPV.UnstructuredContent(), pv); err != nil { + return "", errors.WithStack(err) + } + + if pv.Spec.GCEPersistentDisk == nil { return "", nil } - volumeID, err := collections.GetString(pv.UnstructuredContent(), "spec.gcePersistentDisk.pdName") - if err != nil { - return "", err + if pv.Spec.GCEPersistentDisk.PDName == "" { + return "", errors.New("spec.gcePersistentDisk.pdName not found") } - return volumeID, nil + return pv.Spec.GCEPersistentDisk.PDName, nil } -func (b *blockStore) SetVolumeID(pv runtime.Unstructured, volumeID string) (runtime.Unstructured, error) { - gce, err := collections.GetMap(pv.UnstructuredContent(), "spec.gcePersistentDisk") - if err != nil { - return nil, err +func (b *blockStore) SetVolumeID(unstructuredPV runtime.Unstructured, volumeID string) (runtime.Unstructured, error) { + pv := new(v1.PersistentVolume) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPV.UnstructuredContent(), pv); err != nil { + return nil, errors.WithStack(err) } - gce["pdName"] = volumeID + if pv.Spec.GCEPersistentDisk == nil { + return nil, errors.New("spec.gcePersistentDisk not found") + } - return pv, nil + pv.Spec.GCEPersistentDisk.PDName = volumeID + + res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pv) + if err != nil { + return nil, errors.WithStack(err) + } + + return &unstructured.Unstructured{Object: res}, nil } diff --git a/pkg/cloudprovider/gcp/block_store_test.go b/pkg/cloudprovider/gcp/block_store_test.go index c72e7acc8..91fac52e7 100644 --- a/pkg/cloudprovider/gcp/block_store_test.go +++ b/pkg/cloudprovider/gcp/block_store_test.go @@ -23,9 +23,10 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" - "github.com/heptio/velero/pkg/util/collections" velerotest "github.com/heptio/velero/pkg/util/test" ) @@ -75,9 +76,11 @@ func TestSetVolumeID(t *testing.T) { } updatedPV, err = b.SetVolumeID(pv, "123abc") require.NoError(t, err) - actual, err := collections.GetString(updatedPV.UnstructuredContent(), "spec.gcePersistentDisk.pdName") - require.NoError(t, err) - assert.Equal(t, "123abc", actual) + + res := new(v1.PersistentVolume) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(updatedPV.UnstructuredContent(), res)) + require.NotNil(t, res.Spec.GCEPersistentDisk) + assert.Equal(t, "123abc", res.Spec.GCEPersistentDisk.PDName) } func TestGetSnapshotTags(t *testing.T) { diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 2042f294d..331cfbfd6 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -30,6 +30,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" core "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" @@ -45,7 +46,6 @@ import ( "github.com/heptio/velero/pkg/plugin" pluginmocks "github.com/heptio/velero/pkg/plugin/mocks" "github.com/heptio/velero/pkg/restore" - "github.com/heptio/velero/pkg/util/collections" velerotest "github.com/heptio/velero/pkg/util/test" "github.com/heptio/velero/pkg/volume" ) @@ -440,11 +440,15 @@ func TestProcessRestore(t *testing.T) { return false, nil, err } - phase, err := collections.GetString(patchMap, "status.phase") + phase, found, err := unstructured.NestedString(patchMap, "status", "phase") if err != nil { t.Logf("error getting status.phase: %s\n", err) return false, nil, err } + if !found { + t.Logf("status.phase not found") + return false, nil, errors.New("status.phase not found") + } res := test.restore.DeepCopy() @@ -453,7 +457,8 @@ func TestProcessRestore(t *testing.T) { res.Status.Phase = api.RestorePhase(phase) - if backupName, err := collections.GetString(patchMap, "spec.backupName"); err == nil { + backupName, found, err := unstructured.NestedString(patchMap, "spec", "backupName") + if found { res.Spec.BackupName = backupName } diff --git a/pkg/controller/schedule_controller_test.go b/pkg/controller/schedule_controller_test.go index d490e5365..a2cbe36d8 100644 --- a/pkg/controller/schedule_controller_test.go +++ b/pkg/controller/schedule_controller_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/clock" core "k8s.io/client-go/testing" @@ -35,7 +36,6 @@ import ( "github.com/heptio/velero/pkg/generated/clientset/versioned/fake" informers "github.com/heptio/velero/pkg/generated/informers/externalversions" "github.com/heptio/velero/pkg/metrics" - "github.com/heptio/velero/pkg/util/collections" velerotest "github.com/heptio/velero/pkg/util/test" ) @@ -159,13 +159,13 @@ func TestProcessSchedule(t *testing.T) { } // these are the fields that may be updated by the controller - phase, err := collections.GetString(patchMap, "status.phase") - if err == nil { + phase, found, err := unstructured.NestedString(patchMap, "status", "phase") + if err == nil && found { res.Status.Phase = api.SchedulePhase(phase) } - lastBackupStr, err := collections.GetString(patchMap, "status.lastBackup") - if err == nil { + lastBackupStr, found, err := unstructured.NestedString(patchMap, "status", "lastBackup") + if err == nil && found { parsed, err := time.Parse(time.RFC3339, lastBackupStr) if err != nil { t.Logf("error parsing status.lastBackup: %s\n", err) diff --git a/pkg/podexec/pod_command_executor.go b/pkg/podexec/pod_command_executor.go index 08afe824d..3ee90ded8 100644 --- a/pkg/podexec/pod_command_executor.go +++ b/pkg/podexec/pod_command_executor.go @@ -23,13 +23,13 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" - kapiv1 "k8s.io/api/core/v1" + corev1api "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" kscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/client-go/tools/remotecommand" api "github.com/heptio/velero/pkg/apis/velero/v1" - "github.com/heptio/velero/pkg/util/collections" ) const defaultTimeout = 30 * time.Second @@ -83,11 +83,16 @@ func (e *defaultPodCommandExecutor) ExecutePodCommand(log logrus.FieldLogger, it return errors.New("hook is required") } + pod := new(corev1api.Pod) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(item, pod); err != nil { + return errors.WithStack(err) + } + if hook.Container == "" { - if err := setDefaultHookContainer(item, hook); err != nil { + if err := setDefaultHookContainer(pod, hook); err != nil { return err } - } else if err := ensureContainerExists(item, hook.Container); err != nil { + } else if err := ensureContainerExists(pod, hook.Container); err != nil { return err } @@ -124,7 +129,7 @@ func (e *defaultPodCommandExecutor) ExecutePodCommand(log logrus.FieldLogger, it Name(name). SubResource("exec") - req.VersionedParams(&kapiv1.PodExecOptions{ + req.VersionedParams(&corev1api.PodExecOptions{ Container: hook.Container, Command: hook.Command, Stdout: true, @@ -169,21 +174,9 @@ func (e *defaultPodCommandExecutor) ExecutePodCommand(log logrus.FieldLogger, it return err } -func ensureContainerExists(pod map[string]interface{}, container string) error { - containers, err := collections.GetSlice(pod, "spec.containers") - if err != nil { - return err - } - for _, obj := range containers { - c, ok := obj.(map[string]interface{}) - if !ok { - return errors.Errorf("unexpected type for container %T", obj) - } - name, ok := c["name"].(string) - if !ok { - return errors.Errorf("unexpected type for container name %T", c["name"]) - } - if name == container { +func ensureContainerExists(pod *corev1api.Pod, container string) error { + for _, c := range pod.Spec.Containers { + if c.Name == container { return nil } } @@ -191,26 +184,12 @@ func ensureContainerExists(pod map[string]interface{}, container string) error { return errors.Errorf("no such container: %q", container) } -func setDefaultHookContainer(pod map[string]interface{}, hook *api.ExecHook) error { - containers, err := collections.GetSlice(pod, "spec.containers") - if err != nil { - return err - } - - if len(containers) < 1 { +func setDefaultHookContainer(pod *corev1api.Pod, hook *api.ExecHook) error { + if len(pod.Spec.Containers) < 1 { return errors.New("need at least 1 container") } - container, ok := containers[0].(map[string]interface{}) - if !ok { - return errors.Errorf("unexpected type for container %T", pod) - } - - name, ok := container["name"].(string) - if !ok { - return errors.Errorf("unexpected type for container name %T", container["name"]) - } - hook.Container = name + hook.Container = pod.Spec.Containers[0].Name return nil } diff --git a/pkg/podexec/pod_command_executor_test.go b/pkg/podexec/pod_command_executor_test.go index da8e00aa3..fbdbc9fae 100644 --- a/pkg/podexec/pod_command_executor_test.go +++ b/pkg/podexec/pod_command_executor_test.go @@ -28,6 +28,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" @@ -221,11 +222,11 @@ func TestExecutePodCommand(t *testing.T) { } func TestEnsureContainerExists(t *testing.T) { - pod := map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "foo", + pod := &corev1api.Pod{ + Spec: corev1api.PodSpec{ + Containers: []corev1api.Container{ + { + Name: "foo", }, }, }, diff --git a/pkg/restore/job_action.go b/pkg/restore/job_action.go index 5e8668fd1..ae70ccf82 100644 --- a/pkg/restore/job_action.go +++ b/pkg/restore/job_action.go @@ -17,11 +17,13 @@ limitations under the License. package restore import ( + "github.com/pkg/errors" "github.com/sirupsen/logrus" + batchv1api "k8s.io/api/batch/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - api "github.com/heptio/velero/pkg/apis/velero/v1" - "github.com/heptio/velero/pkg/util/collections" + velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" ) type jobAction struct { @@ -38,21 +40,21 @@ func (a *jobAction) AppliesTo() (ResourceSelector, error) { }, nil } -func (a *jobAction) Execute(obj runtime.Unstructured, restore *api.Restore) (runtime.Unstructured, error, error) { - fieldDeletions := map[string]string{ - "spec.selector.matchLabels": "controller-uid", - "spec.template.metadata.labels": "controller-uid", +func (a *jobAction) Execute(obj runtime.Unstructured, restore *velerov1api.Restore) (runtime.Unstructured, error, error) { + job := new(batchv1api.Job) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), job); err != nil { + return nil, nil, errors.WithStack(err) } - for k, v := range fieldDeletions { - a.logger.Debugf("Getting %s", k) - labels, err := collections.GetMap(obj.UnstructuredContent(), k) - if err != nil { - a.logger.WithError(err).Debugf("Unable to get %s", k) - } else { - delete(labels, v) - } + if job.Spec.Selector != nil { + delete(job.Spec.Selector.MatchLabels, "controller-uid") + } + delete(job.Spec.Template.ObjectMeta.Labels, "controller-uid") + + res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(job) + if err != nil { + return nil, nil, errors.WithStack(err) } - return obj, nil, nil + return &unstructured.Unstructured{Object: res}, nil, nil } diff --git a/pkg/restore/job_action_test.go b/pkg/restore/job_action_test.go index d0096b892..0570935ae 100644 --- a/pkg/restore/job_action_test.go +++ b/pkg/restore/job_action_test.go @@ -20,6 +20,11 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + batchv1api "k8s.io/api/batch/v1" + corev1api "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" velerotest "github.com/heptio/velero/pkg/util/test" @@ -28,95 +33,100 @@ import ( func TestJobActionExecute(t *testing.T) { tests := []struct { name string - obj runtime.Unstructured + obj batchv1api.Job expectedErr bool - expectedRes runtime.Unstructured + expectedRes batchv1api.Job }{ { name: "missing spec.selector and/or spec.template should not error", - obj: NewTestUnstructured().WithName("job-1"). - WithSpec(). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("job-1"). - WithSpec(). - Unstructured, + obj: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + }, + expectedRes: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + }, }, { name: "missing spec.selector.matchLabels should not error", - obj: NewTestUnstructured().WithName("job-1"). - WithSpecField("selector", map[string]interface{}{}). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("job-1"). - WithSpecField("selector", map[string]interface{}{}). - Unstructured, + obj: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: batchv1api.JobSpec{ + Selector: new(metav1.LabelSelector), + }, + }, + expectedRes: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: batchv1api.JobSpec{ + Selector: new(metav1.LabelSelector), + }, + }, }, { name: "spec.selector.matchLabels[controller-uid] is removed", - obj: NewTestUnstructured().WithName("job-1"). - WithSpecField("selector", map[string]interface{}{ - "matchLabels": map[string]interface{}{ - "controller-uid": "foo", - "hello": "world", - }, - }). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("job-1"). - WithSpecField("selector", map[string]interface{}{ - "matchLabels": map[string]interface{}{ - "hello": "world", - }, - }). - Unstructured, - }, - { - name: "missing spec.template.metadata should not error", - obj: NewTestUnstructured().WithName("job-1"). - WithSpecField("template", map[string]interface{}{}). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("job-1"). - WithSpecField("template", map[string]interface{}{}). - Unstructured, - }, - { - name: "missing spec.template.metadata.labels should not error", - obj: NewTestUnstructured().WithName("job-1"). - WithSpecField("template", map[string]interface{}{ - "metadata": map[string]interface{}{}, - }). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("job-1"). - WithSpecField("template", map[string]interface{}{ - "metadata": map[string]interface{}{}, - }). - Unstructured, - }, - { - name: "spec.template.metadata.labels[controller-uid] is removed", - obj: NewTestUnstructured().WithName("job-1"). - WithSpecField("template", map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ + obj: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: batchv1api.JobSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ "controller-uid": "foo", "hello": "world", }, }, - }). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("job-1"). - WithSpecField("template", map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ + }, + }, + expectedRes: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: batchv1api.JobSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ "hello": "world", }, }, - }). - Unstructured, + }, + }, + }, + { + name: "missing spec.template.metadata.labels should not error", + obj: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: batchv1api.JobSpec{ + Template: corev1api.PodTemplateSpec{}, + }, + }, + expectedRes: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: batchv1api.JobSpec{ + Template: corev1api.PodTemplateSpec{}, + }, + }, + }, + { + name: "spec.template.metadata.labels[controller-uid] is removed", + obj: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: batchv1api.JobSpec{ + Template: corev1api.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "controller-uid": "foo", + "hello": "world", + }, + }, + }, + }, + }, + expectedRes: batchv1api.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: batchv1api.JobSpec{ + Template: corev1api.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "hello": "world", + }, + }, + }, + }, + }, }, } @@ -124,10 +134,16 @@ func TestJobActionExecute(t *testing.T) { t.Run(test.name, func(t *testing.T) { action := NewJobAction(velerotest.NewLogger()) - res, _, err := action.Execute(test.obj, nil) + unstructuredJob, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&test.obj) + require.NoError(t, err) + + res, _, err := action.Execute(&unstructured.Unstructured{Object: unstructuredJob}, nil) if assert.Equal(t, test.expectedErr, err != nil) { - assert.Equal(t, test.expectedRes, res) + var job batchv1api.Job + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(res.UnstructuredContent(), &job)) + + assert.Equal(t, test.expectedRes, job) } }) } diff --git a/pkg/restore/merge_service_account.go b/pkg/restore/merge_service_account.go index 53b05ab75..8bd3d8c9c 100644 --- a/pkg/restore/merge_service_account.go +++ b/pkg/restore/merge_service_account.go @@ -24,8 +24,6 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - - "github.com/heptio/velero/pkg/util/collections" ) // mergeServiceAccount takes a backed up serviceaccount and merges attributes into the current in-cluster service account. @@ -46,9 +44,9 @@ func mergeServiceAccounts(fromCluster, fromBackup *unstructured.Unstructured) (* desired.ImagePullSecrets = mergeLocalObjectReferenceSlices(desired.ImagePullSecrets, backupSA.ImagePullSecrets) - desired.Labels = collections.MergeMaps(desired.Labels, backupSA.Labels) + desired.Labels = mergeMaps(desired.Labels, backupSA.Labels) - desired.Annotations = collections.MergeMaps(desired.Annotations, backupSA.Annotations) + desired.Annotations = mergeMaps(desired.Annotations, backupSA.Annotations) desiredUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(desired) if err != nil { @@ -95,6 +93,24 @@ func mergeLocalObjectReferenceSlices(first, second []corev1api.LocalObjectRefere return first } +// 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) map[string]string { + // If the first map passed in is empty, just use all of the second map's data + if first == nil { + first = map[string]string{} + } + + for k, v := range second { + _, ok := first[k] + if !ok { + first[k] = v + } + } + + 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) { diff --git a/pkg/restore/merge_service_account_test.go b/pkg/restore/merge_service_account_test.go index 0e5a04452..038fdf9d2 100644 --- a/pkg/restore/merge_service_account_test.go +++ b/pkg/restore/merge_service_account_test.go @@ -315,6 +315,60 @@ func stripWhitespace(s string) string { }, s) } +func TestMergeMaps(t *testing.T) { + var testCases = []struct { + name string + source map[string]string + destination map[string]string + expected map[string]string + }{ + { + name: "nil destination should result in source being copied", + destination: nil, + source: map[string]string{ + "k1": "v1", + }, + expected: map[string]string{ + "k1": "v1", + }, + }, + { + name: "keys missing from destination should be copied from source", + destination: map[string]string{ + "k2": "v2", + }, + source: map[string]string{ + "k1": "v1", + }, + expected: map[string]string{ + "k1": "v1", + "k2": "v2", + }, + }, + { + name: "matching key should not have value copied from source", + destination: map[string]string{ + "k1": "v1", + }, + source: map[string]string{ + "k1": "v2", + }, + expected: map[string]string{ + "k1": "v1", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + + result := mergeMaps(tc.destination, tc.source) + + assert.Equal(t, tc.expected, result) + }) + } +} + func TestGeneratePatch(t *testing.T) { tests := []struct { name string diff --git a/pkg/restore/pod_action.go b/pkg/restore/pod_action.go index c7748c412..6b6dc9b85 100644 --- a/pkg/restore/pod_action.go +++ b/pkg/restore/pod_action.go @@ -19,11 +19,13 @@ package restore import ( "strings" + "github.com/pkg/errors" "github.com/sirupsen/logrus" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" api "github.com/heptio/velero/pkg/apis/velero/v1" - "github.com/heptio/velero/pkg/util/collections" ) type podAction struct { @@ -41,94 +43,48 @@ func (a *podAction) AppliesTo() (ResourceSelector, error) { } func (a *podAction) Execute(obj runtime.Unstructured, restore *api.Restore) (runtime.Unstructured, error, error) { - a.logger.Debug("getting spec") - spec, err := collections.GetMap(obj.UnstructuredContent(), "spec") + pod := new(v1.Pod) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), pod); err != nil { + return nil, nil, errors.WithStack(err) + } + + pod.Spec.NodeName = "" + pod.Spec.Priority = nil + + serviceAccountTokenPrefix := pod.Spec.ServiceAccountName + "-token-" + + var preservedVolumes []v1.Volume + for _, vol := range pod.Spec.Volumes { + if !strings.HasPrefix(vol.Name, serviceAccountTokenPrefix) { + preservedVolumes = append(preservedVolumes, vol) + } + } + pod.Spec.Volumes = preservedVolumes + + for i, container := range pod.Spec.Containers { + var preservedVolumeMounts []v1.VolumeMount + for _, mount := range container.VolumeMounts { + if !strings.HasPrefix(mount.Name, serviceAccountTokenPrefix) { + preservedVolumeMounts = append(preservedVolumeMounts, mount) + } + } + pod.Spec.Containers[i].VolumeMounts = preservedVolumeMounts + } + + for i, container := range pod.Spec.InitContainers { + var preservedVolumeMounts []v1.VolumeMount + for _, mount := range container.VolumeMounts { + if !strings.HasPrefix(mount.Name, serviceAccountTokenPrefix) { + preservedVolumeMounts = append(preservedVolumeMounts, mount) + } + } + pod.Spec.InitContainers[i].VolumeMounts = preservedVolumeMounts + } + + res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) if err != nil { - return nil, nil, err + return nil, nil, errors.WithStack(err) } - a.logger.Debug("deleting spec.NodeName") - delete(spec, "nodeName") - - a.logger.Debug("deleting spec.priority") - delete(spec, "priority") - - // if there are no volumes, then there can't be any volume mounts, so we're done. - if !collections.Exists(spec, "volumes") { - return obj, nil, nil - } - - serviceAccountName, err := collections.GetString(spec, "serviceAccountName") - if err != nil { - return nil, nil, err - } - prefix := serviceAccountName + "-token-" - - // remove the service account token from volumes - a.logger.Debug("iterating over volumes") - if err := removeItemsWithNamePrefix(spec, "volumes", prefix, a.logger); err != nil { - return nil, nil, err - } - - // remove the service account token volume mount from all containers - a.logger.Debug("iterating over containers") - if err := removeVolumeMounts(spec, "containers", prefix, a.logger); err != nil { - return nil, nil, err - } - - if !collections.Exists(spec, "initContainers") { - return obj, nil, nil - } - - // remove the service account token volume mount from all init containers - a.logger.Debug("iterating over init containers") - if err := removeVolumeMounts(spec, "initContainers", prefix, a.logger); err != nil { - return nil, nil, err - } - - return obj, nil, nil -} - -// removeItemsWithNamePrefix iterates through the collection stored at 'key' in 'unstructuredObj' -// and removes any item that has a name that starts with 'prefix'. -func removeItemsWithNamePrefix(unstructuredObj map[string]interface{}, key, prefix string, log logrus.FieldLogger) error { - var preservedItems []interface{} - - if err := collections.ForEach(unstructuredObj, key, func(item map[string]interface{}) error { - name, err := collections.GetString(item, "name") - if err != nil { - return err - } - - singularKey := strings.TrimSuffix(key, "s") - log := log.WithField(singularKey, name) - - log.Debug("Checking " + singularKey) - switch { - case strings.HasPrefix(name, prefix): - log.Debug("Excluding ", singularKey) - default: - log.Debug("Preserving ", singularKey) - preservedItems = append(preservedItems, item) - } - - return nil - }); err != nil { - return err - } - - unstructuredObj[key] = preservedItems - return nil -} - -// removeVolumeMounts iterates through a slice of containers stored at 'containersKey' in -// 'podSpec' and removes any volume mounts with a name starting with 'prefix'. -func removeVolumeMounts(podSpec map[string]interface{}, containersKey, prefix string, log logrus.FieldLogger) error { - return collections.ForEach(podSpec, containersKey, func(container map[string]interface{}) error { - if !collections.Exists(container, "volumeMounts") { - return nil - } - - return removeItemsWithNamePrefix(container, "volumeMounts", prefix, log) - }) + return &unstructured.Unstructured{Object: res}, nil, nil } diff --git a/pkg/restore/pod_action_test.go b/pkg/restore/pod_action_test.go index 1a5409000..02cfdbe63 100644 --- a/pkg/restore/pod_action_test.go +++ b/pkg/restore/pod_action_test.go @@ -20,148 +20,173 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1api "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" velerotest "github.com/heptio/velero/pkg/util/test" ) func TestPodActionExecute(t *testing.T) { + var priority int32 = 1 + tests := []struct { name string - obj runtime.Unstructured + obj corev1api.Pod expectedErr bool - expectedRes runtime.Unstructured + expectedRes corev1api.Pod }{ - { - name: "no spec should error", - obj: NewTestUnstructured().WithName("pod-1").Unstructured, - expectedErr: true, - }, { name: "nodeName (only) should be deleted from spec", - obj: NewTestUnstructured().WithName("pod-1").WithSpec("nodeName", "foo"). - WithSpecField("containers", []interface{}{}). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("pod-1").WithSpec("foo"). - WithSpecField("containers", []interface{}{}). - Unstructured, + obj: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + NodeName: "foo", + ServiceAccountName: "bar", + }, + }, + expectedRes: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "bar", + }, + }, }, { name: "priority (only) should be deleted from spec", - obj: NewTestUnstructured().WithName("pod-1").WithSpec("priority", "foo"). - WithSpecField("containers", []interface{}{}). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("pod-1").WithSpec("foo"). - WithSpecField("containers", []interface{}{}). - Unstructured, + obj: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + Priority: &priority, + ServiceAccountName: "bar", + }, + }, + expectedRes: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "bar", + }, + }, }, { name: "volumes matching prefix -token- should be deleted", - obj: NewTestUnstructured().WithName("pod-1"). - WithSpec("serviceAccountName", "foo"). - WithSpecField("volumes", []interface{}{ - map[string]interface{}{"name": "foo"}, - map[string]interface{}{"name": "foo-token-foo"}, - }). - WithSpecField("containers", []interface{}{}). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("pod-1"). - WithSpec("serviceAccountName", "foo"). - WithSpecField("volumes", []interface{}{ - map[string]interface{}{"name": "foo"}, - }). - WithSpecField("containers", []interface{}{}). - Unstructured, + obj: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "foo", + Volumes: []corev1api.Volume{ + {Name: "foo"}, + {Name: "foo-token-foo"}, + }, + }, + }, + expectedRes: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "foo", + Volumes: []corev1api.Volume{ + {Name: "foo"}, + }, + }, + }, }, { name: "container volumeMounts matching prefix -token- should be deleted", - obj: NewTestUnstructured().WithName("svc-1"). - WithSpec("serviceAccountName", "foo"). - WithSpecField("volumes", []interface{}{ - map[string]interface{}{"name": "foo"}, - map[string]interface{}{"name": "foo-token-foo"}, - }). - WithSpecField("containers", []interface{}{ - map[string]interface{}{ - "volumeMounts": []interface{}{ - map[string]interface{}{"name": "foo"}, - map[string]interface{}{"name": "foo-token-foo"}, + obj: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "foo", + Volumes: []corev1api.Volume{ + {Name: "foo"}, + {Name: "foo-token-foo"}, + }, + Containers: []corev1api.Container{ + { + VolumeMounts: []corev1api.VolumeMount{ + {Name: "foo"}, + {Name: "foo-token-foo"}, + }, }, }, - }). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1"). - WithSpec("serviceAccountName", "foo"). - WithSpecField("volumes", []interface{}{ - map[string]interface{}{"name": "foo"}, - }). - WithSpecField("containers", []interface{}{ - map[string]interface{}{ - "volumeMounts": []interface{}{ - map[string]interface{}{"name": "foo"}, + }, + }, + expectedRes: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "foo", + Volumes: []corev1api.Volume{ + {Name: "foo"}, + }, + Containers: []corev1api.Container{ + { + VolumeMounts: []corev1api.VolumeMount{ + {Name: "foo"}, + }, }, }, - }). - Unstructured, + }, + }, }, { name: "initContainer volumeMounts matching prefix -token- should be deleted", - obj: NewTestUnstructured().WithName("svc-1"). - WithSpec("serviceAccountName", "foo"). - WithSpecField("containers", []interface{}{}). - WithSpecField("volumes", []interface{}{ - map[string]interface{}{"name": "foo"}, - map[string]interface{}{"name": "foo-token-foo"}, - }). - WithSpecField("initContainers", []interface{}{ - map[string]interface{}{ - "volumeMounts": []interface{}{ - map[string]interface{}{"name": "foo"}, - map[string]interface{}{"name": "foo-token-foo"}, + obj: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "foo", + Volumes: []corev1api.Volume{ + {Name: "foo"}, + {Name: "foo-token-foo"}, + }, + InitContainers: []corev1api.Container{ + { + VolumeMounts: []corev1api.VolumeMount{ + {Name: "foo"}, + {Name: "foo-token-foo"}, + }, }, }, - }). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1"). - WithSpec("serviceAccountName", "foo"). - WithSpecField("containers", []interface{}{}). - WithSpecField("volumes", []interface{}{ - map[string]interface{}{"name": "foo"}, - }). - WithSpecField("initContainers", []interface{}{ - map[string]interface{}{ - "volumeMounts": []interface{}{ - map[string]interface{}{"name": "foo"}, + }, + }, + expectedRes: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "foo", + Volumes: []corev1api.Volume{ + {Name: "foo"}, + }, + InitContainers: []corev1api.Container{ + { + VolumeMounts: []corev1api.VolumeMount{ + {Name: "foo"}, + }, }, }, - }). - Unstructured, + }, + }, }, { name: "containers and initContainers with no volume mounts should not error", - obj: NewTestUnstructured().WithName("pod-1"). - WithSpec("serviceAccountName", "foo"). - WithSpecField("volumes", []interface{}{ - map[string]interface{}{"name": "foo"}, - map[string]interface{}{"name": "foo-token-foo"}, - }). - WithSpecField("containers", []interface{}{}). - WithSpecField("initContainers", []interface{}{}). - Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("pod-1"). - WithSpec("serviceAccountName", "foo"). - WithSpecField("volumes", []interface{}{ - map[string]interface{}{"name": "foo"}, - }). - WithSpecField("containers", []interface{}{}). - WithSpecField("initContainers", []interface{}{}). - Unstructured, + obj: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "foo", + Volumes: []corev1api.Volume{ + {Name: "foo"}, + {Name: "foo-token-foo"}, + }, + }, + }, + expectedRes: corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + Spec: corev1api.PodSpec{ + ServiceAccountName: "foo", + Volumes: []corev1api.Volume{ + {Name: "foo"}, + }, + }, + }, }, } @@ -169,8 +194,10 @@ func TestPodActionExecute(t *testing.T) { t.Run(test.name, func(t *testing.T) { action := NewPodAction(velerotest.NewLogger()) - res, warning, err := action.Execute(test.obj, nil) + unstructuredPod, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&test.obj) + require.NoError(t, err) + res, warning, err := action.Execute(&unstructured.Unstructured{Object: unstructuredPod}, nil) assert.Nil(t, warning) if test.expectedErr { @@ -179,7 +206,10 @@ func TestPodActionExecute(t *testing.T) { assert.Nil(t, err, "expected no error, got %v", err) } - assert.Equal(t, test.expectedRes, res) + var pod corev1api.Pod + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(res.UnstructuredContent(), &pod)) + + assert.Equal(t, test.expectedRes, pod) }) } } diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 7ed090db5..473d0bb3d 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -589,7 +589,7 @@ func (ctx *context) shouldRestore(name string, pvClient client.Dynamic) (bool, e var shouldRestore bool err := wait.PollImmediate(time.Second, ctx.resourceTerminatingTimeout, func() (bool, error) { - clusterPV, err := pvClient.Get(name, metav1.GetOptions{}) + unstructuredPV, err := pvClient.Get(name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { pvLogger.Debug("PV not found, safe to restore") // PV not found, can safely exit loop and proceed with restore. @@ -598,15 +598,14 @@ func (ctx *context) shouldRestore(name string, pvClient client.Dynamic) (bool, e } if err != nil { return false, errors.Wrapf(err, "could not retrieve in-cluster copy of PV %s", name) - - } - phase, err := collections.GetString(clusterPV.UnstructuredContent(), "status.phase") - if err != nil { - // Break the loop since we couldn't read the phase - return false, errors.Wrapf(err, "error getting phase for in-cluster PV %s", name) } - if phase == string(v1.VolumeReleased) || clusterPV.GetDeletionTimestamp() != nil { + clusterPV := new(v1.PersistentVolume) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPV.Object, clusterPV); err != nil { + return false, errors.Wrap(err, "error converting PV from unstructured") + } + + if clusterPV.Status.Phase == v1.VolumeReleased || clusterPV.DeletionTimestamp != nil { // PV was found and marked for deletion, or it was released; wait for it to go away. pvLogger.Debugf("PV found, but marked for deletion, waiting") return false, nil @@ -617,15 +616,14 @@ func (ctx *context) shouldRestore(name string, pvClient client.Dynamic) (bool, e // trying to restore the PV // Not doing so may result in the underlying PV disappearing but not restoring due to timing issues, // then the PVC getting restored and showing as lost. - namespace, err := collections.GetString(clusterPV.UnstructuredContent(), "spec.claimRef.namespace") - if err != nil { - return false, errors.Wrapf(err, "error looking up namespace name for in-cluster PV %s", name) - } - pvcName, err := collections.GetString(clusterPV.UnstructuredContent(), "spec.claimRef.name") - if err != nil { - return false, errors.Wrapf(err, "error looking up persistentvolumeclaim for in-cluster PV %s", name) + if clusterPV.Spec.ClaimRef == nil { + pvLogger.Debugf("PV is not marked for deletion and is not claimed by a PVC") + return true, nil } + namespace := clusterPV.Spec.ClaimRef.Namespace + pvcName := clusterPV.Spec.ClaimRef.Name + // Have to create the PVC client here because we don't know what namespace we're using til we get to this point. // Using a dynamic client since it's easier to mock for testing pvcResource := metav1.APIResource{Name: "persistentvolumeclaims", Namespaced: true} @@ -635,7 +633,6 @@ func (ctx *context) shouldRestore(name string, pvClient client.Dynamic) (bool, e } pvc, err := pvcClient.Get(pvcName, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { pvLogger.Debugf("PVC %s for PV not found, waiting", pvcName) // PVC wasn't found, but the PV still exists, so continue to wait. @@ -839,21 +836,25 @@ func (ctx *context) restoreResource(resource, namespace, resourcePath string) (a } if groupResource == kuberesource.PersistentVolumeClaims { - spec, err := collections.GetMap(obj.UnstructuredContent(), "spec") - if err != nil { + pvc := new(v1.PersistentVolumeClaim) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), pvc); err != nil { addToResult(&errs, namespace, err) continue } - if volumeName, exists := spec["volumeName"]; exists && ctx.pvsToProvision.Has(volumeName.(string)) { - ctx.log.Infof("Resetting PersistentVolumeClaim %s/%s for dynamic provisioning because its PV %v has a reclaim policy of Delete", namespace, name, volumeName) + if pvc.Spec.VolumeName != "" && ctx.pvsToProvision.Has(pvc.Spec.VolumeName) { + ctx.log.Infof("Resetting PersistentVolumeClaim %s/%s for dynamic provisioning because its PV %v has a reclaim policy of Delete", namespace, name, pvc.Spec.VolumeName) - delete(spec, "volumeName") + pvc.Spec.VolumeName = "" + delete(pvc.Annotations, "pv.kubernetes.io/bind-completed") + delete(pvc.Annotations, "pv.kubernetes.io/bound-by-controller") - annotations := obj.GetAnnotations() - delete(annotations, "pv.kubernetes.io/bind-completed") - delete(annotations, "pv.kubernetes.io/bound-by-controller") - obj.SetAnnotations(annotations) + res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pvc) + if err != nil { + addToResult(&errs, namespace, err) + continue + } + obj.Object = res } } @@ -992,12 +993,8 @@ func (ctx *context) restoreResource(resource, namespace, resourcePath string) (a } func hasDeleteReclaimPolicy(obj map[string]interface{}) bool { - reclaimPolicy, err := collections.GetString(obj, "spec.persistentVolumeReclaimPolicy") - if err != nil { - return false - } - - return reclaimPolicy == "Delete" + policy, _, _ := unstructured.NestedString(obj, "spec", "persistentVolumeReclaimPolicy") + return policy == string(v1.PersistentVolumeReclaimDelete) } func waitForReady( @@ -1120,9 +1117,16 @@ func (r *pvRestorer) executePVAction(obj *unstructured.Unstructured) (*unstructu return nil, errors.New("PersistentVolume is missing its name") } - spec, err := collections.GetMap(obj.UnstructuredContent(), "spec") - if err != nil { - return nil, errors.WithStack(err) + // It's simpler to just access the spec through the unstructured object than to convert + // to structured and back here, especially since the SetVolumeID(...) call below needs + // the unstructured representation (and does a conversion internally). + res, ok := obj.Object["spec"] + if !ok { + return nil, errors.New("spec not found") + } + spec, ok := res.(map[string]interface{}) + if !ok { + return nil, errors.Errorf("spec was of type %T, expected map[string]interface{}", res) } delete(spec, "claimRef") @@ -1177,18 +1181,18 @@ func (r *pvRestorer) executePVAction(obj *unstructured.Unstructured) (*unstructu } func isPVReady(obj runtime.Unstructured) bool { - phase, err := collections.GetString(obj.UnstructuredContent(), "status.phase") - if err != nil { - return false - } - + phase, _, _ := unstructured.NestedString(obj.UnstructuredContent(), "status", "phase") return phase == string(v1.VolumeAvailable) } func resetMetadataAndStatus(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { - metadata, err := collections.GetMap(obj.UnstructuredContent(), "metadata") - if err != nil { - return nil, err + res, ok := obj.Object["metadata"] + if !ok { + return nil, errors.New("metadata not found") + } + metadata, ok := res.(map[string]interface{}) + if !ok { + return nil, errors.Errorf("metadata was of type %T, expected map[string]interface{}", res) } for k := range metadata { diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 475365b9b..7fdb50bc7 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -1054,9 +1054,7 @@ status: pvClient.On("Watch", metav1.ListOptions{}).Return(pvWatch, nil) pvWatchChan := make(chan watch.Event, 1) readyPV := restoredPV.DeepCopy() - readyStatus, err := collections.GetMap(readyPV.Object, "status") - require.NoError(t, err) - readyStatus["phase"] = string(v1.VolumeAvailable) + require.NoError(t, unstructured.SetNestedField(readyPV.UnstructuredContent(), string(v1.VolumeAvailable), "status", "phase")) pvWatchChan <- watch.Event{ Type: watch.Modified, Object: readyPV, @@ -1777,9 +1775,7 @@ status: // Set up test expectations if test.pvPhase != "" { - status, err := collections.GetMap(pvObj.UnstructuredContent(), "status") - require.NoError(t, err) - status["phase"] = test.pvPhase + require.NoError(t, unstructured.SetNestedField(pvObj.Object, test.pvPhase, "status", "phase")) } if test.expectPVFound { @@ -2135,16 +2131,19 @@ func (r *fakeAction) AppliesTo() (ResourceSelector, error) { } func (r *fakeAction) Execute(obj runtime.Unstructured, restore *api.Restore) (runtime.Unstructured, error, error) { - metadata, err := collections.GetMap(obj.UnstructuredContent(), "metadata") + labels, found, err := unstructured.NestedMap(obj.UnstructuredContent(), "metadata", "labels") if err != nil { return nil, nil, err } - - if _, found := metadata["labels"]; !found { - metadata["labels"] = make(map[string]interface{}) + if !found { + labels = make(map[string]interface{}) } - metadata["labels"].(map[string]interface{})["fake-restorer"] = "foo" + labels["fake-restorer"] = "foo" + + if err := unstructured.SetNestedField(obj.UnstructuredContent(), labels, "metadata", "labels"); err != nil { + return nil, nil, err + } unstructuredObj, ok := obj.(*unstructured.Unstructured) if !ok { diff --git a/pkg/restore/service_action.go b/pkg/restore/service_action.go index e425ba1c3..a271e6e1c 100644 --- a/pkg/restore/service_action.go +++ b/pkg/restore/service_action.go @@ -22,11 +22,11 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" corev1api "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/sets" api "github.com/heptio/velero/pkg/apis/velero/v1" - "github.com/heptio/velero/pkg/util/collections" ) const annotationLastAppliedConfig = "kubectl.kubernetes.io/last-applied-configuration" @@ -46,67 +46,55 @@ func (a *serviceAction) AppliesTo() (ResourceSelector, error) { } func (a *serviceAction) Execute(obj runtime.Unstructured, restore *api.Restore) (runtime.Unstructured, error, error) { - spec, err := collections.GetMap(obj.UnstructuredContent(), "spec") - if err != nil { + service := new(corev1api.Service) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), service); err != nil { + return nil, nil, errors.WithStack(err) + } + + if service.Spec.ClusterIP != "None" { + service.Spec.ClusterIP = "" + } + + if err := deleteNodePorts(service); err != nil { return nil, nil, err } - // Since clusterIP is an optional key, we can ignore 'not found' errors. Also assuming it was a string already. - if val, _ := collections.GetString(spec, "clusterIP"); val != "None" { - delete(spec, "clusterIP") - } - - if err := deleteNodePorts(obj, &spec); err != nil { - return nil, nil, err - } - return obj, nil, nil -} - -func getPreservedPorts(obj runtime.Unstructured) (map[string]bool, error) { - preservedPorts := map[string]bool{} - metadata, err := meta.Accessor(obj) + res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(service) if err != nil { - return nil, errors.WithStack(err) + return nil, nil, errors.WithStack(err) } - if lac, ok := metadata.GetAnnotations()[annotationLastAppliedConfig]; ok { - var svc corev1api.Service - if err := json.Unmarshal([]byte(lac), &svc); err != nil { - return nil, errors.WithStack(err) - } - for _, port := range svc.Spec.Ports { - if port.NodePort > 0 { - preservedPorts[port.Name] = true - } - } - } - return preservedPorts, nil + + return &unstructured.Unstructured{Object: res}, nil, nil } -func deleteNodePorts(obj runtime.Unstructured, spec *map[string]interface{}) error { - if serviceType, _ := collections.GetString(*spec, "type"); serviceType == "ExternalName" { +func deleteNodePorts(service *corev1api.Service) error { + if service.Spec.Type == corev1api.ServiceTypeExternalName { return nil } - preservedPorts, err := getPreservedPorts(obj) - if err != nil { - return err + // find any NodePorts whose values were explicitly specified according + // to the last-applied-config annotation. We'll retain these values, and + // clear out any other (presumably auto-assigned) NodePort values. + explicitNodePorts := sets.NewString() + lastAppliedConfig, ok := service.Annotations[annotationLastAppliedConfig] + if ok { + appliedService := new(corev1api.Service) + if err := json.Unmarshal([]byte(lastAppliedConfig), appliedService); err != nil { + return errors.WithStack(err) + } + + for _, port := range appliedService.Spec.Ports { + if port.NodePort > 0 { + explicitNodePorts.Insert(port.Name) + } + } } - ports, err := collections.GetSlice(obj.UnstructuredContent(), "spec.ports") - if err != nil { - return err + for i, port := range service.Spec.Ports { + if !explicitNodePorts.Has(port.Name) { + service.Spec.Ports[i].NodePort = 0 + } } - for _, port := range ports { - p := port.(map[string]interface{}) - var name string - if nameVal, ok := p["name"]; ok { - name = nameVal.(string) - } - if preservedPorts[name] { - continue - } - delete(p, "nodePort") - } return nil } diff --git a/pkg/restore/service_action_test.go b/pkg/restore/service_action_test.go index 815591ccb..c1f629936 100644 --- a/pkg/restore/service_action_test.go +++ b/pkg/restore/service_action_test.go @@ -21,7 +21,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1api "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" velerotest "github.com/heptio/velero/pkg/util/test" @@ -46,136 +49,221 @@ func TestServiceActionExecute(t *testing.T) { tests := []struct { name string - obj runtime.Unstructured + obj corev1api.Service expectedErr bool - expectedRes runtime.Unstructured + expectedRes corev1api.Service }{ { - name: "no spec should error", - obj: NewTestUnstructured().WithName("svc-1").Unstructured, - expectedErr: true, - }, - { - name: "no spec ports should error", - obj: NewTestUnstructured().WithName("svc-1").WithSpec().Unstructured, - expectedErr: true, - }, - { - name: "clusterIP (only) should be deleted from spec", - obj: NewTestUnstructured().WithName("svc-1").WithSpec("clusterIP", "foo").WithSpecField("ports", []interface{}{}).Unstructured, + name: "clusterIP (only) should be deleted from spec", + obj: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + }, + Spec: corev1api.ServiceSpec{ + ClusterIP: "should-be-removed", + LoadBalancerIP: "should-be-kept", + }, + }, expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1").WithSpec("foo").WithSpecField("ports", []interface{}{}).Unstructured, + expectedRes: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + }, + Spec: corev1api.ServiceSpec{ + LoadBalancerIP: "should-be-kept", + }, + }, }, { - name: "headless clusterIP should not be deleted from spec", - obj: NewTestUnstructured().WithName("svc-1").WithSpecField("clusterIP", "None").WithSpecField("ports", []interface{}{}).Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1").WithSpecField("clusterIP", "None").WithSpecField("ports", []interface{}{}).Unstructured, + name: "headless clusterIP should not be deleted from spec", + obj: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + }, + Spec: corev1api.ServiceSpec{ + ClusterIP: "None", + }, + }, + expectedRes: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + }, + Spec: corev1api.ServiceSpec{ + ClusterIP: "None", + }, + }, }, { name: "nodePort (only) should be deleted from all spec.ports", - obj: NewTestUnstructured().WithName("svc-1"). - WithSpecField("ports", []interface{}{ - map[string]interface{}{"nodePort": ""}, - map[string]interface{}{"nodePort": "", "foo": "bar"}, - }).Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1"). - WithSpecField("ports", []interface{}{ - map[string]interface{}{}, - map[string]interface{}{"foo": "bar"}, - }).Unstructured, + obj: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + { + Port: 32000, + NodePort: 32000, + }, + { + Port: 32001, + NodePort: 32001, + }, + }, + }, + }, + expectedRes: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + { + Port: 32000, + }, + { + Port: 32001, + }, + }, + }, + }, }, { name: "unnamed nodePort should be deleted when missing in annotation", - obj: NewTestUnstructured().WithName("svc-1"). - WithAnnotationValues(map[string]string{ - annotationLastAppliedConfig: svcJSON(), - }). - WithSpecField("ports", []interface{}{ - map[string]interface{}{"nodePort": 8080}, - }).Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1"). - WithAnnotationValues(map[string]string{ - annotationLastAppliedConfig: svcJSON(), - }). - WithSpecField("ports", []interface{}{ - map[string]interface{}{}, - }).Unstructured, + obj: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Annotations: map[string]string{ + annotationLastAppliedConfig: svcJSON(), + }, + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + { + NodePort: 8080, + }, + }, + }, + }, + expectedRes: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Annotations: map[string]string{ + annotationLastAppliedConfig: svcJSON(), + }, + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + {}, + }, + }, + }, }, { name: "unnamed nodePort should be preserved when specified in annotation", - obj: NewTestUnstructured().WithName("svc-1"). - WithAnnotationValues(map[string]string{ - annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{NodePort: 8080}), - }). - WithSpecField("ports", []interface{}{ - map[string]interface{}{ - "nodePort": 8080, + obj: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Annotations: map[string]string{ + annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{NodePort: 8080}), }, - }).Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1"). - WithAnnotationValues(map[string]string{ - annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{NodePort: 8080}), - }). - WithSpecField("ports", []interface{}{ - map[string]interface{}{ - "nodePort": 8080, + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + { + NodePort: 8080, + }, }, - }).Unstructured, + }, + }, + expectedRes: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Annotations: map[string]string{ + annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{NodePort: 8080}), + }, + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + { + NodePort: 8080, + }, + }, + }, + }, }, { name: "unnamed nodePort should be deleted when named nodePort specified in annotation", - obj: NewTestUnstructured().WithName("svc-1"). - WithAnnotationValues(map[string]string{ - annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}), - }). - WithSpecField("ports", []interface{}{ - map[string]interface{}{ - "nodePort": 8080, + obj: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Annotations: map[string]string{ + annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}), }, - }).Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1"). - WithAnnotationValues(map[string]string{ - annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}), - }). - WithSpecField("ports", []interface{}{ - map[string]interface{}{}, - }).Unstructured, + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + { + NodePort: 8080, + }, + }, + }, + }, + expectedRes: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Annotations: map[string]string{ + annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}), + }, + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + {}, + }, + }, + }, }, { name: "named nodePort should be preserved when specified in annotation", - obj: NewTestUnstructured().WithName("svc-1"). - WithAnnotationValues(map[string]string{ - annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}), - }). - WithSpecField("ports", []interface{}{ - map[string]interface{}{ - "name": "http", - "nodePort": 8080, + obj: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Annotations: map[string]string{ + annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}), }, - map[string]interface{}{ - "name": "admin", - "nodePort": 9090, + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + { + Name: "http", + NodePort: 8080, + }, + { + Name: "admin", + NodePort: 9090, + }, }, - }).Unstructured, - expectedErr: false, - expectedRes: NewTestUnstructured().WithName("svc-1"). - WithAnnotationValues(map[string]string{ - annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}), - }). - WithSpecField("ports", []interface{}{ - map[string]interface{}{ - "name": "http", - "nodePort": 8080, + }, + }, + expectedRes: corev1api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Annotations: map[string]string{ + annotationLastAppliedConfig: svcJSON(corev1api.ServicePort{Name: "http", NodePort: 8080}), }, - map[string]interface{}{ - "name": "admin", + }, + Spec: corev1api.ServiceSpec{ + Ports: []corev1api.ServicePort{ + { + Name: "http", + NodePort: 8080, + }, + { + Name: "admin", + }, }, - }).Unstructured, + }, + }, }, } @@ -183,10 +271,16 @@ func TestServiceActionExecute(t *testing.T) { t.Run(test.name, func(t *testing.T) { action := NewServiceAction(velerotest.NewLogger()) - res, _, err := action.Execute(test.obj, nil) + unstructuredSvc, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&test.obj) + require.NoError(t, err) + + res, _, err := action.Execute(&unstructured.Unstructured{Object: unstructuredSvc}, nil) if assert.Equal(t, test.expectedErr, err != nil) { - assert.Equal(t, test.expectedRes, res) + var svc corev1api.Service + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(res.UnstructuredContent(), &svc)) + + assert.Equal(t, test.expectedRes, svc) } }) } diff --git a/pkg/util/collections/map_utils.go b/pkg/util/collections/map_utils.go deleted file mode 100644 index 6ddfc8c3c..000000000 --- a/pkg/util/collections/map_utils.go +++ /dev/null @@ -1,142 +0,0 @@ -/* -Copyright 2017 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 collections - -import ( - "strings" - - "github.com/pkg/errors" -) - -// GetValue returns the object at root[path], where path is a dot separated string. -func GetValue(root map[string]interface{}, path string) (interface{}, error) { - if root == nil { - return "", errors.New("root is nil") - } - - pathParts := strings.Split(path, ".") - key := pathParts[0] - - obj, found := root[pathParts[0]] - if !found { - return "", errors.Errorf("key %v not found", pathParts[0]) - } - - if len(pathParts) == 1 { - return obj, nil - } - - subMap, ok := obj.(map[string]interface{}) - if !ok { - return "", errors.Errorf("value at key %v is not a map[string]interface{}", key) - } - - return GetValue(subMap, strings.Join(pathParts[1:], ".")) -} - -// GetString returns the string at root[path], where path is a dot separated string. -func GetString(root map[string]interface{}, path string) (string, error) { - obj, err := GetValue(root, path) - if err != nil { - return "", err - } - - str, ok := obj.(string) - if !ok { - return "", errors.Errorf("value at path %v is not a string", path) - } - - return str, nil -} - -// GetMap returns the map at root[path], where path is a dot separated string. -func GetMap(root map[string]interface{}, path string) (map[string]interface{}, error) { - obj, err := GetValue(root, path) - if err != nil { - return nil, err - } - - ret, ok := obj.(map[string]interface{}) - if !ok { - return nil, errors.Errorf("value at path %v is not a map[string]interface{}", path) - } - - return ret, nil -} - -// GetSlice returns the slice at root[path], where path is a dot separated string. -func GetSlice(root map[string]interface{}, path string) ([]interface{}, error) { - obj, err := GetValue(root, path) - if err != nil { - return nil, err - } - - ret, ok := obj.([]interface{}) - if !ok { - return nil, errors.Errorf("value at path %v is not a []interface{}", path) - } - - return ret, nil -} - -// ForEach calls fn on each object in the root[path] array, where path is a dot separated string. -func ForEach(root map[string]interface{}, path string, fn func(obj map[string]interface{}) error) error { - s, err := GetSlice(root, path) - if err != nil { - return err - } - - for i := range s { - obj, ok := s[i].(map[string]interface{}) - if !ok { - return errors.Errorf("unable to convert %s[%d] to an object", path, i) - } - if err := fn(obj); err != nil { - return err - } - } - - return nil -} - -// Exists returns true if root[path] exists, or false otherwise. -func Exists(root map[string]interface{}, path string) bool { - if root == nil { - return false - } - - _, 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) map[string]string { - // If the first map passed in is empty, just use all of the second map's data - if first == nil { - first = map[string]string{} - } - - for k, v := range second { - _, ok := first[k] - if !ok { - first[k] = v - } - } - - return first -} diff --git a/pkg/util/collections/map_utils_test.go b/pkg/util/collections/map_utils_test.go deleted file mode 100644 index e2577a96f..000000000 --- a/pkg/util/collections/map_utils_test.go +++ /dev/null @@ -1,102 +0,0 @@ -/* -Copyright 2017 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 collections - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestGetString(t *testing.T) { - var testCases = []struct { - root map[string]interface{} - path string - expectErr bool - result string - }{ - {map[string]interface{}{"path": "value"}, "path", false, "value"}, - {map[string]interface{}{"path": "value"}, "path2", true, ""}, - {map[string]interface{}{"path1": map[string]interface{}{"path2": "value"}}, "path1.path2", false, "value"}, - {map[string]interface{}{"path1": map[string]interface{}{"path2": "value"}}, "path1.path1", true, ""}, - } - - for _, tc := range testCases { - res, err := GetString(tc.root, tc.path) - - if (err != nil) != tc.expectErr { - t.Error("err") - } - if res != tc.result { - t.Error("res") - } - } -} - -func TestMergeMaps(t *testing.T) { - var testCases = []struct { - name string - source map[string]string - destination map[string]string - expected map[string]string - }{ - { - name: "nil destination should result in source being copied", - destination: nil, - source: map[string]string{ - "k1": "v1", - }, - expected: map[string]string{ - "k1": "v1", - }, - }, - { - name: "keys missing from destination should be copied from source", - destination: map[string]string{ - "k2": "v2", - }, - source: map[string]string{ - "k1": "v1", - }, - expected: map[string]string{ - "k1": "v1", - "k2": "v2", - }, - }, - { - name: "matching key should not have value copied from source", - destination: map[string]string{ - "k1": "v1", - }, - source: map[string]string{ - "k1": "v2", - }, - expected: map[string]string{ - "k1": "v1", - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - - result := MergeMaps(tc.destination, tc.source) - - assert.Equal(t, tc.expected, result) - }) - } -}