From 36d51d5f535e68b1ee1e2cbe6a9d48432690dfa8 Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Tue, 26 Mar 2019 19:10:53 +0900 Subject: [PATCH 1/4] Add resource tags support for Cloud Watch Events --- aws/tagsCloudWatchEvent.go | 133 ++++++++++++++++++++++++++++++++ aws/tagsCloudWatchEvent_test.go | 77 ++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 aws/tagsCloudWatchEvent.go create mode 100644 aws/tagsCloudWatchEvent_test.go diff --git a/aws/tagsCloudWatchEvent.go b/aws/tagsCloudWatchEvent.go new file mode 100644 index 000000000000..85c145930d19 --- /dev/null +++ b/aws/tagsCloudWatchEvent.go @@ -0,0 +1,133 @@ +package aws + +import ( + "fmt" + "log" + "regexp" + + "github.com/aws/aws-sdk-go/aws" + events "github.com/aws/aws-sdk-go/service/cloudwatchevents" + "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" +func setTagsCloudWatchEvents(conn *events.CloudWatchEvents, d *schema.ResourceData, arn string) error { + if d.HasChange("tags") { + oraw, nraw := d.GetChange("tags") + o := oraw.(map[string]interface{}) + n := nraw.(map[string]interface{}) + create, remove := diffTagsCloudWatchEvents(tagsFromMapCloudWatchEvents(o), tagsFromMapCloudWatchEvents(n)) + + // Set tags + if len(remove) > 0 { + log.Printf("[DEBUG] Removing tags: %#v", remove) + k := make([]*string, 0, len(remove)) + for _, t := range remove { + k = append(k, t.Key) + } + _, err := conn.UntagResource(&events.UntagResourceInput{ + ResourceARN: aws.String(arn), + TagKeys: k, + }) + if err != nil { + return err + } + } + if len(create) > 0 { + log.Printf("[DEBUG] Creating tags: %#v", create) + _, err := conn.TagResource(&events.TagResourceInput{ + ResourceARN: aws.String(arn), + Tags: create, + }) + 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 diffTagsCloudWatchEvents(oldTags, newTags []*events.Tag) ([]*events.Tag, []*events.Tag) { + // First, we're creating everything we have + create := make(map[string]interface{}) + for _, t := range newTags { + create[*t.Key] = *t.Value + } + + // Build the list of what to remove + var remove []*events.Tag + for _, t := range oldTags { + old, ok := create[*t.Key] + if !ok || old != *t.Value { + // Delete it! + remove = append(remove, t) + } + } + + return tagsFromMapCloudWatchEvents(create), remove +} + +// tagsFromMap returns the tags for the given map of data. +func tagsFromMapCloudWatchEvents(m map[string]interface{}) []*events.Tag { + var result []*events.Tag + for k, v := range m { + t := &events.Tag{ + Key: aws.String(k), + Value: aws.String(v.(string)), + } + if !tagIgnoredCloudWatchEvents(t) { + result = append(result, t) + } + } + + return result +} + +// tagsToMap turns the list of tags into a map. +func tagsToMapCloudWatchEvents(ts []*events.Tag) map[string]string { + result := make(map[string]string) + for _, t := range ts { + if !tagIgnoredCloudWatchEvents(t) { + result[*t.Key] = *t.Value + } + } + + return result +} + +func saveTagsCloudWatchEvents(conn *events.CloudWatchEvents, d *schema.ResourceData, arn string) error { + resp, err := conn.ListTagsForResource(&events.ListTagsForResourceInput{ + ResourceARN: aws.String(arn), + }) + + if err != nil { + return fmt.Errorf("Error retreiving tags for ARN: %s", arn) + } + + var tagList []*events.Tag + if len(resp.Tags) > 0 { + tagList = resp.Tags + } + + return d.Set("tags", tagsToMapCloudWatchEvents(tagList)) +} + +// compare a tag against a list of strings and checks if it should +// be ignored or not +func tagIgnoredCloudWatchEvents(t *events.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 +} diff --git a/aws/tagsCloudWatchEvent_test.go b/aws/tagsCloudWatchEvent_test.go new file mode 100644 index 000000000000..9c8b89386318 --- /dev/null +++ b/aws/tagsCloudWatchEvent_test.go @@ -0,0 +1,77 @@ +package aws + +import ( + "reflect" + "testing" + + "github.com/aws/aws-sdk-go/aws" + events "github.com/aws/aws-sdk-go/service/cloudwatchevents" +) + +func TestDiffCloudWatchEventTags(t *testing.T) { + cases := []struct { + Old, New map[string]interface{} + Create, Remove map[string]string + }{ + // Basic add/remove + { + Old: map[string]interface{}{ + "foo": "bar", + }, + New: map[string]interface{}{ + "bar": "baz", + }, + Create: map[string]string{ + "bar": "baz", + }, + Remove: map[string]string{ + "foo": "bar", + }, + }, + + // 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", + }, + }, + } + + for i, tc := range cases { + c, r := diffTagsCloudWatchEvents(tagsFromMapCloudWatchEvents(tc.Old), tagsFromMapCloudWatchEvents(tc.New)) + cm := tagsToMapCloudWatchEvents(c) + rm := tagsToMapCloudWatchEvents(r) + if !reflect.DeepEqual(cm, tc.Create) { + t.Fatalf("%d: bad create: %#v", i, cm) + } + if !reflect.DeepEqual(rm, tc.Remove) { + t.Fatalf("%d: bad remove: %#v", i, rm) + } + } +} + +func TestIgnoringTagsCloudWatchEvents(t *testing.T) { + var ignoredTags []*events.Tag + ignoredTags = append(ignoredTags, &events.Tag{ + Key: aws.String("aws:cloudformation:logical-id"), + Value: aws.String("foo"), + }) + ignoredTags = append(ignoredTags, &events.Tag{ + Key: aws.String("aws:foo:bar"), + Value: aws.String("baz"), + }) + for _, tag := range ignoredTags { + if !tagIgnoredCloudWatchEvents(tag) { + t.Fatalf("Tag %v with value %v not ignored, but should be!", *tag.Key, *tag.Value) + } + } +} From 9e19b6c55a017d7240de063888b4ff3ec81f0746 Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Tue, 26 Mar 2019 19:12:25 +0900 Subject: [PATCH 2/4] Add tags attribute for aws_cloudwatch_event_rule resource --- aws/resource_aws_cloudwatch_event_rule.go | 12 ++++ ...resource_aws_cloudwatch_event_rule_test.go | 59 +++++++++++++++++++ .../r/cloudwatch_event_rule.html.markdown | 1 + 3 files changed, 72 insertions(+) diff --git a/aws/resource_aws_cloudwatch_event_rule.go b/aws/resource_aws_cloudwatch_event_rule.go index 2c5dcfd6e5a6..f12d6322f529 100644 --- a/aws/resource_aws_cloudwatch_event_rule.go +++ b/aws/resource_aws_cloudwatch_event_rule.go @@ -73,6 +73,7 @@ func resourceAwsCloudWatchEventRule() *schema.Resource { Type: schema.TypeString, Computed: true, }, + "tags": tagsSchema(), }, } } @@ -121,6 +122,10 @@ func resourceAwsCloudWatchEventRuleCreate(d *schema.ResourceData, meta interface log.Printf("[INFO] CloudWatch Event Rule %q created", *out.RuleArn) + if err := setTagsCloudWatchEvents(conn, d, aws.StringValue(out.RuleArn)); err != nil { + return fmt.Errorf("Error creating tags for %s: %s", d.Id(), err) + } + return resourceAwsCloudWatchEventRuleUpdate(d, meta) } @@ -163,6 +168,7 @@ func resourceAwsCloudWatchEventRuleRead(d *schema.ResourceData, meta interface{} } log.Printf("[DEBUG] Setting boolean state: %t", boolState) d.Set("is_enabled", boolState) + d.Set("tags", saveTagsCloudWatchEvents(conn, d, aws.StringValue(out.Arn))) return nil } @@ -217,6 +223,12 @@ func resourceAwsCloudWatchEventRuleUpdate(d *schema.ResourceData, meta interface log.Printf("[DEBUG] CloudWatch Event Rule (%q) disabled", d.Id()) } + if d.HasChange("tags") { + if err := setTagsCloudWatchEvents(conn, d, d.Get("arn").(string)); err != nil { + return fmt.Errorf("Error updating tags for %s: %s", d.Id(), err) + } + } + return resourceAwsCloudWatchEventRuleRead(d, meta) } diff --git a/aws/resource_aws_cloudwatch_event_rule_test.go b/aws/resource_aws_cloudwatch_event_rule_test.go index 3e25e0db32aa..0d57993a4278 100644 --- a/aws/resource_aws_cloudwatch_event_rule_test.go +++ b/aws/resource_aws_cloudwatch_event_rule_test.go @@ -136,6 +136,36 @@ func TestAccAWSCloudWatchEventRule_prefix(t *testing.T) { }) } +func TestAccAWSCloudWatchEventRule_tags(t *testing.T) { + var rule events.DescribeRuleOutput + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSCloudWatchEventRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSCloudWatchEventRuleConfig_tags, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudWatchEventRuleExists("aws_cloudwatch_event_rule.default", &rule), + resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.%", "2"), + resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.fizz", "buzz"), + resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.foo", "bar"), + ), + }, + { + Config: testAccAWSCloudWatchEventRuleConfig_updateTags, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudWatchEventRuleExists("aws_cloudwatch_event_rule.default", &rule), + resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.%", "2"), + resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.foo", "bar2"), + resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.good", "bad"), + ), + }, + }, + }) +} + func TestAccAWSCloudWatchEventRule_full(t *testing.T) { var rule events.DescribeRuleOutput @@ -154,6 +184,8 @@ func TestAccAWSCloudWatchEventRule_full(t *testing.T) { resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.moobar", "description", "He's not dead, he's just resting!"), resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.moobar", "role_arn", ""), testAccCheckCloudWatchEventRuleEnabled("aws_cloudwatch_event_rule.moobar", "DISABLED", &rule), + resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.moobar", "tags.%", "1"), + resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.moobar", "tags.Name", "tf-acc-cw-event-rule-full"), ), }, }, @@ -356,6 +388,30 @@ PATTERN } ` +var testAccAWSCloudWatchEventRuleConfig_tags = ` +resource "aws_cloudwatch_event_rule" "default" { + name = "tf-acc-cw-event-rule" + schedule_expression = "rate(1 hour)" + + tags = { + fizz = "buzz" + foo = "bar" + } +} +` + +var testAccAWSCloudWatchEventRuleConfig_updateTags = ` +resource "aws_cloudwatch_event_rule" "default" { + name = "tf-acc-cw-event-rule" + schedule_expression = "rate(1 hour)" + + tags = { + foo = "bar2" + good = "bad" + } +} +` + var testAccAWSCloudWatchEventRuleConfig_full = ` resource "aws_cloudwatch_event_rule" "moobar" { name = "tf-acc-cw-event-rule-full" @@ -365,6 +421,9 @@ resource "aws_cloudwatch_event_rule" "moobar" { PATTERN description = "He's not dead, he's just resting!" is_enabled = false + tags = { + Name = "tf-acc-cw-event-rule-full" + } } ` diff --git a/website/docs/r/cloudwatch_event_rule.html.markdown b/website/docs/r/cloudwatch_event_rule.html.markdown index f79733b5e529..e4fb8ec04382 100644 --- a/website/docs/r/cloudwatch_event_rule.html.markdown +++ b/website/docs/r/cloudwatch_event_rule.html.markdown @@ -70,6 +70,7 @@ The following arguments are supported: * `description` - (Optional) The description of the rule. * `role_arn` - (Optional) The Amazon Resource Name (ARN) associated with the role that is used for target invocation. * `is_enabled` - (Optional) Whether the rule should be enabled (defaults to `true`). +* `tags` - (Optional) A mapping of tags to assign to the resource. ## Attributes Reference From bc3355b83123208ba6ac0adfd2b62e1337013f2e Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Fri, 29 Mar 2019 11:51:52 +0900 Subject: [PATCH 3/4] fix setting tags to state --- aws/resource_aws_cloudwatch_event_rule.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_cloudwatch_event_rule.go b/aws/resource_aws_cloudwatch_event_rule.go index f12d6322f529..853830ed7115 100644 --- a/aws/resource_aws_cloudwatch_event_rule.go +++ b/aws/resource_aws_cloudwatch_event_rule.go @@ -168,8 +168,9 @@ func resourceAwsCloudWatchEventRuleRead(d *schema.ResourceData, meta interface{} } log.Printf("[DEBUG] Setting boolean state: %t", boolState) d.Set("is_enabled", boolState) - d.Set("tags", saveTagsCloudWatchEvents(conn, d, aws.StringValue(out.Arn))) - + if err := saveTagsCloudWatchEvents(conn, d, aws.StringValue(out.Arn)); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } return nil } From de999c07961a75c27183d7678459a1b641e26ee6 Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Fri, 29 Mar 2019 11:52:10 +0900 Subject: [PATCH 4/4] Add test case for managing tags --- ...resource_aws_cloudwatch_event_rule_test.go | 47 +++++++++++++++---- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/aws/resource_aws_cloudwatch_event_rule_test.go b/aws/resource_aws_cloudwatch_event_rule_test.go index 0d57993a4278..c4ad895fa49f 100644 --- a/aws/resource_aws_cloudwatch_event_rule_test.go +++ b/aws/resource_aws_cloudwatch_event_rule_test.go @@ -138,6 +138,7 @@ func TestAccAWSCloudWatchEventRule_prefix(t *testing.T) { func TestAccAWSCloudWatchEventRule_tags(t *testing.T) { var rule events.DescribeRuleOutput + resourceName := "aws_cloudwatch_event_rule.default" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -147,21 +148,35 @@ func TestAccAWSCloudWatchEventRule_tags(t *testing.T) { { Config: testAccAWSCloudWatchEventRuleConfig_tags, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventRuleExists("aws_cloudwatch_event_rule.default", &rule), - resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.%", "2"), - resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.fizz", "buzz"), - resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.foo", "bar"), + testAccCheckCloudWatchEventRuleExists(resourceName, &rule), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.fizz", "buzz"), + resource.TestCheckResourceAttr(resourceName, "tags.foo", "bar"), ), }, { Config: testAccAWSCloudWatchEventRuleConfig_updateTags, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventRuleExists("aws_cloudwatch_event_rule.default", &rule), - resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.%", "2"), - resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.foo", "bar2"), - resource.TestCheckResourceAttr("aws_cloudwatch_event_rule.default", "tags.good", "bad"), + testAccCheckCloudWatchEventRuleExists(resourceName, &rule), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.fizz", "buzz"), + resource.TestCheckResourceAttr(resourceName, "tags.foo", "bar2"), + resource.TestCheckResourceAttr(resourceName, "tags.good", "bad"), ), }, + { + Config: testAccAWSCloudWatchEventRuleConfig_removeTags, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudWatchEventRuleExists(resourceName, &rule), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.fizz", "buzz"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } @@ -390,7 +405,7 @@ PATTERN var testAccAWSCloudWatchEventRuleConfig_tags = ` resource "aws_cloudwatch_event_rule" "default" { - name = "tf-acc-cw-event-rule" + name = "tf-acc-cw-event-rule-tags" schedule_expression = "rate(1 hour)" tags = { @@ -402,16 +417,28 @@ resource "aws_cloudwatch_event_rule" "default" { var testAccAWSCloudWatchEventRuleConfig_updateTags = ` resource "aws_cloudwatch_event_rule" "default" { - name = "tf-acc-cw-event-rule" + name = "tf-acc-cw-event-rule-tags" schedule_expression = "rate(1 hour)" tags = { + fizz = "buzz" foo = "bar2" good = "bad" } } ` +var testAccAWSCloudWatchEventRuleConfig_removeTags = ` +resource "aws_cloudwatch_event_rule" "default" { + name = "tf-acc-cw-event-rule-tags" + schedule_expression = "rate(1 hour)" + + tags = { + fizz = "buzz" + } +} +` + var testAccAWSCloudWatchEventRuleConfig_full = ` resource "aws_cloudwatch_event_rule" "moobar" { name = "tf-acc-cw-event-rule-full"