resource/aws_dynamodb_table: Update and validate attributes

hashicorp · Jan 31, 2018 · 823cad4 · 823cad4
1 parent dd80639
commit 823cad4
Show file tree

Hide file tree

Showing 3 changed files with 190 additions and 7 deletions.
diff --git a/aws/resource_aws_dynamodb_table.go b/aws/resource_aws_dynamodb_table.go
@@ -31,7 +31,11 @@ func resourceAwsDynamoDbTable() *schema.Resource {
 		},
 
 		CustomizeDiff: func(diff *schema.ResourceDiff, v interface{}) error {
-			return validateDynamoDbStreamSpec(diff)
+			err := validateDynamoDbStreamSpec(diff)
+			if err != nil {
+				return err
+			}
+			return validateDynamoDbTableAttributes(diff)
 		},
 
 		SchemaVersion: 1,
@@ -322,11 +326,10 @@ func resourceAwsDynamoDbTableUpdate(d *schema.ResourceData, meta interface{}) er
 		}
 	}
 
-	if d.HasChange("global_secondary_index") && !d.IsNewResource() {
-		var attributes []*dynamodb.AttributeDefinition
-		if v, ok := d.GetOk("attribute"); ok {
-			attributes = expandDynamoDbAttributes(v.(*schema.Set).List())
-		}
+	// Indexes and attributes are tightly coupled (DynamoDB requires attribute definitions
+	// for all indexed attributes) so it's necessary to update these together.
+	if (d.HasChange("global_secondary_index") || d.HasChange("attribute")) && !d.IsNewResource() {
+		attributes := d.Get("attribute").(*schema.Set).List()
 
 		o, n := d.GetChange("global_secondary_index")
 		ops, err := diffDynamoDbGSI(o.(*schema.Set).List(), n.(*schema.Set).List())
@@ -337,12 +340,13 @@ func resourceAwsDynamoDbTableUpdate(d *schema.ResourceData, meta interface{}) er
 
 		input := &dynamodb.UpdateTableInput{
 			TableName:            aws.String(d.Id()),
-			AttributeDefinitions: attributes,
+			AttributeDefinitions: expandDynamoDbAttributes(attributes),
 		}
 
 		// Only 1 online index can be created or deleted simultaneously per table
 		for _, op := range ops {
 			input.GlobalSecondaryIndexUpdates = []*dynamodb.GlobalSecondaryIndexUpdate{op}
+			log.Printf("[DEBUG] Updating DynamoDB Table: %s", input)
 			_, err := conn.UpdateTable(input)
 			if err != nil {
 				return err
@@ -367,6 +371,14 @@ func resourceAwsDynamoDbTableUpdate(d *schema.ResourceData, meta interface{}) er
 			}
 		}
 
+		// We may only be changing the attribute type
+		if len(ops) == 0 {
+			_, err := conn.UpdateTable(input)
+			if err != nil {
+				return err
+			}
+		}
+
 		if err := waitForDynamoDbTableToBeActive(d.Id(), d.Timeout(schema.TimeoutUpdate), conn); err != nil {
 			return fmt.Errorf("Error waiting for DynamoDB Table op: %s", err)
 		}

diff --git a/aws/resource_aws_dynamodb_table_test.go b/aws/resource_aws_dynamodb_table_test.go
@@ -583,6 +583,65 @@ func TestAccAWSDynamoDbTable_ttl(t *testing.T) {
 		},
 	})
 }
+
+func TestAccAWSDynamoDbTable_attributeUpdate(t *testing.T) {
+	var conf dynamodb.DescribeTableOutput
+
+	rName := acctest.RandomWithPrefix("TerraformTestTable-")
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSDynamoDbTableDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSDynamoDbConfigOneAttribute(rName, "firstKey", "firstKey", "S"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
+				),
+			},
+			{ // Attribute type change
+				Config: testAccAWSDynamoDbConfigOneAttribute(rName, "firstKey", "firstKey", "N"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
+				),
+			},
+			{ // New attribute addition (index update)
+				Config: testAccAWSDynamoDbConfigTwoAttributes(rName, "firstKey", "secondKey", "firstKey", "N", "secondKey", "S"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
+				),
+			},
+			{ // Attribute removal (index update)
+				Config: testAccAWSDynamoDbConfigOneAttribute(rName, "firstKey", "firstKey", "S"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
+				),
+			},
+		},
+	})
+}
+
+func TestAccAWSDynamoDbTable_attributeUpdateValidation(t *testing.T) {
+	rName := acctest.RandomWithPrefix("TerraformTestTable-")
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSDynamoDbTableDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccAWSDynamoDbConfigOneAttribute(rName, "firstKey", "unusedKey", "S"),
+				ExpectError: regexp.MustCompile(`All attributes must be indexed. Unused attributes: \["unusedKey"\]$`),
+			},
+			{
+				Config:      testAccAWSDynamoDbConfigTwoAttributes(rName, "firstKey", "secondKey", "firstUnused", "N", "secondUnused", "S"),
+				ExpectError: regexp.MustCompile(`All attributes must be indexed. Unused attributes: \["firstUnused"\ \"secondUnused\"]$`),
+			},
+		},
+	})
+}
+
 func testAccCheckDynamoDbTableTimeToLiveWasUpdated(n string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		log.Printf("[DEBUG] Trying to create initial table state!")
@@ -1347,3 +1406,65 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {
 }
 `, rName)
 }
+
+func testAccAWSDynamoDbConfigOneAttribute(rName, hashKey, attrName, attrType string) string {
+	return fmt.Sprintf(`
+resource "aws_dynamodb_table" "basic-dynamodb-table" {
+  name = "%s"
+  read_capacity = 10
+  write_capacity = 10
+  hash_key = "staticHashKey"
+
+  attribute {
+    name = "staticHashKey"
+    type = "S"
+  }
+  attribute {
+    name = "%s"
+    type = "%s"
+  }
+
+  global_secondary_index {
+    name = "gsiName"
+    hash_key = "%s"
+    write_capacity = 10
+    read_capacity = 10
+    projection_type = "KEYS_ONLY"
+  }
+}
+`, rName, attrName, attrType, hashKey)
+}
+
+func testAccAWSDynamoDbConfigTwoAttributes(rName, hashKey, rangeKey, attrName1, attrType1, attrName2, attrType2 string) string {
+	return fmt.Sprintf(`
+resource "aws_dynamodb_table" "basic-dynamodb-table" {
+  name = "%s"
+  read_capacity = 10
+  write_capacity = 10
+  hash_key = "staticHashKey"
+
+  attribute {
+    name = "staticHashKey"
+    type = "S"
+  }
+
+  attribute {
+    name = "%s"
+    type = "%s"
+  }
+  attribute {
+    name = "%s"
+    type = "%s"
+  }
+
+  global_secondary_index {
+    name = "gsiName"
+    hash_key = "%s"
+    range_key = "%s"
+    write_capacity = 10
+    read_capacity = 10
+    projection_type = "KEYS_ONLY"
+  }
+}
+`, rName, attrName1, attrType1, attrName2, attrType2, hashKey, rangeKey)
+}
diff --git a/aws/validators.go b/aws/validators.go
@@ -2213,3 +2213,53 @@ func validateDynamoDbStreamSpec(d *schema.ResourceDiff) error {
 	}
 	return nil
 }
+
+func validateDynamoDbTableAttributes(d *schema.ResourceDiff) error {
+	// Collect all indexed attributes
+	primaryHashKey := d.Get("hash_key").(string)
+	indexedAttributes := map[string]bool{
+		primaryHashKey: true,
+	}
+	if v, ok := d.GetOk("range_key"); ok {
+		indexedAttributes[v.(string)] = true
+	}
+	if v, ok := d.GetOk("local_secondary_index"); ok {
+		indexes := v.(*schema.Set).List()
+		for _, idx := range indexes {
+			index := idx.(map[string]interface{})
+			rangeKey := index["range_key"].(string)
+			indexedAttributes[rangeKey] = true
+		}
+	}
+	if v, ok := d.GetOk("global_secondary_index"); ok {
+		indexes := v.(*schema.Set).List()
+		for _, idx := range indexes {
+			index := idx.(map[string]interface{})
+
+			hashKey := index["hash_key"].(string)
+			indexedAttributes[hashKey] = true
+
+			if rk, ok := index["range_key"]; ok {
+				indexedAttributes[rk.(string)] = true
+			}
+		}
+	}
+
+	// Check if all indexed attributes have an attribute definition
+	attributes := d.Get("attribute").(*schema.Set).List()
+	missingAttrDefs := []string{}
+	for _, attr := range attributes {
+		attribute := attr.(map[string]interface{})
+		attrName := attribute["name"].(string)
+
+		if _, ok := indexedAttributes[attrName]; !ok {
+			missingAttrDefs = append(missingAttrDefs, attrName)
+		}
+	}
+
+	if len(missingAttrDefs) > 0 {
+		return fmt.Errorf("All attributes must be indexed. Unused attributes: %q", missingAttrDefs)
+	}
+
+	return nil
+}