From 91e0fed3338bf04753b7dbe6eb25b7c48241bb9c Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Wed, 1 Feb 2017 15:44:30 -0500 Subject: [PATCH] provider/aws: Add tag support to the dynamodb table resource Adds tag support to the `aws_dynamodb_table` resource. Also adds a test for the resource, and a test to ensure that the tags are populated correctly from a resource import. ``` $ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSDynamoDBTable_tags' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/02/01 15:35:00 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSDynamoDBTable_tags -timeout 120m === RUN TestAccAWSDynamoDBTable_tags --- PASS: TestAccAWSDynamoDBTable_tags (28.69s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 28.713s ``` ``` $ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSDynamoDbTable_importTags' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/02/01 15:39:49 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSDynamoDbTable_importTags -timeout 120m === RUN TestAccAWSDynamoDbTable_importTags --- PASS: TestAccAWSDynamoDbTable_importTags (30.62s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 30.645s ``` --- .../aws/import_aws_dynamodb_table_test.go | 25 +++- .../aws/resource_aws_dynamodb_table.go | 130 +++++++++++++----- .../aws/resource_aws_dynamodb_table_test.go | 77 ++++++++++- builtin/providers/aws/tags.go | 101 ++++++++++++++ .../aws/r/dynamodb_table.html.markdown | 5 + 5 files changed, 295 insertions(+), 43 deletions(-) diff --git a/builtin/providers/aws/import_aws_dynamodb_table_test.go b/builtin/providers/aws/import_aws_dynamodb_table_test.go index 49da4b0eb..dc5e2feab 100644 --- a/builtin/providers/aws/import_aws_dynamodb_table_test.go +++ b/builtin/providers/aws/import_aws_dynamodb_table_test.go @@ -14,11 +14,32 @@ func TestAccAWSDynamoDbTable_importBasic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSDynamoDbConfigInitialState(), }, - resource.TestStep{ + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAWSDynamoDbTable_importTags(t *testing.T) { + resourceName := "aws_dynamodb_table.basic-dynamodb-table" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSDynamoDbConfigTags(), + }, + + { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, diff --git a/builtin/providers/aws/resource_aws_dynamodb_table.go b/builtin/providers/aws/resource_aws_dynamodb_table.go index cd8db72e6..23e99b5b2 100644 --- a/builtin/providers/aws/resource_aws_dynamodb_table.go +++ b/builtin/providers/aws/resource_aws_dynamodb_table.go @@ -40,43 +40,43 @@ func resourceAwsDynamoDbTable() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "arn": &schema.Schema{ + "arn": { Type: schema.TypeString, Computed: true, }, - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "hash_key": &schema.Schema{ + "hash_key": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "range_key": &schema.Schema{ + "range_key": { Type: schema.TypeString, Optional: true, ForceNew: true, }, - "write_capacity": &schema.Schema{ + "write_capacity": { Type: schema.TypeInt, Required: true, }, - "read_capacity": &schema.Schema{ + "read_capacity": { Type: schema.TypeInt, Required: true, }, - "attribute": &schema.Schema{ + "attribute": { Type: schema.TypeSet, Required: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, }, - "type": &schema.Schema{ + "type": { Type: schema.TypeString, Required: true, }, @@ -89,25 +89,25 @@ func resourceAwsDynamoDbTable() *schema.Resource { return hashcode.String(buf.String()) }, }, - "local_secondary_index": &schema.Schema{ + "local_secondary_index": { Type: schema.TypeSet, Optional: true, ForceNew: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, }, - "range_key": &schema.Schema{ + "range_key": { Type: schema.TypeString, Required: true, }, - "projection_type": &schema.Schema{ + "projection_type": { Type: schema.TypeString, Required: true, }, - "non_key_attributes": &schema.Schema{ + "non_key_attributes": { Type: schema.TypeList, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, @@ -121,36 +121,36 @@ func resourceAwsDynamoDbTable() *schema.Resource { return hashcode.String(buf.String()) }, }, - "global_secondary_index": &schema.Schema{ + "global_secondary_index": { Type: schema.TypeSet, Optional: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, }, - "write_capacity": &schema.Schema{ + "write_capacity": { Type: schema.TypeInt, Required: true, }, - "read_capacity": &schema.Schema{ + "read_capacity": { Type: schema.TypeInt, Required: true, }, - "hash_key": &schema.Schema{ + "hash_key": { Type: schema.TypeString, Required: true, }, - "range_key": &schema.Schema{ + "range_key": { Type: schema.TypeString, Optional: true, }, - "projection_type": &schema.Schema{ + "projection_type": { Type: schema.TypeString, Required: true, }, - "non_key_attributes": &schema.Schema{ + "non_key_attributes": { Type: schema.TypeList, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, @@ -167,12 +167,12 @@ func resourceAwsDynamoDbTable() *schema.Resource { return hashcode.String(buf.String()) }, }, - "stream_enabled": &schema.Schema{ + "stream_enabled": { Type: schema.TypeBool, Optional: true, Computed: true, }, - "stream_view_type": &schema.Schema{ + "stream_view_type": { Type: schema.TypeString, Optional: true, Computed: true, @@ -182,10 +182,11 @@ func resourceAwsDynamoDbTable() *schema.Resource { }, ValidateFunc: validateStreamViewType, }, - "stream_arn": &schema.Schema{ + "stream_arn": { Type: schema.TypeString, Computed: true, }, + "tags": tagsSchema(), }, } } @@ -204,7 +205,7 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er hash_key_name := d.Get("hash_key").(string) keyschema := []*dynamodb.KeySchemaElement{ - &dynamodb.KeySchemaElement{ + { AttributeName: aws.String(hash_key_name), KeyType: aws.String("HASH"), }, @@ -239,7 +240,7 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er } if lsidata, ok := d.GetOk("local_secondary_index"); ok { - fmt.Printf("[DEBUG] Adding LSI data to the table") + log.Printf("[DEBUG] Adding LSI data to the table") lsiSet := lsidata.(*schema.Set) localSecondaryIndexes := []*dynamodb.LocalSecondaryIndex{} @@ -261,11 +262,11 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er localSecondaryIndexes = append(localSecondaryIndexes, &dynamodb.LocalSecondaryIndex{ IndexName: aws.String(lsi["name"].(string)), KeySchema: []*dynamodb.KeySchemaElement{ - &dynamodb.KeySchemaElement{ + { AttributeName: aws.String(hash_key_name), KeyType: aws.String("HASH"), }, - &dynamodb.KeySchemaElement{ + { AttributeName: aws.String(lsi["range_key"].(string)), KeyType: aws.String("RANGE"), }, @@ -276,7 +277,7 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er req.LocalSecondaryIndexes = localSecondaryIndexes - fmt.Printf("[DEBUG] Added %d LSI definitions", len(localSecondaryIndexes)) + log.Printf("[DEBUG] Added %d LSI definitions", len(localSecondaryIndexes)) } if gsidata, ok := d.GetOk("global_secondary_index"); ok { @@ -298,9 +299,11 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er StreamViewType: aws.String(d.Get("stream_view_type").(string)), } - fmt.Printf("[DEBUG] Adding StreamSpecifications to the table") + log.Printf("[DEBUG] Adding StreamSpecifications to the table") } + _, tagsOk := d.GetOk("tags") + attemptCount := 1 for attemptCount <= DYNAMODB_MAX_THROTTLE_RETRIES { output, err := dynamodbconn.CreateTable(req) @@ -325,10 +328,16 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er } else { // No error, set ID and return d.SetId(*output.TableDescription.TableName) - if err := d.Set("arn", *output.TableDescription.TableArn); err != nil { + tableArn := *output.TableDescription.TableArn + if err := d.Set("arn", tableArn); err != nil { return err } - + if tagsOk { + log.Printf("[DEBUG] Setting DynamoDB Tags on arn: %s", tableArn) + if err := createTableTags(d, meta); err != nil { + return err + } + } return resourceAwsDynamoDbTableRead(d, meta) } } @@ -581,6 +590,11 @@ func resourceAwsDynamoDbTableUpdate(d *schema.ResourceData, meta interface{}) er } + // Update tags + if err := setTagsDynamoDB(dynamodbconn, d); err != nil { + return err + } + return resourceAwsDynamoDbTableRead(d, meta) } @@ -700,6 +714,14 @@ func resourceAwsDynamoDbTableRead(d *schema.ResourceData, meta interface{}) erro d.Set("arn", table.TableArn) + tags, err := readTableTags(d, meta) + if err != nil { + return err + } + if len(tags) != 0 { + d.Set("tags", tags) + } + return nil } @@ -770,7 +792,7 @@ func createGSIFromData(data *map[string]interface{}) dynamodb.GlobalSecondaryInd readCapacity := (*data)["read_capacity"].(int) key_schema := []*dynamodb.KeySchemaElement{ - &dynamodb.KeySchemaElement{ + { AttributeName: aws.String((*data)["hash_key"].(string)), KeyType: aws.String("HASH"), }, @@ -890,3 +912,43 @@ func waitForTableToBeActive(tableName string, meta interface{}) error { return nil } + +func createTableTags(d *schema.ResourceData, meta interface{}) error { + // DynamoDB Table has to be in the ACTIVE state in order to tag the resource + if err := waitForTableToBeActive(d.Id(), meta); err != nil { + return err + } + tags := d.Get("tags").(map[string]interface{}) + arn := d.Get("arn").(string) + dynamodbconn := meta.(*AWSClient).dynamodbconn + req := &dynamodb.TagResourceInput{ + ResourceArn: aws.String(arn), + Tags: tagsFromMapDynamoDB(tags), + } + _, err := dynamodbconn.TagResource(req) + if err != nil { + return fmt.Errorf("Error tagging dynamodb resource: %s", err) + } + return nil +} + +func readTableTags(d *schema.ResourceData, meta interface{}) (map[string]string, error) { + if err := waitForTableToBeActive(d.Id(), meta); err != nil { + return nil, err + } + arn := d.Get("arn").(string) + //result := make(map[string]string) + + dynamodbconn := meta.(*AWSClient).dynamodbconn + req := &dynamodb.ListTagsOfResourceInput{ + ResourceArn: aws.String(arn), + } + + output, err := dynamodbconn.ListTagsOfResource(req) + if err != nil { + return nil, fmt.Errorf("Error reading tags from dynamodb resource: %s", err) + } + result := tagsToMapDynamoDB(output.Tags) + // TODO Read NextToken if avail + return result, nil +} diff --git a/builtin/providers/aws/resource_aws_dynamodb_table_test.go b/builtin/providers/aws/resource_aws_dynamodb_table_test.go index 044497303..a5f2a3a70 100644 --- a/builtin/providers/aws/resource_aws_dynamodb_table_test.go +++ b/builtin/providers/aws/resource_aws_dynamodb_table_test.go @@ -19,13 +19,13 @@ func TestAccAWSDynamoDbTable_basic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSDynamoDbConfigInitialState(), Check: resource.ComposeTestCheckFunc( testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table"), ), }, - resource.TestStep{ + { Config: testAccAWSDynamoDbConfigAddSecondaryGSI, Check: resource.ComposeTestCheckFunc( testAccCheckDynamoDbTableWasUpdated("aws_dynamodb_table.basic-dynamodb-table"), @@ -41,7 +41,7 @@ func TestAccAWSDynamoDbTable_streamSpecification(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSDynamoDbConfigStreamSpecification(), Check: resource.ComposeTestCheckFunc( testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table"), @@ -55,6 +55,24 @@ func TestAccAWSDynamoDbTable_streamSpecification(t *testing.T) { }) } +func TestAccAWSDynamoDBTable_tags(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSDynamoDbConfigTags(), + Check: resource.ComposeTestCheckFunc( + testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table"), + resource.TestCheckResourceAttr( + "aws_dynamodb_table.basic-dynamodb-table", "tags.%", "3"), + ), + }, + }, + }) +} + func TestResourceAWSDynamoDbTableStreamViewType_validation(t *testing.T) { cases := []struct { Value string @@ -127,7 +145,7 @@ func testAccCheckAWSDynamoDbTableDestroy(s *terraform.State) error { func testAccCheckInitialAWSDynamoDbTableExists(n string) resource.TestCheckFunc { return func(s *terraform.State) error { - fmt.Printf("[DEBUG] Trying to create initial table state!") + log.Printf("[DEBUG] Trying to create initial table state!") rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("Not found: %s", n) @@ -146,13 +164,12 @@ func testAccCheckInitialAWSDynamoDbTableExists(n string) resource.TestCheckFunc resp, err := conn.DescribeTable(params) if err != nil { - fmt.Printf("[ERROR] Problem describing table '%s': %s", rs.Primary.ID, err) - return err + return fmt.Errorf("[ERROR] Problem describing table '%s': %s", rs.Primary.ID, err) } table := resp.Table - fmt.Printf("[DEBUG] Checking on table %s", rs.Primary.ID) + log.Printf("[DEBUG] Checking on table %s", rs.Primary.ID) if *table.ProvisionedThroughput.WriteCapacityUnits != 20 { return fmt.Errorf("Provisioned write capacity was %d, not 20!", table.ProvisionedThroughput.WriteCapacityUnits) @@ -404,3 +421,49 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" { } `, acctest.RandInt()) } + +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" + } +} +`, acctest.RandInt(), acctest.RandInt()) +} diff --git a/builtin/providers/aws/tags.go b/builtin/providers/aws/tags.go index 758ea6238..13df84a22 100644 --- a/builtin/providers/aws/tags.go +++ b/builtin/providers/aws/tags.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/dynamodb" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/elbv2" "github.com/hashicorp/terraform/helper/resource" @@ -247,3 +248,103 @@ func tagIgnoredELBv2(t *elbv2.Tag) bool { } return false } + +// tagsToMapDynamoDB turns the list of tags into a map for dynamoDB +func tagsToMapDynamoDB(ts []*dynamodb.Tag) map[string]string { + result := make(map[string]string) + for _, t := range ts { + result[*t.Key] = *t.Value + } + return result +} + +// tagsFromMapDynamoDB returns the tags for a given map +func tagsFromMapDynamoDB(m map[string]interface{}) []*dynamodb.Tag { + result := make([]*dynamodb.Tag, 0, len(m)) + for k, v := range m { + t := &dynamodb.Tag{ + Key: aws.String(k), + Value: aws.String(v.(string)), + } + result = append(result, t) + } + return result +} + +// setTagsDynamoDB is a helper to set the tags for a dynamoDB resource +// This is needed because dynamodb requires a completely different set and delete +// method from the ec2 tag resource handling. Also the `UntagResource` method +// for dynamoDB only requires a list of tag keys, instead of the full map of keys. +func setTagsDynamoDB(conn *dynamodb.DynamoDB, d *schema.ResourceData) error { + if d.HasChange("tags") { + arn := d.Get("arn").(string) + oraw, nraw := d.GetChange("tags") + o := oraw.(map[string]interface{}) + n := nraw.(map[string]interface{}) + create, remove := diffTagsDynamoDB(tagsFromMapDynamoDB(o), tagsFromMapDynamoDB(n)) + + // Set tags + if len(remove) > 0 { + err := resource.Retry(2*time.Minute, func() *resource.RetryError { + log.Printf("[DEBUG] Removing tags: %#v from %s", remove, d.Id()) + _, err := conn.UntagResource(&dynamodb.UntagResourceInput{ + ResourceArn: aws.String(arn), + TagKeys: remove, + }) + if err != nil { + ec2err, ok := err.(awserr.Error) + if ok && strings.Contains(ec2err.Code(), "ResourceNotFoundException") { + return resource.RetryableError(err) // retry + } + return resource.NonRetryableError(err) + } + return nil + }) + if err != nil { + return err + } + } + if len(create) > 0 { + err := resource.Retry(2*time.Minute, func() *resource.RetryError { + log.Printf("[DEBUG] Creating tags: %s for %s", create, d.Id()) + _, err := conn.TagResource(&dynamodb.TagResourceInput{ + ResourceArn: aws.String(arn), + Tags: create, + }) + if err != nil { + ec2err, ok := err.(awserr.Error) + if ok && strings.Contains(ec2err.Code(), "ResourceNotFoundException") { + return resource.RetryableError(err) // retry + } + return resource.NonRetryableError(err) + } + return nil + }) + if err != nil { + return err + } + } + } + + return nil +} + +// diffTagsDynamoDB takes a local set of dynamodb tags and the ones found remotely +// and returns the set of tags that must be created as a map, and returns a list of tag keys +// that must be destroyed. +func diffTagsDynamoDB(oldTags, newTags []*dynamodb.Tag) ([]*dynamodb.Tag, []*string) { + create := make(map[string]interface{}) + for _, t := range newTags { + create[*t.Key] = *t.Value + } + + var remove []*string + for _, t := range oldTags { + // Verify the old tag is not a tag we're currently attempting to create + old, ok := create[*t.Key] + if !ok || old != *t.Value { + remove = append(remove, t.Key) + } + } + return tagsFromMapDynamoDB(create), remove +} diff --git a/website/source/docs/providers/aws/r/dynamodb_table.html.markdown b/website/source/docs/providers/aws/r/dynamodb_table.html.markdown index 31a08263e..1f06f98e7 100644 --- a/website/source/docs/providers/aws/r/dynamodb_table.html.markdown +++ b/website/source/docs/providers/aws/r/dynamodb_table.html.markdown @@ -43,6 +43,10 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" { projection_type = "INCLUDE" non_key_attributes = [ "UserId" ] } + tags { + Name = "dynamodb-table-1" + Environment = "production" + } } ``` @@ -69,6 +73,7 @@ definition after you have created the resource. * `global_secondary_index` - (Optional) Describe a GSO for the table; subject to the normal limits on the number of GSIs, projected attributes, etc. +* `tags` - (Optional) A map of tags to populate on the created table. For both `local_secondary_index` and `global_secondary_index` objects, the following properties are supported: