From c8c03a38e93ce62129266f56a3e28547525951dc Mon Sep 17 00:00:00 2001
From: Steve Kriss <krisss@vmware.com>
Date: Wed, 27 Mar 2019 17:01:34 -0600
Subject: [PATCH] remove support for legacy Azure snapshot ID format

Signed-off-by: Steve Kriss <krisss@vmware.com>
---
 pkg/cloudprovider/azure/volume_snapshotter.go | 32 ++-----------------
 .../azure/volume_snapshotter_test.go          | 24 ++------------
 2 files changed, 5 insertions(+), 51 deletions(-)

diff --git a/pkg/cloudprovider/azure/volume_snapshotter.go b/pkg/cloudprovider/azure/volume_snapshotter.go
index a66662714..18c3f229c 100644
--- a/pkg/cloudprovider/azure/volume_snapshotter.go
+++ b/pkg/cloudprovider/azure/volume_snapshotter.go
@@ -129,7 +129,7 @@ func (b *VolumeSnapshotter) Init(config map[string]string) error {
 }
 
 func (b *VolumeSnapshotter) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (string, error) {
-	snapshotIdentifier, err := b.parseSnapshotName(snapshotID)
+	snapshotIdentifier, err := parseFullSnapshotName(snapshotID)
 	if err != nil {
 		return "", err
 	}
@@ -266,7 +266,7 @@ func stringPtr(s string) *string {
 }
 
 func (b *VolumeSnapshotter) DeleteSnapshot(snapshotID string) error {
-	snapshotInfo, err := b.parseSnapshotName(snapshotID)
+	snapshotInfo, err := parseFullSnapshotName(snapshotID)
 	if err != nil {
 		return err
 	}
@@ -306,34 +306,6 @@ func getComputeResourceName(subscription, resourceGroup, resource, name string)
 var snapshotURIRegexp = regexp.MustCompile(
 	`^\/subscriptions\/(?P<subscription>.*)\/resourceGroups\/(?P<resourceGroup>.*)\/providers\/Microsoft.Compute\/snapshots\/(?P<snapshotName>.*)$`)
 
-// parseSnapshotName takes a snapshot name, either fully-qualified or not, and returns
-// a snapshot identifier or an error if the name is not in a valid format. If the name
-// is not fully-qualified, the subscription and resource group are assumed to be the
-// ones that the volume snapshotter is configured with.
-//
-// TODO(1.0) remove this function and replace usage with `parseFullSnapshotName` since
-// we won't support the legacy snapshot name format for 1.0.
-func (b *VolumeSnapshotter) parseSnapshotName(name string) (*snapshotIdentifier, error) {
-	switch {
-	// legacy format - name only (not fully-qualified)
-	case !strings.Contains(name, "/"):
-		return &snapshotIdentifier{
-			subscription: b.subscription,
-			// use the disksResourceGroup here because Velero only
-			// supported storing snapshots in that resource group
-			// when the legacy snapshot format was used.
-			resourceGroup: b.disksResourceGroup,
-			name:          name,
-		}, nil
-	// current format - fully qualified
-	case snapshotURIRegexp.MatchString(name):
-		return parseFullSnapshotName(name)
-	// unrecognized format
-	default:
-		return nil, errors.New("snapshot name is not in a valid format")
-	}
-}
-
 // parseFullSnapshotName takes a fully-qualified snapshot name and returns
 // a snapshot identifier or an error if the snapshot name does not match the
 // regexp.
diff --git a/pkg/cloudprovider/azure/volume_snapshotter_test.go b/pkg/cloudprovider/azure/volume_snapshotter_test.go
index 6c57bcf8c..1df1ba7d8 100644
--- a/pkg/cloudprovider/azure/volume_snapshotter_test.go
+++ b/pkg/cloudprovider/azure/volume_snapshotter_test.go
@@ -94,38 +94,20 @@ func TestSetVolumeID(t *testing.T) {
 	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
-// the `parseFullSnapshotName` function, and remove case for legacy
-// format
-func TestParseSnapshotName(t *testing.T) {
-	b := &VolumeSnapshotter{
-		subscription:       "default-sub",
-		disksResourceGroup: "default-rg-legacy",
-	}
-
+func TestParseFullSnapshotName(t *testing.T) {
 	// invalid name
 	fullName := "foo/bar"
-	_, err := b.parseSnapshotName(fullName)
+	_, err := parseFullSnapshotName(fullName)
 	assert.Error(t, err)
 
 	// valid name (current format)
 	fullName = "/subscriptions/sub-1/resourceGroups/rg-1/providers/Microsoft.Compute/snapshots/snap-1"
-	snap, err := b.parseSnapshotName(fullName)
+	snap, err := parseFullSnapshotName(fullName)
 	require.NoError(t, err)
 
 	assert.Equal(t, "sub-1", snap.subscription)
 	assert.Equal(t, "rg-1", snap.resourceGroup)
 	assert.Equal(t, "snap-1", snap.name)
-
-	// valid name (legacy format)
-	// TODO(1.0) remove this test case
-	fullName = "foobar"
-	snap, err = b.parseSnapshotName(fullName)
-	require.NoError(t, err)
-	assert.Equal(t, b.subscription, snap.subscription)
-	assert.Equal(t, b.disksResourceGroup, snap.resourceGroup)
-	assert.Equal(t, fullName, snap.name)
-
 }
 
 func TestGetComputeResourceName(t *testing.T) {