Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for tag-on-create for dynamodb tables #8469

Merged
merged 2 commits into from
May 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions aws/resource_aws_dynamodb_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,13 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er

log.Printf("[DEBUG] Creating DynamoDB table with key schema: %#v", keySchemaMap)

tags := tagsFromMapDynamoDb(d.Get("tags").(map[string]interface{}))

req := &dynamodb.CreateTableInput{
TableName: aws.String(d.Get("name").(string)),
BillingMode: aws.String(d.Get("billing_mode").(string)),
KeySchema: expandDynamoDbKeySchema(keySchemaMap),
Tags: tags,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that AWS GovCloud (US) has unfortunately not been updated for tag-on-create support yet, returning the following error:

--- FAIL: TestAccAWSDynamoDbTable_basic (9.68s)
    testing.go:568: Step 0 error: errors during apply:

        Error: error creating DynamoDB Table: ValidationException: Unsupported input parameter Tags

To workaround this error while automatically supporting tag-on-create without code change in the future, we can catch this specific error during our resource.Retry() logic, set the parameter to nil, and perform a similar tagging API call after creation as it did previously. e.g.

	var requiresTagging bool
// ...
			// AWS GovCloud (US) and others may reply with the following until their API is updated:
			// ValidationException: Unsupported input parameter Tags
			if isAWSErr(err, "ValidationException", "Unsupported input parameter Tags") {
				req.Tags = nil
				requiresTagging = true
				return resource.RetryableError(err)
			}
// ...
	if requiresTagging {
		if err := setTagsDynamoDb(conn, d); err != nil {
			return fmt.Errorf("error adding DynamoDB Table (%s) tags: %s", d.Id(), err)
		}
	}

}

billingMode := d.Get("billing_mode").(string)
Expand Down Expand Up @@ -392,10 +395,6 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er
}
}

if err := setTagsDynamoDb(conn, d); err != nil {
return fmt.Errorf("error adding DynamoDB Table (%s) tags: %s", d.Id(), err)
}

if d.Get("point_in_time_recovery.0.enabled").(bool) {
if err := updateDynamoDbPITR(d, conn); err != nil {
return fmt.Errorf("error enabling DynamoDB Table (%s) point in time recovery: %s", d.Id(), err)
Expand Down Expand Up @@ -550,10 +549,11 @@ func resourceAwsDynamoDbTableUpdate(d *schema.ResourceData, meta interface{}) er
return fmt.Errorf("error updating DynamoDB Table (%s) time to live: %s", d.Id(), err)
}
}

if d.HasChange("tags") {
if err := setTagsDynamoDb(conn, d); err != nil {
return fmt.Errorf("error updating DynamoDB Table (%s) tags: %s", d.Id(), err)
if !d.IsNewResource() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that the Create function returns the Read function instead of the Update function (as is best practice) so this check is extraneous. 👍

if d.HasChange("tags") {
if err := setTagsDynamoDb(conn, d); err != nil {
return fmt.Errorf("error updating DynamoDB Table (%s) tags: %s", d.Id(), err)
}
}
}

Expand Down
97 changes: 0 additions & 97 deletions aws/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ 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/hashcode"
Expand Down Expand Up @@ -361,102 +360,6 @@ 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 {
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 {
if isAWSErr(err, dynamodb.ErrCodeResourceNotFoundException, "") {
return resource.RetryableError(err)
}
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 {
if isAWSErr(err, dynamodb.ErrCodeResourceNotFoundException, "") {
return resource.RetryableError(err)
}
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
}

// tagsMapToHash returns a stable hash value for a raw tags map.
// The returned value map be negative (i.e. not suitable for a 'Set' function).
func tagsMapToHash(tags map[string]interface{}) int {
Expand Down
131 changes: 131 additions & 0 deletions aws/tagsDynamoDb.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package aws

import (
"log"
"regexp"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/dynamodb"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
)

// setTags is a helper to set the tags for a resource. It expects the
// tags field to be named "tags" and the ARN field to be named "arn".
func setTagsDynamoDb(conn *dynamodb.DynamoDB, d *schema.ResourceData) error {
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 {
if isAWSErr(err, dynamodb.ErrCodeResourceNotFoundException, "") {
return resource.RetryableError(err)
}
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 {
if isAWSErr(err, dynamodb.ErrCodeResourceNotFoundException, "") {
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}
return nil
})
if err != nil {
return err
}
}

return nil
}

// diffTags takes our tags locally and the ones remotely and returns
// the set of tags that must be created, and the set of tags that must
// be destroyed.
func diffTagsDynamoDb(oldTags, newTags []*dynamodb.Tag) ([]*dynamodb.Tag, []*string) {
// First, we're creating everything we have
create := make(map[string]interface{})
for _, t := range newTags {
create[aws.StringValue(t.Key)] = aws.StringValue(t.Value)
}

// Build the list of what to remove
var remove []*string
for _, t := range oldTags {
old, ok := create[aws.StringValue(t.Key)]
if !ok || old != aws.StringValue(t.Value) {
remove = append(remove, t.Key)
} else if ok {
// already present so remove from new
delete(create, aws.StringValue(t.Key))
}
}

return tagsFromMapDynamoDb(create), remove
}

// tagsFromMapDynamoDb returns the tags for the given map of data.
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)),
}
if !tagIgnoredDynamoDb(t) {
result = append(result, t)
}
}

return result
}

// tagsToMap turns the list of tags into a map.
func tagsToMapDynamoDb(ts []*dynamodb.Tag) map[string]string {
result := make(map[string]string)
for _, t := range ts {
if !tagIgnoredDynamoDb(t) {
result[aws.StringValue(t.Key)] = aws.StringValue(t.Value)
}
}
return result
}

// compare a tag against a list of strings and checks if it should
// be ignored or not
func tagIgnoredDynamoDb(t *dynamodb.Tag) bool {
filter := []string{"^aws:"}
for _, v := range filter {
log.Printf("[DEBUG] Matching %v with %v\n", v, *t.Key)
r, _ := regexp.MatchString(v, *t.Key)
if r {
log.Printf("[DEBUG] Found AWS specific tag %s (val: %s), ignoring.\n", *t.Key, *t.Value)
return true
}
}
return false
}
111 changes: 111 additions & 0 deletions aws/tagsDynamoDb_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package aws

import (
"reflect"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/dynamodb"
)

func TestDiffDynamoDbTags(t *testing.T) {
cases := []struct {
Old, New map[string]interface{}
Create, Remove map[string]string
}{
// Add
{
Old: map[string]interface{}{
"foo": "bar",
},
New: map[string]interface{}{
"foo": "bar",
"bar": "baz",
},
Create: map[string]string{
"bar": "baz",
},
Remove: map[string]string{},
},

// Modify
{
Old: map[string]interface{}{
"foo": "bar",
},
New: map[string]interface{}{
"foo": "baz",
},
Create: map[string]string{
"foo": "baz",
},
Remove: map[string]string{
"foo": "bar",
},
},

// Overlap
{
Old: map[string]interface{}{
"foo": "bar",
"hello": "world",
},
New: map[string]interface{}{
"foo": "baz",
"hello": "world",
},
Create: map[string]string{
"foo": "baz",
},
Remove: map[string]string{
"foo": "bar",
},
},

// Remove
{
Old: map[string]interface{}{
"foo": "bar",
"bar": "baz",
},
New: map[string]interface{}{
"foo": "bar",
},
Create: map[string]string{},
Remove: map[string]string{
"bar": "baz",
},
},
}

for i, tc := range cases {
c, r := diffTagsDynamoDb(tagsFromMapDynamoDb(tc.Old), tagsFromMapDynamoDb(tc.New))
cm := tagsToMapDynamoDb(c)
if !reflect.DeepEqual(cm, tc.Create) {
t.Fatalf("%d: bad create: %#v", i, cm)
}
for _, key := range r {
if _, ok := tc.Remove[aws.StringValue(key)]; !ok {
t.Fatalf("%d: bad remove: %#v", i, key)
}
}
}
}

func TestIgnoringTagsDynamoDb(t *testing.T) {
ignoredTags := []*dynamodb.Tag{
{
Key: aws.String("aws:cloudformation:logical-id"),
Value: aws.String("foo"),
},
{
Key: aws.String("aws:foo:bar"),
Value: aws.String("baz"),
},
}
for _, tag := range ignoredTags {
if !tagIgnoredDynamoDb(tag) {
t.Fatalf("Tag %v with value %v not ignored, but should be!", *tag.Key, *tag.Value)
}
}
}