diff --git a/aws/resource_aws_dynamodb_table.go b/aws/resource_aws_dynamodb_table.go index 50cf27a08c1..7610f3f81f5 100644 --- 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, @@ -326,11 +330,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()) @@ -341,12 +344,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 @@ -371,6 +375,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 index 445be50893b..1528512443f 100644 --- a/aws/resource_aws_dynamodb_table_test.go +++ b/aws/resource_aws_dynamodb_table_test.go @@ -585,6 +585,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!") @@ -1349,3 +1408,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 index cd6efb3f691..d72c4ddf995 100644 --- a/aws/validators.go +++ b/aws/validators.go @@ -2154,3 +2154,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 +}