Bug Fix: azurerm_storage_table - Migrate id to new format

azurerm/resource_arm_storage_table.go

@@ -23,8 +23,20 @@ func resourceArmStorageTable() *schema.Resource {
 		Importer: &schema.ResourceImporter{
 			State: schema.ImportStatePassthrough,
 		},
-		SchemaVersion: 1,
-		MigrateState:  resourceStorageTableMigrateState,
+		SchemaVersion: 2,
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				// this should have been applied from pre-0.12 migration system; backporting just in-case
+				Type:    resourceStorageTableStateResourceV0V1().CoreConfigSchema().ImpliedType(),
+				Upgrade: resourceStorageTableStateUpgradeV0ToV1,
+				Version: 0,
+			},
+			{
+				Type:    resourceStorageTableStateResourceV0V1().CoreConfigSchema().ImpliedType(),
+				Upgrade: resourceStorageTableStateUpgradeV1ToV2,
+				Version: 1,
+			},
+		},
 
 		Schema: map[string]*schema.Schema{
 			"name": {

azurerm/resource_arm_storage_table_migration.go

@@ -4,37 +4,91 @@ import (
 	"fmt"
 	"log"
 
-	"github.com/hashicorp/terraform/terraform"
+	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/hashicorp/terraform/helper/validation"
+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate"
 )
 
-func resourceStorageTableMigrateState(
-	v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
-	switch v {
-	case 0:
-		log.Println("[INFO] Found AzureRM Storage Table State v0; migrating to v1")
-		return migrateStorageTableStateV0toV1(is, meta)
-	default:
-		return is, fmt.Errorf("Unexpected schema version: %d", v)
+// the schema schema was used for both V0 and V1
+func resourceStorageTableStateResourceV0V1() *schema.Resource {
+	return &schema.Resource{
+		Schema: map[string]*schema.Schema{
+			"name": {
+				Type:         schema.TypeString,
+				Required:     true,
+				ForceNew:     true,
+				ValidateFunc: validateArmStorageTableName,
+			},
+			"storage_account_name": {
+				Type:         schema.TypeString,
+				Required:     true,
+				ForceNew:     true,
+				ValidateFunc: validateArmStorageAccountName,
+			},
+			"resource_group_name": azure.SchemaResourceGroupNameDeprecated(),
+			"acl": {
+				Type:     schema.TypeSet,
+				Optional: true,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"id": {
+							Type:         schema.TypeString,
+							Required:     true,
+							ValidateFunc: validation.StringLenBetween(1, 64),
+						},
+						"access_policy": {
+							Type:     schema.TypeList,
+							Optional: true,
+							Elem: &schema.Resource{
+								Schema: map[string]*schema.Schema{
+									"start": {
+										Type:         schema.TypeString,
+										Required:     true,
+										ValidateFunc: validate.NoEmptyStrings,
+									},
+									"expiry": {
+										Type:         schema.TypeString,
+										Required:     true,
+										ValidateFunc: validate.NoEmptyStrings,
+									},
+									"permissions": {
+										Type:         schema.TypeString,
+										Required:     true,
+										ValidateFunc: validate.NoEmptyStrings,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 }
 
-func migrateStorageTableStateV0toV1(is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
-	if is.Empty() {
-		log.Println("[DEBUG] Empty InstanceState; nothing to migrate.")
-		return is, nil
-	}
+func resourceStorageTableStateUpgradeV0ToV1(rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
+	tableName := rawState["name"].(string)
+	accountName := rawState["storage_account_name"].(string)
+	environment := meta.(*ArmClient).environment
 
-	log.Printf("[DEBUG] ARM Storage Table Attributes before Migration: %#v", is.Attributes)
+	id := rawState["id"].(string)
+	newResourceID := fmt.Sprintf("https://%s.table.%s/%s", accountName, environment.StorageEndpointSuffix, tableName)
+	log.Printf("[DEBUG] Updating ID from %q to %q", id, newResourceID)
 
-	environment := meta.(*ArmClient).environment
+	rawState["id"] = newResourceID
+	return rawState, nil
+}
 
-	tableName := is.Attributes["name"]
-	storageAccountName := is.Attributes["storage_account_name"]
-	newID := fmt.Sprintf("https://%s.table.%s/%s", storageAccountName, environment.StorageEndpointSuffix, tableName)
-	is.Attributes["id"] = newID
-	is.ID = newID
+func resourceStorageTableStateUpgradeV1ToV2(rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
+	tableName := rawState["name"].(string)
+	accountName := rawState["storage_account_name"].(string)
+	environment := meta.(*ArmClient).environment
 
-	log.Printf("[DEBUG] ARM Storage Table Attributes after State Migration: %#v", is.Attributes)
+	id := rawState["id"].(string)
+	newResourceID := fmt.Sprintf("https://%s.table.%s/Tables('%s')", accountName, environment.StorageEndpointSuffix, tableName)
+	log.Printf("[DEBUG] Updating ID from %q to %q", id, newResourceID)
 
-	return is, nil
+	rawState["id"] = newResourceID
+	return rawState, nil
 }

azurerm/resource_arm_storage_table_migration_test.go

@@ -2,65 +2,88 @@ package azurerm
 
 import (
 	"fmt"
+	"reflect"
 	"testing"
 
-	"github.com/hashicorp/terraform/terraform"
+	"github.com/Azure/go-autorest/autorest/azure"
 )
 
-// NOTE: this is intentionally an acceptance test (and we're not explicitly setting the env)
-// as we want to run this depending on the cloud we're in.
-func TestAccAzureRMStorageTableMigrateState(t *testing.T) {
-	config := testGetAzureConfig(t)
-	if config == nil {
-		t.SkipNow()
-		return
+func TestAzureRMStorageTableMigrateStateV0ToV1(t *testing.T) {
+	clouds := []azure.Environment{
+		azure.ChinaCloud,
+		azure.GermanCloud,
+		azure.PublicCloud,
+		azure.USGovernmentCloud,
 	}
 
-	client, err := getArmClient(config, false, "")
-	if err != nil {
-		t.Fatal(fmt.Errorf("Error building ARM Client: %+v", err))
-		return
+	for _, cloud := range clouds {
+		t.Logf("[DEBUG] Testing with Cloud %q", cloud.Name)
+
+		input := map[string]interface{}{
+			"id":                   "table1",
+			"name":                 "table1",
+			"storage_account_name": "account1",
+		}
+		meta := &ArmClient{
+			environment: cloud,
+		}
+		suffix := meta.environment.StorageEndpointSuffix
+
+		expected := map[string]interface{}{
+			"id":                   fmt.Sprintf("https://account1.table.%s/table1", suffix),
+			"name":                 "table1",
+			"storage_account_name": "account1",
+		}
+
+		actual, err := resourceStorageTableStateUpgradeV0ToV1(input, meta)
+		if err != nil {
+			t.Fatalf("Expected no error but got: %s", err)
+		}
+
+		if !reflect.DeepEqual(expected, actual) {
+			t.Fatalf("Expected %+v. Got %+v. But expected them to be the same", expected, actual)
+		}
+
+		t.Logf("[DEBUG] Ok!")
 	}
+}
 
-	client.StopContext = testAccProvider.StopContext()
-
-	suffix := client.environment.StorageEndpointSuffix
-
-	cases := map[string]struct {
-		StateVersion       int
-		ID                 string
-		InputAttributes    map[string]string
-		ExpectedAttributes map[string]string
-	}{
-		"v0_1_without_value": {
-			StateVersion: 0,
-			ID:           "some_id",
-			InputAttributes: map[string]string{
-				"name":                 "table1",
-				"storage_account_name": "example",
-			},
-			ExpectedAttributes: map[string]string{
-				"id": fmt.Sprintf("https://example.table.%s/table1", suffix),
-			},
-		},
+func TestAzureRMStorageTableMigrateStateV1ToV2(t *testing.T) {
+	clouds := []azure.Environment{
+		azure.ChinaCloud,
+		azure.GermanCloud,
+		azure.PublicCloud,
+		azure.USGovernmentCloud,
 	}
 
-	for tn, tc := range cases {
-		is := &terraform.InstanceState{
-			ID:         tc.ID,
-			Attributes: tc.InputAttributes,
+	for _, cloud := range clouds {
+		t.Logf("[DEBUG] Testing with Cloud %q", cloud.Name)
+
+		meta := &ArmClient{
+			environment: cloud,
 		}
-		is, err := resourceStorageTableMigrateState(tc.StateVersion, is, client)
+		suffix := meta.environment.StorageEndpointSuffix
 
+		input := map[string]interface{}{
+			"id":                   fmt.Sprintf("https://account1.table.%s/table1", suffix),
+			"name":                 "table1",
+			"storage_account_name": "account1",
+		}
+		expected := map[string]interface{}{
+			"id":                   fmt.Sprintf("https://account1.table.%s/Tables('table1')", suffix),
+			"name":                 "table1",
+			"storage_account_name": "account1",
+		}
+
+		actual, err := resourceStorageTableStateUpgradeV1ToV2(input, meta)
 		if err != nil {
-			t.Fatalf("bad: %s, err: %#v", tn, err)
+			t.Fatalf("Expected no error but got: %s", err)
 		}
 
-		for k, v := range tc.ExpectedAttributes {
-			actual := is.Attributes[k]
-			if actual != v {
-				t.Fatalf("Bad Storage Table Migrate for %q: %q\n\n expected: %q", k, actual, v)
-			}
+		if !reflect.DeepEqual(expected, actual) {
+			t.Fatalf("Expected %+v. Got %+v. But expected them to be the same", expected, actual)
 		}
+
+		t.Logf("[DEBUG] Ok!")
 	}
 }