From aa9102b2bd315e11e9b2cf5174a8818680569976 Mon Sep 17 00:00:00 2001 From: Sunil Kumar Mohanty Date: Sat, 27 Apr 2019 09:42:21 +0300 Subject: [PATCH 1/3] add support for tags for SNS topic --- aws/resource_aws_sns_topic.go | 22 ++++- aws/resource_aws_sns_topic_test.go | 63 +++++++++++++- aws/tagsSNS.go | 135 +++++++++++++++++++++++++++++ aws/tagsSNS_test.go | 110 +++++++++++++++++++++++ 4 files changed, 327 insertions(+), 3 deletions(-) create mode 100644 aws/tagsSNS.go create mode 100644 aws/tagsSNS_test.go diff --git a/aws/resource_aws_sns_topic.go b/aws/resource_aws_sns_topic.go index df1006e5ea5..13f209e7180 100644 --- a/aws/resource_aws_sns_topic.go +++ b/aws/resource_aws_sns_topic.go @@ -144,13 +144,14 @@ func resourceAwsSnsTopic() *schema.Resource { Type: schema.TypeString, Computed: true, }, + "tags": tagsSchema(), }, } } func resourceAwsSnsTopicCreate(d *schema.ResourceData, meta interface{}) error { snsconn := meta.(*AWSClient).snsconn - + tags := tagsFromMapSNS(d.Get("tags").(map[string]interface{})) var name string if v, ok := d.GetOk("name"); ok { name = v.(string) @@ -164,6 +165,7 @@ func resourceAwsSnsTopicCreate(d *schema.ResourceData, meta interface{}) error { req := &sns.CreateTopicInput{ Name: aws.String(name), + Tags: tags, } output, err := snsconn.CreateTopic(req) @@ -172,7 +174,6 @@ func resourceAwsSnsTopicCreate(d *schema.ResourceData, meta interface{}) error { } d.SetId(*output.TopicArn) - return resourceAwsSnsTopicUpdate(d, meta) } @@ -188,6 +189,11 @@ func resourceAwsSnsTopicUpdate(d *schema.ResourceData, meta interface{}) error { } } } + if !d.IsNewResource() { + if err := setTagsSNS(conn, d); err != nil { + return fmt.Errorf("error updating SNS Topic tags for %s: %s", d.Id(), err) + } + } return resourceAwsSnsTopicRead(d, meta) } @@ -231,6 +237,18 @@ func resourceAwsSnsTopicRead(d *schema.ResourceData, meta interface{}) error { } } + // List tags + + tagList, err := snsconn.ListTagsForResource(&sns.ListTagsForResourceInput{ + ResourceArn: aws.String(d.Id()), + }) + if err != nil { + return fmt.Errorf("error listing SNS Topic tags for %s: %s", d.Id(), err) + } + if err := d.Set("tags", tagsToMapSNS(tagList.Tags)); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + return nil } diff --git a/aws/resource_aws_sns_topic_test.go b/aws/resource_aws_sns_topic_test.go index 543af3f0c79..9ed1f7cc9fd 100644 --- a/aws/resource_aws_sns_topic_test.go +++ b/aws/resource_aws_sns_topic_test.go @@ -11,7 +11,7 @@ import ( "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" - "github.com/jen20/awspolicyequivalence" + awspolicy "github.com/jen20/awspolicyequivalence" ) func TestAccAWSSNSTopic_importBasic(t *testing.T) { @@ -256,6 +256,46 @@ func TestAccAWSSNSTopic_encryption(t *testing.T) { }) } +func TestAccAWSSNSTopic_tags(t *testing.T) { + attributes := make(map[string]string) + + rName := acctest.RandString(10) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_sns_topic.test_topic", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSNSTopicDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSNSTopicConfigTags1(rName, "key1", "value1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSNSTopicExists("aws_sns_topic.test_topic", attributes), + resource.TestCheckResourceAttr("aws_sns_topic.test_topic", "tags.%", "1"), + resource.TestCheckResourceAttr("aws_sns_topic.test_topic", "tags.key1", "value1"), + ), + }, + { + Config: testAccAWSSNSTopicConfigTags2(rName, "key1", "value1updated", "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSNSTopicExists("aws_sns_topic.test_topic", attributes), + resource.TestCheckResourceAttr("aws_sns_topic.test_topic", "tags.%", "2"), + resource.TestCheckResourceAttr("aws_sns_topic.test_topic", "tags.key1", "value1updated"), + resource.TestCheckResourceAttr("aws_sns_topic.test_topic", "tags.key2", "value2"), + ), + }, + { + Config: testAccAWSSNSTopicConfigTags1(rName, "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSNSTopicExists("aws_sns_topic.test_topic", attributes), + resource.TestCheckResourceAttr("aws_sns_topic.test_topic", "tags.%", "1"), + resource.TestCheckResourceAttr("aws_sns_topic.test_topic", "tags.key2", "value2"), + ), + }, + }, + }) +} + func testAccCheckAWSNSTopicHasPolicy(n string, expectedPolicyText string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -631,3 +671,24 @@ resource "aws_sns_topic" "test_topic" { } `, r) } + +func testAccAWSSNSTopicConfigTags1(r, tag1Key, tag1Value string) string { + return fmt.Sprintf(` +resource "aws_sns_topic" "test_topic" { + name = "terraform-test-topic-%s" + tags = { + %q = %q + } + }`, r, tag1Key, tag1Value) +} + +func testAccAWSSNSTopicConfigTags2(r, tag1Key, tag1Value, tag2Key, tag2Value string) string { + return fmt.Sprintf(` +resource "aws_sns_topic" "test_topic" { + name = "terraform-test-topic-%s" + tags = { + %q = %q + %q = %q + } + }`, r, tag1Key, tag1Value, tag2Key, tag2Value) +} diff --git a/aws/tagsSNS.go b/aws/tagsSNS.go new file mode 100644 index 00000000000..d25cb01792e --- /dev/null +++ b/aws/tagsSNS.go @@ -0,0 +1,135 @@ +package aws + +import ( + "log" + "regexp" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/sns" + "github.com/hashicorp/terraform/helper/schema" +) + +// getTags is a helper to get the tags for a resource. It expects the +// tags field to be named "tags" and the ARN field to be named "arn". +func getTagsSNS(conn *sns.SNS, d *schema.ResourceData) error { + resp, err := conn.ListTagsForResource(&sns.ListTagsForResourceInput{ + ResourceArn: aws.String(d.Get("arn").(string)), + }) + if err != nil { + return err + } + + if err := d.Set("tags", tagsToMapSNS(resp.Tags)); err != nil { + return err + } + + return nil +} + +// 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 setTagsSNS(conn *sns.SNS, d *schema.ResourceData) error { + if d.HasChange("tags") { + oraw, nraw := d.GetChange("tags") + o := oraw.(map[string]interface{}) + n := nraw.(map[string]interface{}) + create, remove := diffTagsSNS(tagsFromMapSNS(o), tagsFromMapSNS(n)) + + // Set tags + if len(remove) > 0 { + log.Printf("[DEBUG] Removing tags: %#v", remove) + k := make([]*string, len(remove)) + for i, t := range remove { + k[i] = t.Key + } + + _, err := conn.UntagResource(&sns.UntagResourceInput{ + ResourceArn: aws.String(d.Get("arn").(string)), + TagKeys: k, + }) + if err != nil { + return err + } + } + if len(create) > 0 { + log.Printf("[DEBUG] Creating tags: %#v", create) + _, err := conn.TagResource(&sns.TagResourceInput{ + ResourceArn: aws.String(d.Get("arn").(string)), + 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 diffTagsSNS(oldTags, newTags []*sns.Tag) ([]*sns.Tag, []*sns.Tag) { + // 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 []*sns.Tag + for _, t := range oldTags { + old, ok := create[aws.StringValue(t.Key)] + if !ok || old != aws.StringValue(t.Value) { + remove = append(remove, t) + } else if ok { + // already present so remove from new + delete(create, aws.StringValue(t.Key)) + } + } + + return tagsFromMapSNS(create), remove +} + +// tagsFromMap returns the tags for the given map of data. +func tagsFromMapSNS(m map[string]interface{}) []*sns.Tag { + result := make([]*sns.Tag, 0, len(m)) + for k, v := range m { + t := &sns.Tag{ + Key: aws.String(k), + Value: aws.String(v.(string)), + } + if !tagIgnoredSNS(t) { + result = append(result, t) + } + } + + return result +} + +// tagsToMap turns the list of tags into a map. +func tagsToMapSNS(ts []*sns.Tag) map[string]string { + result := make(map[string]string) + for _, t := range ts { + if !tagIgnoredSNS(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 tagIgnoredSNS(t *sns.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/tagsSNS_test.go b/aws/tagsSNS_test.go new file mode 100644 index 00000000000..77a9c149750 --- /dev/null +++ b/aws/tagsSNS_test.go @@ -0,0 +1,110 @@ +package aws + +import ( + "reflect" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/sns" +) + +func TestDiffSNSTags(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 := diffTagsSNS(tagsFromMapSNS(tc.Old), tagsFromMapSNS(tc.New)) + cm := tagsToMapSNS(c) + rm := tagsToMapSNS(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 TestIgnoringTagsSNS(t *testing.T) { + ignoredTags := []*sns.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 !tagIgnoredSNS(tag) { + t.Fatalf("Tag %v with value %v not ignored, but should be!", *tag.Key, *tag.Value) + } + } +} From cf57c386a13a5ac96c779c03817d1948c9170ce6 Mon Sep 17 00:00:00 2001 From: Sunil Kumar Mohanty Date: Sat, 27 Apr 2019 09:46:28 +0300 Subject: [PATCH 2/3] update documentation for sns topic --- website/docs/r/sns_topic.html.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/r/sns_topic.html.markdown b/website/docs/r/sns_topic.html.markdown index c40a95144fa..b7249d16175 100644 --- a/website/docs/r/sns_topic.html.markdown +++ b/website/docs/r/sns_topic.html.markdown @@ -80,6 +80,7 @@ The following arguments are supported: * `sqs_success_feedback_role_arn` - (Optional) The IAM role permitted to receive success feedback for this topic * `sqs_success_feedback_sample_rate` - (Optional) Percentage of success to sample * `sqs_failure_feedback_role_arn` - (Optional) IAM role for failure feedback +* `tags` - (Optional) Key-value mapping of resource tags ## Attributes Reference From 6654fa221d3f3b67b956da636a680bd405d7ca70 Mon Sep 17 00:00:00 2001 From: Sunil Kumar Mohanty Date: Sat, 27 Apr 2019 10:02:30 +0300 Subject: [PATCH 3/3] remove unused function getTagsSNS --- aws/tagsSNS.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/aws/tagsSNS.go b/aws/tagsSNS.go index d25cb01792e..66e198c3736 100644 --- a/aws/tagsSNS.go +++ b/aws/tagsSNS.go @@ -9,23 +9,6 @@ import ( "github.com/hashicorp/terraform/helper/schema" ) -// getTags is a helper to get the tags for a resource. It expects the -// tags field to be named "tags" and the ARN field to be named "arn". -func getTagsSNS(conn *sns.SNS, d *schema.ResourceData) error { - resp, err := conn.ListTagsForResource(&sns.ListTagsForResourceInput{ - ResourceArn: aws.String(d.Get("arn").(string)), - }) - if err != nil { - return err - } - - if err := d.Set("tags", tagsToMapSNS(resp.Tags)); err != nil { - return err - } - - return nil -} - // 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 setTagsSNS(conn *sns.SNS, d *schema.ResourceData) error {