From c44afc41791a5a7549dfc9834973d106964d785f Mon Sep 17 00:00:00 2001 From: Gauthier Wallet Date: Fri, 7 Apr 2017 17:13:00 +0200 Subject: [PATCH] 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 --- .../aws/resource_aws_dynamodb_table.go | 19 +- .../resource_aws_dynamodb_table_migrate.go | 70 +++ .../aws/resource_aws_dynamodb_table_test.go | 504 +++++++++++++----- 3 files changed, 434 insertions(+), 159 deletions(-) create mode 100644 builtin/providers/aws/resource_aws_dynamodb_table_migrate.go diff --git a/builtin/providers/aws/resource_aws_dynamodb_table.go b/builtin/providers/aws/resource_aws_dynamodb_table.go index e0c63760d5..fff6775c16 100644 --- a/builtin/providers/aws/resource_aws_dynamodb_table.go +++ b/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) + } } } } diff --git a/builtin/providers/aws/resource_aws_dynamodb_table_migrate.go b/builtin/providers/aws/resource_aws_dynamodb_table_migrate.go new file mode 100644 index 0000000000..59865effc8 --- /dev/null +++ b/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 +} diff --git a/builtin/providers/aws/resource_aws_dynamodb_table_test.go b/builtin/providers/aws/resource_aws_dynamodb_table_test.go index 4d5437f170..fba65851cd 100644 --- a/builtin/providers/aws/resource_aws_dynamodb_table_test.go +++ b/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] @@ -301,123 +366,141 @@ func dynamoDbAttributesToMap(attributes *[]*dynamodb.AttributeDefinition) map[st func testAccAWSDynamoDbConfigInitialState() string { return fmt.Sprintf(` resource "aws_dynamodb_table" "basic-dynamodb-table" { - name = "TerraformTestTable-%d" - read_capacity = 10 - 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" - range_key = "TestGSIRangeKey" - write_capacity = 10 - read_capacity = 10 - projection_type = "KEYS_ONLY" - } + name = "TerraformTestTable-%d" + read_capacity = 10 + 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" + range_key = "TestGSIRangeKey" + write_capacity = 10 + read_capacity = 10 + projection_type = "KEYS_ONLY" + } } `, acctest.RandInt()) } const testAccAWSDynamoDbConfigAddSecondaryGSI = ` resource "aws_dynamodb_table" "basic-dynamodb-table" { - name = "TerraformTestTable" - read_capacity = 20 - 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" - range_key = "ReplacementGSIRangeKey" - write_capacity = 5 - read_capacity = 5 - projection_type = "INCLUDE" - non_key_attributes = ["TestNonKeyAttribute"] - } + name = "TerraformTestTable" + read_capacity = 20 + 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" + range_key = "ReplacementGSIRangeKey" + write_capacity = 5 + read_capacity = 5 + projection_type = "INCLUDE" + non_key_attributes = ["TestNonKeyAttribute"] + } } ` func testAccAWSDynamoDbConfigStreamSpecification() string { return fmt.Sprintf(` resource "aws_dynamodb_table" "basic-dynamodb-table" { - name = "TerraformTestStreamTable-%d" - read_capacity = 10 - 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" - range_key = "TestGSIRangeKey" - write_capacity = 10 - read_capacity = 10 - projection_type = "KEYS_ONLY" - } - stream_enabled = true - stream_view_type = "KEYS_ONLY" + name = "TerraformTestStreamTable-%d" + read_capacity = 10 + 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" + range_key = "TestGSIRangeKey" + write_capacity = 10 + read_capacity = 10 + projection_type = "KEYS_ONLY" + } + stream_enabled = true + stream_view_type = "KEYS_ONLY" } `, acctest.RandInt()) } @@ -425,45 +508,170 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" { func testAccAWSDynamoDbConfigTags() string { return fmt.Sprintf(` resource "aws_dynamodb_table" "basic-dynamodb-table" { - name = "TerraformTestTable-%d" - read_capacity = 10 - 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" - range_key = "TestGSIRangeKey" - write_capacity = 10 - read_capacity = 10 - projection_type = "KEYS_ONLY" - } - tags { - Name = "terraform-test-table-%d" - AccTest = "yes" - Testing = "absolutely" - } + name = "TerraformTestTable-%d" + read_capacity = 10 + 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" + range_key = "TestGSIRangeKey" + write_capacity = 10 + read_capacity = 10 + projection_type = "KEYS_ONLY" + } + + tags { + Name = "terraform-test-table-%d" + AccTest = "yes" + Testing = "absolutely" + } } `, 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) +}