From e752270d7b3b1f895cc1923fbdf247bda0e82caa Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Thu, 28 Mar 2019 19:02:43 +0900 Subject: [PATCH 1/4] Add tag resources method for appmesh service --- aws/tagsAppmesh.go | 128 ++++++++++++++++++++++++++++++++++++++++ aws/tagsAppmesh_test.go | 79 +++++++++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 aws/tagsAppmesh.go create mode 100644 aws/tagsAppmesh_test.go diff --git a/aws/tagsAppmesh.go b/aws/tagsAppmesh.go new file mode 100644 index 000000000000..2042b1beb3e9 --- /dev/null +++ b/aws/tagsAppmesh.go @@ -0,0 +1,128 @@ +package aws + +import ( + "log" + "regexp" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/appmesh" + "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 setTagsAppmesh(conn *appmesh.AppMesh, 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 := diffTagsAppmesh(tagsFromMapAppmesh(o), tagsFromMapAppmesh(n)) + + // Set tags + if len(remove) > 0 { + input := appmesh.UntagResourceInput{ + ResourceArn: aws.String(arn), + TagKeys: remove, + } + log.Printf("[DEBUG] Removing Appmesh tags: %s", input) + _, err := conn.UntagResource(&input) + if err != nil { + return err + } + } + if len(create) > 0 { + input := appmesh.TagResourceInput{ + ResourceArn: aws.String(arn), + Tags: create, + } + log.Printf("[DEBUG] Adding Appmesh tags: %s", input) + _, err := conn.TagResource(&input) + 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 diffTagsAppmesh(oldTags, newTags []*appmesh.TagRef) ([]*appmesh.TagRef, []*string) { + // 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 []*string + for _, t := range oldTags { + if _, ok := create[*t.Key]; !ok { + // Delete it! + remove = append(remove, t.Key) + } + } + + return tagsFromMapAppmesh(create), remove +} + +// tagsFromMap returns the tags for the given map of data. +func tagsFromMapAppmesh(m map[string]interface{}) []*appmesh.TagRef { + var result []*appmesh.TagRef + for k, v := range m { + t := &appmesh.TagRef{ + Key: aws.String(k), + Value: aws.String(v.(string)), + } + if !tagIgnoredAppmesh(t) { + result = append(result, t) + } + } + + return result +} + +// tagsToMap turns the list of tags into a map. +func tagsToMapAppmesh(ts []*appmesh.TagRef) map[string]string { + result := make(map[string]string) + for _, t := range ts { + if !tagIgnoredAppmesh(t) { + result[*t.Key] = *t.Value + } + } + + return result +} + +func saveTagsAppmesh(conn *appmesh.AppMesh, d *schema.ResourceData, arn string) error { + resp, err := conn.ListTagsForResource(&appmesh.ListTagsForResourceInput{ + ResourceArn: aws.String(arn), + }) + if err != nil { + return err + } + + var dt []*appmesh.TagRef + if len(resp.Tags) > 0 { + dt = resp.Tags + } + + return d.Set("tags", tagsToMapAppmesh(dt)) +} + +// compare a tag against a list of strings and checks if it should +// be ignored or not +func tagIgnoredAppmesh(t *appmesh.TagRef) bool { + filter := []string{"^aws:", "^appmesh:", "Name"} + 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/tagsAppmesh_test.go b/aws/tagsAppmesh_test.go new file mode 100644 index 000000000000..84a80ea943ea --- /dev/null +++ b/aws/tagsAppmesh_test.go @@ -0,0 +1,79 @@ +package aws + +import ( + "reflect" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/appmesh" +) + +func TestDiffAppmeshTags(t *testing.T) { + cases := []struct { + Old, New map[string]interface{} + Create map[string]string + Remove []string + }{ + // Basic add/remove + { + Old: map[string]interface{}{ + "foo": "bar", + }, + New: map[string]interface{}{ + "bar": "baz", + }, + Create: map[string]string{ + "bar": "baz", + }, + Remove: []string{ + "foo", + }, + }, + + // Modify + { + Old: map[string]interface{}{ + "foo": "bar", + }, + New: map[string]interface{}{ + "foo": "baz", + }, + Create: map[string]string{ + "foo": "baz", + }, + Remove: []string{}, + }, + } + + for i, tc := range cases { + c, r := diffTagsAppmesh(tagsFromMapAppmesh(tc.Old), tagsFromMapAppmesh(tc.New)) + cm := tagsToMapAppmesh(c) + rl := []string{} + for _, tagName := range r { + rl = append(rl, *tagName) + } + if !reflect.DeepEqual(cm, tc.Create) { + t.Fatalf("%d: bad create: %#v", i, cm) + } + if !reflect.DeepEqual(rl, tc.Remove) { + t.Fatalf("%d: bad remove: %#v", i, rl) + } + } +} + +func TestIgnoringTagsAppmesh(t *testing.T) { + var ignoredTags []*appmesh.TagRef + ignoredTags = append(ignoredTags, &appmesh.TagRef{ + Key: aws.String("aws:cloudformation:logical-id"), + Value: aws.String("foo"), + }) + ignoredTags = append(ignoredTags, &appmesh.TagRef{ + Key: aws.String("aws:foo:bar"), + Value: aws.String("baz"), + }) + for _, tag := range ignoredTags { + if !tagIgnoredAppmesh(tag) { + t.Fatalf("Tag %v with value %v not ignored, but should be!", *tag.Key, *tag.Value) + } + } +} From 94cf6dff48fdb9fbab503d47f80caf342ee17d0c Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Thu, 28 Mar 2019 19:03:43 +0900 Subject: [PATCH 2/4] Add tags attribute for aws_appmesh_mesh resource --- aws/resource_aws_appmesh_mesh.go | 21 +++++ aws/resource_aws_appmesh_mesh_test.go | 93 +++++++++++++++++++++++ aws/resource_aws_appmesh_test.go | 1 + website/docs/r/appmesh_mesh.html.markdown | 1 + 4 files changed, 116 insertions(+) diff --git a/aws/resource_aws_appmesh_mesh.go b/aws/resource_aws_appmesh_mesh.go index 0ad5679d70af..c9ffd44d8de6 100644 --- a/aws/resource_aws_appmesh_mesh.go +++ b/aws/resource_aws_appmesh_mesh.go @@ -74,6 +74,8 @@ func resourceAwsAppmeshMesh() *schema.Resource { Type: schema.TypeString, Computed: true, }, + + "tags": tagsSchema(), }, } } @@ -85,6 +87,7 @@ func resourceAwsAppmeshMeshCreate(d *schema.ResourceData, meta interface{}) erro req := &appmesh.CreateMeshInput{ MeshName: aws.String(meshName), Spec: expandAppmeshMeshSpec(d.Get("spec").([]interface{})), + Tags: tagsFromMapAppmesh(d.Get("tags").(map[string]interface{})), } log.Printf("[DEBUG] Creating App Mesh service mesh: %#v", req) @@ -127,6 +130,15 @@ func resourceAwsAppmeshMeshRead(d *schema.ResourceData, meta interface{}) error return fmt.Errorf("error setting spec: %s", err) } + if err := saveTagsAppmesh(conn, d, aws.StringValue(resp.Mesh.Metadata.Arn)); err != nil { + if isAWSErr(err, appmesh.ErrCodeNotFoundException, "") { + log.Printf("[WARN] App Mesh service mesh (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + return fmt.Errorf("error saving tags: %s", err) + } + return nil } @@ -147,6 +159,15 @@ func resourceAwsAppmeshMeshUpdate(d *schema.ResourceData, meta interface{}) erro } } + if err := setTagsAppmesh(conn, d, d.Get("arn").(string)); err != nil { + if isAWSErr(err, appmesh.ErrCodeNotFoundException, "") { + log.Printf("[WARN] App Mesh service mesh (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + return fmt.Errorf("error setting tags: %s", err) + } + return resourceAwsAppmeshMeshRead(d, meta) } diff --git a/aws/resource_aws_appmesh_mesh_test.go b/aws/resource_aws_appmesh_mesh_test.go index 1dfa25467e75..20e13093ff89 100644 --- a/aws/resource_aws_appmesh_mesh_test.go +++ b/aws/resource_aws_appmesh_mesh_test.go @@ -131,6 +131,60 @@ func testAccAwsAppmeshMesh_egressFilter(t *testing.T) { }) } +func testAccAwsAppmeshMesh_tags(t *testing.T) { + var mesh appmesh.MeshData + resourceName := "aws_appmesh_mesh.foo" + rName := fmt.Sprintf("tf-test-%d", acctest.RandInt()) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAppmeshMeshDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAppmeshMeshConfigWithTags(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAppmeshMeshExists( + resourceName, &mesh), + resource.TestCheckResourceAttr( + resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr( + resourceName, "tags.foo", "bar"), + resource.TestCheckResourceAttr( + resourceName, "tags.good", "bad"), + ), + }, + { + Config: testAccAppmeshMeshConfigWithUpdateTags(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAppmeshMeshExists( + resourceName, &mesh), + resource.TestCheckResourceAttr( + resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr( + resourceName, "tags.good", "bad2"), + resource.TestCheckResourceAttr( + resourceName, "tags.fizz", "buzz"), + ), + }, + { + Config: testAccAppmeshMeshConfigWithRemoveTags(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAppmeshMeshExists( + resourceName, &mesh), + resource.TestCheckResourceAttr( + resourceName, "tags.%", "1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func testAccCheckAppmeshMeshDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).appmeshconn @@ -200,3 +254,42 @@ resource "aws_appmesh_mesh" "foo" { } `, name, egressFilterType) } + +func testAccAppmeshMeshConfigWithTags(name string) string { + return fmt.Sprintf(` +resource "aws_appmesh_mesh" "foo" { + name = "%s" + + tags = { + foo = "bar" + good = "bad" + } +} +`, name) +} + +func testAccAppmeshMeshConfigWithUpdateTags(name string) string { + return fmt.Sprintf(` +resource "aws_appmesh_mesh" "foo" { + name = "%s" + + tags = { + foo = "bar" + good = "bad2" + fizz = "buzz" + } +} +`, name) +} + +func testAccAppmeshMeshConfigWithRemoveTags(name string) string { + return fmt.Sprintf(` +resource "aws_appmesh_mesh" "foo" { + name = "%s" + + tags = { + foo = "bar" + } +} +`, name) +} diff --git a/aws/resource_aws_appmesh_test.go b/aws/resource_aws_appmesh_test.go index 2fa38fb04cca..ccc4769570b6 100644 --- a/aws/resource_aws_appmesh_test.go +++ b/aws/resource_aws_appmesh_test.go @@ -9,6 +9,7 @@ func TestAccAWSAppmesh(t *testing.T) { "Mesh": { "basic": testAccAwsAppmeshMesh_basic, "egressFilter": testAccAwsAppmeshMesh_egressFilter, + "tags": testAccAwsAppmeshMesh_tags, }, "Route": { "httpRoute": testAccAwsAppmeshRoute_httpRoute, diff --git a/website/docs/r/appmesh_mesh.html.markdown b/website/docs/r/appmesh_mesh.html.markdown index 0d9e39d5c9ba..24545d6a11ff 100644 --- a/website/docs/r/appmesh_mesh.html.markdown +++ b/website/docs/r/appmesh_mesh.html.markdown @@ -40,6 +40,7 @@ The following arguments are supported: * `name` - (Required) The name to use for the service mesh. * `spec` - (Optional) The service mesh specification to apply. +* `tags` - (Optional) A mapping of tags to assign to the resource. The `spec` object supports the following: From 0aa114d1213a55ae3631da03953860fe478548fa Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Mon, 22 Apr 2019 13:12:44 +0900 Subject: [PATCH 3/4] fix error handling --- aws/resource_aws_appmesh_mesh.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/aws/resource_aws_appmesh_mesh.go b/aws/resource_aws_appmesh_mesh.go index c9ffd44d8de6..7ab40e45c40e 100644 --- a/aws/resource_aws_appmesh_mesh.go +++ b/aws/resource_aws_appmesh_mesh.go @@ -107,7 +107,7 @@ func resourceAwsAppmeshMeshRead(d *schema.ResourceData, meta interface{}) error resp, err := conn.DescribeMesh(&appmesh.DescribeMeshInput{ MeshName: aws.String(d.Id()), }) - if isAWSErr(err, "NotFoundException", "") { + if isAWSErr(err, appmesh.ErrCodeNotFoundException, "") { log.Printf("[WARN] App Mesh service mesh (%s) not found, removing from state", d.Id()) d.SetId("") return nil @@ -130,12 +130,13 @@ func resourceAwsAppmeshMeshRead(d *schema.ResourceData, meta interface{}) error return fmt.Errorf("error setting spec: %s", err) } - if err := saveTagsAppmesh(conn, d, aws.StringValue(resp.Mesh.Metadata.Arn)); err != nil { - if isAWSErr(err, appmesh.ErrCodeNotFoundException, "") { - log.Printf("[WARN] App Mesh service mesh (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } + err = saveTagsAppmesh(conn, d, aws.StringValue(resp.Mesh.Metadata.Arn)) + if isAWSErr(err, appmesh.ErrCodeNotFoundException, "") { + log.Printf("[WARN] App Mesh service mesh (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { return fmt.Errorf("error saving tags: %s", err) } @@ -159,12 +160,13 @@ func resourceAwsAppmeshMeshUpdate(d *schema.ResourceData, meta interface{}) erro } } - if err := setTagsAppmesh(conn, d, d.Get("arn").(string)); err != nil { - if isAWSErr(err, appmesh.ErrCodeNotFoundException, "") { - log.Printf("[WARN] App Mesh service mesh (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } + err := setTagsAppmesh(conn, d, d.Get("arn").(string)) + if isAWSErr(err, appmesh.ErrCodeNotFoundException, "") { + log.Printf("[WARN] App Mesh service mesh (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { return fmt.Errorf("error setting tags: %s", err) } @@ -178,7 +180,7 @@ func resourceAwsAppmeshMeshDelete(d *schema.ResourceData, meta interface{}) erro _, err := conn.DeleteMesh(&appmesh.DeleteMeshInput{ MeshName: aws.String(d.Id()), }) - if isAWSErr(err, "NotFoundException", "") { + if isAWSErr(err, appmesh.ErrCodeNotFoundException, "") { return nil } if err != nil { From 0aa99f7bc52d755244ae5056e853cc5057427149 Mon Sep 17 00:00:00 2001 From: kterada0509 Date: Fri, 28 Jun 2019 07:32:43 +0900 Subject: [PATCH 4/4] refactor resource tag for appmesh --- aws/tagsAppmesh.go | 19 ++++++++++-------- aws/tagsAppmesh_test.go | 44 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/aws/tagsAppmesh.go b/aws/tagsAppmesh.go index 2042b1beb3e9..cd15db36b7b9 100644 --- a/aws/tagsAppmesh.go +++ b/aws/tagsAppmesh.go @@ -53,15 +53,18 @@ func diffTagsAppmesh(oldTags, newTags []*appmesh.TagRef) ([]*appmesh.TagRef, []* // First, we're creating everything we have create := make(map[string]interface{}) for _, t := range newTags { - create[*t.Key] = *t.Value + create[aws.StringValue(t.Key)] = aws.StringValue(t.Value) } // Build the list of what to remove var remove []*string for _, t := range oldTags { - if _, ok := create[*t.Key]; !ok { - // Delete it! + 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)) } } @@ -89,7 +92,7 @@ func tagsToMapAppmesh(ts []*appmesh.TagRef) map[string]string { result := make(map[string]string) for _, t := range ts { if !tagIgnoredAppmesh(t) { - result[*t.Key] = *t.Value + result[aws.StringValue(t.Key)] = aws.StringValue(t.Value) } } @@ -115,12 +118,12 @@ func saveTagsAppmesh(conn *appmesh.AppMesh, d *schema.ResourceData, arn string) // compare a tag against a list of strings and checks if it should // be ignored or not func tagIgnoredAppmesh(t *appmesh.TagRef) bool { - filter := []string{"^aws:", "^appmesh:", "Name"} + filter := []string{"^aws:"} for _, v := range filter { - log.Printf("[DEBUG] Matching %v with %v\n", v, *t.Key) - r, _ := regexp.MatchString(v, *t.Key) + log.Printf("[DEBUG] Matching %v with %v\n", v, aws.StringValue(t.Key)) + r, _ := regexp.MatchString(v, aws.StringValue(t.Key)) if r { - log.Printf("[DEBUG] Found AWS specific tag %s (val: %s), ignoring.\n", *t.Key, *t.Value) + log.Printf("[DEBUG] Found AWS specific tag %s (val: %s), ignoring.\n", aws.StringValue(t.Key), aws.StringValue(t.Value)) return true } } diff --git a/aws/tagsAppmesh_test.go b/aws/tagsAppmesh_test.go index 84a80ea943ea..398ceccd821e 100644 --- a/aws/tagsAppmesh_test.go +++ b/aws/tagsAppmesh_test.go @@ -14,20 +14,19 @@ func TestDiffAppmeshTags(t *testing.T) { Create map[string]string Remove []string }{ - // Basic add/remove + // Add { Old: map[string]interface{}{ "foo": "bar", }, New: map[string]interface{}{ + "foo": "bar", "bar": "baz", }, Create: map[string]string{ "bar": "baz", }, - Remove: []string{ - "foo", - }, + Remove: map[string]string{}, }, // Modify @@ -41,7 +40,42 @@ func TestDiffAppmeshTags(t *testing.T) { Create: map[string]string{ "foo": "baz", }, - Remove: []string{}, + 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", + }, }, }