provider/aws: Fix DynamoDB issues about GSIs indexes (#13256)

* provider/aws: Fixed DynamoDB GSI update when using multiple indexes

* provider/aws: Fixed DynamoDB GSI set hash function

* Added DynamoDB table state migration

builtin/providers/aws/resource_aws_dynamodb_table.go

@@ -39,6 +39,9 @@ func resourceAwsDynamoDbTable() *schema.Resource {
 			State: schema.ImportStatePassthrough,
 		},
 
+		SchemaVersion: 1,
+		MigrateState:  resourceAwsDynamoDbTableMigrateState,
+
 		Schema: map[string]*schema.Schema{
 			"arn": {
 				Type:     schema.TypeString,
@@ -157,15 +160,6 @@ func resourceAwsDynamoDbTable() *schema.Resource {
 						},
 					},
 				},
-				// GSI names are the uniqueness constraint
-				Set: func(v interface{}) int {
-					var buf bytes.Buffer
-					m := v.(map[string]interface{})
-					buf.WriteString(fmt.Sprintf("%s-", m["name"].(string)))
-					buf.WriteString(fmt.Sprintf("%d-", m["write_capacity"].(int)))
-					buf.WriteString(fmt.Sprintf("%d-", m["read_capacity"].(int)))
-					return hashcode.String(buf.String())
-				},
 			},
 			"stream_enabled": {
 				Type:     schema.TypeBool,
@@ -533,9 +527,8 @@ func resourceAwsDynamoDbTableUpdate(d *schema.ResourceData, meta interface{}) er
 
 			table := tableDescription.Table
 
-			updates := []*dynamodb.GlobalSecondaryIndexUpdate{}
-
 			for _, updatedgsidata := range gsiSet.List() {
+				updates := []*dynamodb.GlobalSecondaryIndexUpdate{}
 				gsidata := updatedgsidata.(map[string]interface{})
 				gsiName := gsidata["name"].(string)
 				gsiWriteCapacity := gsidata["write_capacity"].(int)
@@ -584,6 +577,10 @@ func resourceAwsDynamoDbTableUpdate(d *schema.ResourceData, meta interface{}) er
 						log.Printf("[DEBUG] Error updating table: %s", err)
 						return err
 					}
+
+					if err := waitForGSIToBeActive(d.Id(), gsiName, meta); err != nil {
+						return errwrap.Wrapf("Error waiting for Dynamo DB GSI to be active: {{err}}", err)
+					}
 				}
 			}
 		}

builtin/providers/aws/resource_aws_dynamodb_table_migrate.go

@@ -0,0 +1,70 @@
+package aws
+
+import (
+	"fmt"
+	"log"
+
+	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/hashicorp/terraform/terraform"
+	"strings"
+)
+
+func resourceAwsDynamoDbTableMigrateState(
+	v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
+	switch v {
+	case 0:
+		log.Println("[INFO] Found AWS DynamoDB Table State v0; migrating to v1")
+		return migrateDynamoDBStateV0toV1(is)
+	default:
+		return is, fmt.Errorf("Unexpected schema version: %d", v)
+	}
+}
+
+func migrateDynamoDBStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) {
+	if is.Empty() {
+		log.Println("[DEBUG] Empty InstanceState; nothing to migrate.")
+		return is, nil
+	}
+
+	log.Printf("[DEBUG] DynamoDB Table Attributes before Migration: %#v", is.Attributes)
+
+	prefix := "global_secondary_index"
+	entity := resourceAwsDynamoDbTable()
+
+	// Read old keys
+	reader := &schema.MapFieldReader{
+		Schema: entity.Schema,
+		Map:    schema.BasicMapReader(is.Attributes),
+	}
+	result, err := reader.ReadField([]string{prefix})
+	if err != nil {
+		return nil, err
+	}
+
+	oldKeys, ok := result.Value.(*schema.Set)
+	if !ok {
+		return nil, fmt.Errorf("Got unexpected value from state: %#v", result.Value)
+	}
+
+	// Delete old keys
+	for k := range is.Attributes {
+		if strings.HasPrefix(k, fmt.Sprintf("%s.", prefix)) {
+			delete(is.Attributes, k)
+		}
+	}
+
+	// Write new keys
+	writer := schema.MapFieldWriter{
+		Schema: entity.Schema,
+	}
+	if err := writer.WriteField([]string{prefix}, oldKeys); err != nil {
+		return is, err
+	}
+	for k, v := range writer.Map() {
+		is.Attributes[k] = v
+	}
+
+	log.Printf("[DEBUG] DynamoDB Table Attributes after State Migration: %#v", is.Attributes)
+
+	return is, nil
+}

builtin/providers/aws/resource_aws_dynamodb_table_test.go

@@ -14,6 +14,8 @@ import (
 )
 
 func TestAccAWSDynamoDbTable_basic(t *testing.T) {
+	var conf dynamodb.DescribeTableOutput
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
@@ -22,7 +24,8 @@ func TestAccAWSDynamoDbTable_basic(t *testing.T) {
 			{
 				Config: testAccAWSDynamoDbConfigInitialState(),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table"),
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
+					testAccCheckInitialAWSDynamoDbTableConf("aws_dynamodb_table.basic-dynamodb-table"),
 				),
 			},
 			{
@@ -36,6 +39,8 @@ func TestAccAWSDynamoDbTable_basic(t *testing.T) {
 }
 
 func TestAccAWSDynamoDbTable_streamSpecification(t *testing.T) {
+	var conf dynamodb.DescribeTableOutput
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
@@ -44,7 +49,8 @@ func TestAccAWSDynamoDbTable_streamSpecification(t *testing.T) {
 			{
 				Config: testAccAWSDynamoDbConfigStreamSpecification(),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table"),
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
+					testAccCheckInitialAWSDynamoDbTableConf("aws_dynamodb_table.basic-dynamodb-table"),
 					resource.TestCheckResourceAttr(
 						"aws_dynamodb_table.basic-dynamodb-table", "stream_enabled", "true"),
 					resource.TestCheckResourceAttr(
@@ -56,6 +62,8 @@ func TestAccAWSDynamoDbTable_streamSpecification(t *testing.T) {
 }
 
 func TestAccAWSDynamoDbTable_tags(t *testing.T) {
+	var conf dynamodb.DescribeTableOutput
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
@@ -64,7 +72,8 @@ func TestAccAWSDynamoDbTable_tags(t *testing.T) {
 			{
 				Config: testAccAWSDynamoDbConfigTags(),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table"),
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
+					testAccCheckInitialAWSDynamoDbTableConf("aws_dynamodb_table.basic-dynamodb-table"),
 					resource.TestCheckResourceAttr(
 						"aws_dynamodb_table.basic-dynamodb-table", "tags.%", "3"),
 				),
@@ -73,6 +82,32 @@ func TestAccAWSDynamoDbTable_tags(t *testing.T) {
 	})
 }
 
+// https://github.com/hashicorp/terraform/issues/13243
+func TestAccAWSDynamoDbTable_gsiUpdate(t *testing.T) {
+	var conf dynamodb.DescribeTableOutput
+	name := acctest.RandString(10)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSDynamoDbTableDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSDynamoDbConfigGsiUpdate(name),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.test", &conf),
+				),
+			},
+			{
+				Config: testAccAWSDynamoDbConfigGsiUpdated(name),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.test", &conf),
+				),
+			},
+		},
+	})
+}
+
 func TestResourceAWSDynamoDbTableStreamViewType_validation(t *testing.T) {
 	cases := []struct {
 		Value    string
@@ -143,7 +178,37 @@ func testAccCheckAWSDynamoDbTableDestroy(s *terraform.State) error {
 	return nil
 }
 
-func testAccCheckInitialAWSDynamoDbTableExists(n string) resource.TestCheckFunc {
+func testAccCheckInitialAWSDynamoDbTableExists(n string, table *dynamodb.DescribeTableOutput) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		log.Printf("[DEBUG] Trying to create initial table state!")
+		rs, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("Not found: %s", n)
+		}
+
+		if rs.Primary.ID == "" {
+			return fmt.Errorf("No DynamoDB table name specified!")
+		}
+
+		conn := testAccProvider.Meta().(*AWSClient).dynamodbconn
+
+		params := &dynamodb.DescribeTableInput{
+			TableName: aws.String(rs.Primary.ID),
+		}
+
+		resp, err := conn.DescribeTable(params)
+
+		if err != nil {
+			return fmt.Errorf("[ERROR] Problem describing table '%s': %s", rs.Primary.ID, err)
+		}
+
+		*table = *resp
+
+		return nil
+	}
+}
+
+func testAccCheckInitialAWSDynamoDbTableConf(n string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		log.Printf("[DEBUG] Trying to create initial table state!")
 		rs, ok := s.RootModule().Resources[n]
@@ -306,27 +371,33 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {
   write_capacity = 20
   hash_key = "TestTableHashKey"
   range_key = "TestTableRangeKey"
+
   attribute {
     name = "TestTableHashKey"
     type = "S"
   }
+
   attribute {
     name = "TestTableRangeKey"
     type = "S"
   }
+
   attribute {
     name = "TestLSIRangeKey"
     type = "N"
   }
+
   attribute {
     name = "TestGSIRangeKey"
     type = "S"
   }
+
   local_secondary_index {
     name = "TestTableLSI"
     range_key = "TestLSIRangeKey"
     projection_type = "ALL"
   }
+
   global_secondary_index {
     name = "InitialTestTableGSI"
     hash_key = "TestTableHashKey"
@@ -346,27 +417,33 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {
   write_capacity = 20
   hash_key = "TestTableHashKey"
   range_key = "TestTableRangeKey"
+
   attribute {
     name = "TestTableHashKey"
     type = "S"
   }
+
   attribute {
     name = "TestTableRangeKey"
     type = "S"
   }
+
   attribute {
     name = "TestLSIRangeKey"
     type = "N"
   }
+
   attribute {
     name = "ReplacementGSIRangeKey"
     type = "N"
   }
+
   local_secondary_index {
     name = "TestTableLSI"
     range_key = "TestLSIRangeKey"
     projection_type = "ALL"
   }
+
   global_secondary_index {
     name = "ReplacementTestTableGSI"
     hash_key = "TestTableHashKey"
@@ -387,27 +464,33 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {
   write_capacity = 20
   hash_key = "TestTableHashKey"
   range_key = "TestTableRangeKey"
+
   attribute {
     name = "TestTableHashKey"
     type = "S"
   }
+
   attribute {
     name = "TestTableRangeKey"
     type = "S"
   }
+
   attribute {
     name = "TestLSIRangeKey"
     type = "N"
   }
+
   attribute {
     name = "TestGSIRangeKey"
     type = "S"
   }
+
   local_secondary_index {
     name = "TestTableLSI"
     range_key = "TestLSIRangeKey"
     projection_type = "ALL"
   }
+
   global_secondary_index {
     name = "InitialTestTableGSI"
     hash_key = "TestTableHashKey"
@@ -430,27 +513,33 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {
   write_capacity = 20
   hash_key = "TestTableHashKey"
   range_key = "TestTableRangeKey"
+
   attribute {
     name = "TestTableHashKey"
     type = "S"
   }
+
   attribute {
     name = "TestTableRangeKey"
     type = "S"
   }
+
   attribute {
     name = "TestLSIRangeKey"
     type = "N"
   }
+
   attribute {
     name = "TestGSIRangeKey"
     type = "S"
   }
+
   local_secondary_index {
     name = "TestTableLSI"
     range_key = "TestLSIRangeKey"
     projection_type = "ALL"
   }
+
   global_secondary_index {
     name = "InitialTestTableGSI"
     hash_key = "TestTableHashKey"
@@ -459,6 +548,7 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {
     read_capacity = 10
     projection_type = "KEYS_ONLY"
   }
+
   tags {
     Name = "terraform-test-table-%d"
     AccTest = "yes"
@@ -467,3 +557,121 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {
 }
 `, acctest.RandInt(), acctest.RandInt())
 }
+
+func testAccAWSDynamoDbConfigGsiUpdate(name string) string {
+	return fmt.Sprintf(`
+variable "capacity" {
+  default = 10
+}
+
+resource "aws_dynamodb_table" "test" {
+  name           = "tf-acc-test-%s"
+  read_capacity  = "${var.capacity}"
+  write_capacity = "${var.capacity}"
+  hash_key       = "id"
+
+  attribute {
+    name = "id"
+    type = "S"
+  }
+
+  attribute {
+    name = "att1"
+    type = "S"
+  }
+
+  attribute {
+    name = "att2"
+    type = "S"
+  }
+
+  attribute {
+    name = "att3"
+    type = "S"
+  }
+
+  global_secondary_index {
+    name            = "att1-index"
+    hash_key        = "att1"
+    write_capacity  = "${var.capacity}"
+    read_capacity   = "${var.capacity}"
+    projection_type = "ALL"
+  }
+
+  global_secondary_index {
+    name            = "att2-index"
+    hash_key        = "att2"
+    write_capacity  = "${var.capacity}"
+    read_capacity   = "${var.capacity}"
+    projection_type = "ALL"
+  }
+
+  global_secondary_index {
+    name            = "att3-index"
+    hash_key        = "att3"
+    write_capacity  = "${var.capacity}"
+    read_capacity   = "${var.capacity}"
+    projection_type = "ALL"
+  }
+}
+`, name)
+}
+
+func testAccAWSDynamoDbConfigGsiUpdated(name string) string {
+	return fmt.Sprintf(`
+variable "capacity" {
+  default = 20
+}
+
+resource "aws_dynamodb_table" "test" {
+  name           = "tf-acc-test-%s"
+  read_capacity  = "${var.capacity}"
+  write_capacity = "${var.capacity}"
+  hash_key       = "id"
+
+  attribute {
+    name = "id"
+    type = "S"
+  }
+
+  attribute {
+    name = "att1"
+    type = "S"
+  }
+
+  attribute {
+    name = "att2"
+    type = "S"
+  }
+
+  attribute {
+    name = "att3"
+    type = "S"
+  }
+
+  global_secondary_index {
+    name            = "att1-index"
+    hash_key        = "att1"
+    write_capacity  = "${var.capacity}"
+    read_capacity   = "${var.capacity}"
+    projection_type = "ALL"
+  }
+
+  global_secondary_index {
+    name            = "att2-index"
+    hash_key        = "att2"
+    write_capacity  = "${var.capacity}"
+    read_capacity   = "${var.capacity}"
+    projection_type = "ALL"
+  }
+
+  global_secondary_index {
+    name            = "att3-index"
+    hash_key        = "att3"
+    write_capacity  = "${var.capacity}"
+    read_capacity   = "${var.capacity}"
+    projection_type = "ALL"
+  }
+}
+`, name)
+}