From cf2c27141be7efd06a82df4a56b214ed5d26a43b Mon Sep 17 00:00:00 2001
From: Steve Kriss <steve@heptio.com>
Date: Wed, 17 Oct 2018 15:10:42 -0600
Subject: [PATCH] fix bugs in GetBackupVolumeSnapshots and add test

Signed-off-by: Steve Kriss <steve@heptio.com>
---
 pkg/persistence/object_store.go      | 37 ++++++++++++++++++++++-
 pkg/persistence/object_store_test.go | 45 ++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/pkg/persistence/object_store.go b/pkg/persistence/object_store.go
index 050b498f1..885c5e808 100644
--- a/pkg/persistence/object_store.go
+++ b/pkg/persistence/object_store.go
@@ -17,6 +17,7 @@ limitations under the License.
 package persistence
 
 import (
+	"compress/gzip"
 	"encoding/json"
 	"io"
 	"io/ioutil"
@@ -233,17 +234,51 @@ func (s *objectBackupStore) GetBackupMetadata(name string) (*arkv1api.Backup, er
 	return backupObj, nil
 }
 
+func keyExists(objectStore cloudprovider.ObjectStore, bucket, prefix, key string) (bool, error) {
+	keys, err := objectStore.ListObjects(bucket, prefix)
+	if err != nil {
+		return false, err
+	}
+
+	var found bool
+	for _, existing := range keys {
+		if key == existing {
+			found = true
+			break
+		}
+	}
+
+	return found, nil
+}
+
 func (s *objectBackupStore) GetBackupVolumeSnapshots(name string) ([]*volume.Snapshot, error) {
 	key := s.layout.getBackupVolumeSnapshotsKey(name)
 
+	// if the volumesnapshots file doesn't exist, we don't want to return an error, since
+	// a legacy backup or a backup with no snapshots would not have this file, so check for
+	// its existence before attempting to get its contents.
+	ok, err := keyExists(s.objectStore, s.bucket, s.layout.getBackupDir(name), key)
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
+	if !ok {
+		return nil, nil
+	}
+
 	res, err := s.objectStore.GetObject(s.bucket, key)
 	if err != nil {
 		return nil, err
 	}
 	defer res.Close()
 
+	gzr, err := gzip.NewReader(res)
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
+	defer gzr.Close()
+
 	var volumeSnapshots []*volume.Snapshot
-	if err := json.NewDecoder(res).Decode(&volumeSnapshots); err != nil {
+	if err := json.NewDecoder(gzr).Decode(&volumeSnapshots); err != nil {
 		return nil, errors.Wrap(err, "error decoding object data")
 	}
 
diff --git a/pkg/persistence/object_store_test.go b/pkg/persistence/object_store_test.go
index bd32f12fd..b58c2c66c 100644
--- a/pkg/persistence/object_store_test.go
+++ b/pkg/persistence/object_store_test.go
@@ -18,6 +18,8 @@ package persistence
 
 import (
 	"bytes"
+	"compress/gzip"
+	"encoding/json"
 	"errors"
 	"io"
 	"io/ioutil"
@@ -38,6 +40,7 @@ import (
 	cloudprovidermocks "github.com/heptio/ark/pkg/cloudprovider/mocks"
 	"github.com/heptio/ark/pkg/util/encode"
 	arktest "github.com/heptio/ark/pkg/util/test"
+	"github.com/heptio/ark/pkg/volume"
 )
 
 type objectBackupStoreTestHarness struct {
@@ -304,6 +307,48 @@ func TestPutBackup(t *testing.T) {
 	}
 }
 
+func TestGetBackupVolumeSnapshots(t *testing.T) {
+	harness := newObjectBackupStoreTestHarness("test-bucket", "")
+
+	// volumesnapshots file not found should not error
+	harness.objectStore.PutObject(harness.bucket, "backups/test-backup/ark-backup.json", newStringReadSeeker("foo"))
+	res, err := harness.GetBackupVolumeSnapshots("test-backup")
+	assert.NoError(t, err)
+	assert.Nil(t, res)
+
+	// volumesnapshots file containing invalid data should error
+	harness.objectStore.PutObject(harness.bucket, "backups/test-backup/test-backup-volumesnapshots.json.gz", newStringReadSeeker("foo"))
+	res, err = harness.GetBackupVolumeSnapshots("test-backup")
+	assert.NotNil(t, err)
+
+	// volumesnapshots file containing gzipped json data should return correctly
+	snapshots := []*volume.Snapshot{
+		{
+			Spec: volume.SnapshotSpec{
+				BackupName:           "test-backup",
+				PersistentVolumeName: "pv-1",
+			},
+		},
+		{
+			Spec: volume.SnapshotSpec{
+				BackupName:           "test-backup",
+				PersistentVolumeName: "pv-2",
+			},
+		},
+	}
+
+	obj := new(bytes.Buffer)
+	gzw := gzip.NewWriter(obj)
+
+	require.NoError(t, json.NewEncoder(gzw).Encode(snapshots))
+	require.NoError(t, gzw.Close())
+	require.NoError(t, harness.objectStore.PutObject(harness.bucket, "backups/test-backup/test-backup-volumesnapshots.json.gz", obj))
+
+	res, err = harness.GetBackupVolumeSnapshots("test-backup")
+	assert.NoError(t, err)
+	assert.EqualValues(t, snapshots, res)
+}
+
 func TestGetBackupContents(t *testing.T) {
 	harness := newObjectBackupStoreTestHarness("test-bucket", "")