From b689da30e957a850660410a5c3215612a4465970 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 15 Nov 2019 14:38:59 -0500 Subject: [PATCH] service/transfer: Refactor tagging logic to keyvaluetags package (#10739) Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/10688 Output from acceptance testing: ``` --- PASS: TestAccAWSTransferUser_UserName_Validation (5.86s) --- PASS: TestAccAWSTransferServer_disappears (11.20s) --- PASS: TestAccAWSTransferUser_disappears (14.61s) --- PASS: TestAccAWSTransferServer_forcedestroy (15.39s) --- PASS: TestAccAWSTransferUser_basic (16.12s) --- PASS: TestAccAWSTransferServer_apigateway (20.54s) --- PASS: TestAccAWSTransferServer_basic (21.48s) --- PASS: TestAccAWSTransferUser_modifyWithOptions (36.18s) --- PASS: TestAccAWSTransferServer_vpcEndpointId (77.08s) ``` --- aws/resource_aws_transfer_server.go | 13 +-- aws/resource_aws_transfer_user.go | 13 +-- aws/tagsTransfer.go | 118 ---------------------------- aws/tagsTransfer_test.go | 112 -------------------------- 4 files changed, 16 insertions(+), 240 deletions(-) delete mode 100644 aws/tagsTransfer.go delete mode 100644 aws/tagsTransfer_test.go diff --git a/aws/resource_aws_transfer_server.go b/aws/resource_aws_transfer_server.go index f2db50b259d1..eea94fb15711 100644 --- a/aws/resource_aws_transfer_server.go +++ b/aws/resource_aws_transfer_server.go @@ -8,10 +8,10 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/transfer" - "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsTransferServer() *schema.Resource { @@ -110,7 +110,7 @@ func resourceAwsTransferServer() *schema.Resource { func resourceAwsTransferServerCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).transferconn - tags := tagsFromMapTransfer(d.Get("tags").(map[string]interface{})) + tags := keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().TransferTags() createOpts := &transfer.CreateServerInput{} if len(tags) != 0 { @@ -192,7 +192,7 @@ func resourceAwsTransferServerRead(d *schema.ResourceData, meta interface{}) err d.Set("identity_provider_type", resp.Server.IdentityProviderType) d.Set("logging_role", resp.Server.LoggingRole) - if err := d.Set("tags", tagsToMapTransfer(resp.Server.Tags)); err != nil { + if err := d.Set("tags", keyvaluetags.TransferKeyValueTags(resp.Server.Tags).IgnoreAws().Map()); err != nil { return fmt.Errorf("Error setting tags: %s", err) } return nil @@ -249,8 +249,11 @@ func resourceAwsTransferServerUpdate(d *schema.ResourceData, meta interface{}) e } } - if err := setTagsTransfer(conn, d); err != nil { - return fmt.Errorf("Error update tags: %s", err) + if d.HasChange("tags") { + o, n := d.GetChange("tags") + if err := keyvaluetags.TransferUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { + return fmt.Errorf("error updating tags: %s", err) + } } return resourceAwsTransferServerRead(d, meta) diff --git a/aws/resource_aws_transfer_user.go b/aws/resource_aws_transfer_user.go index cd9df67f417d..4747829b4e13 100644 --- a/aws/resource_aws_transfer_user.go +++ b/aws/resource_aws_transfer_user.go @@ -8,10 +8,10 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/transfer" - "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsTransferUser() *schema.Resource { @@ -89,7 +89,7 @@ func resourceAwsTransferUserCreate(d *schema.ResourceData, meta interface{}) err } if attr, ok := d.GetOk("tags"); ok { - createOpts.Tags = tagsFromMapTransfer(attr.(map[string]interface{})) + createOpts.Tags = keyvaluetags.New(attr.(map[string]interface{})).IgnoreAws().TransferTags() } log.Printf("[DEBUG] Create Transfer User Option: %#v", createOpts) @@ -135,7 +135,7 @@ func resourceAwsTransferUserRead(d *schema.ResourceData, meta interface{}) error d.Set("policy", resp.User.Policy) d.Set("role", resp.User.Role) - if err := d.Set("tags", tagsToMapTransfer(resp.User.Tags)); err != nil { + if err := d.Set("tags", keyvaluetags.TransferKeyValueTags(resp.User.Tags).IgnoreAws().Map()); err != nil { return fmt.Errorf("Error setting tags: %s", err) } return nil @@ -181,8 +181,11 @@ func resourceAwsTransferUserUpdate(d *schema.ResourceData, meta interface{}) err } } - if err := setTagsTransfer(conn, d); err != nil { - return fmt.Errorf("Error update tags: %s", err) + if d.HasChange("tags") { + o, n := d.GetChange("tags") + if err := keyvaluetags.TransferUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { + return fmt.Errorf("error updating tags: %s", err) + } } return resourceAwsTransferUserRead(d, meta) diff --git a/aws/tagsTransfer.go b/aws/tagsTransfer.go deleted file mode 100644 index 941e542fc92a..000000000000 --- a/aws/tagsTransfer.go +++ /dev/null @@ -1,118 +0,0 @@ -package aws - -import ( - "log" - "regexp" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/transfer" - "github.com/hashicorp/terraform-plugin-sdk/helper/schema" -) - -// setTags is a helper to set the tags for a resource. It expects the -// tags field to be named "tags" -func setTagsTransfer(conn *transfer.Transfer, 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 := diffTagsTransfer(tagsFromMapTransfer(o), tagsFromMapTransfer(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(&transfer.UntagResourceInput{ - Arn: 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(&transfer.TagResourceInput{ - Arn: 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 diffTagsTransfer(oldTags, newTags []*transfer.Tag) ([]*transfer.Tag, []*transfer.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 []*transfer.Tag - for _, t := range oldTags { - old, ok := create[aws.StringValue(t.Key)] - if !ok || old != aws.StringValue(t.Value) { - // Delete it! - remove = append(remove, t) - } else if ok { - // already present so remove from new - delete(create, aws.StringValue(t.Key)) - } - } - - return tagsFromMapTransfer(create), remove -} - -// tagsFromMap returns the tags for the given map of data. -func tagsFromMapTransfer(m map[string]interface{}) []*transfer.Tag { - result := make([]*transfer.Tag, 0, len(m)) - for k, v := range m { - t := &transfer.Tag{ - Key: aws.String(k), - Value: aws.String(v.(string)), - } - if !tagIgnoredTransfer(t) { - result = append(result, t) - } - } - - return result -} - -// tagsToMap turns the list of tags into a map. -func tagsToMapTransfer(ts []*transfer.Tag) map[string]string { - result := make(map[string]string) - for _, t := range ts { - if !tagIgnoredTransfer(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 tagIgnoredTransfer(t *transfer.Tag) bool { - filter := []string{"^aws:"} - for _, v := range filter { - log.Printf("[DEBUG] Matching %v with %v\n", v, aws.StringValue(t.Key)) - if r, _ := regexp.MatchString(v, aws.StringValue(t.Key)); r { - log.Printf("[DEBUG] Found AWS specific tag %s (val: %s), ignoring.\n", aws.StringValue(t.Key), aws.StringValue(t.Value)) - return true - } - } - return false -} diff --git a/aws/tagsTransfer_test.go b/aws/tagsTransfer_test.go deleted file mode 100644 index abb8cf19638b..000000000000 --- a/aws/tagsTransfer_test.go +++ /dev/null @@ -1,112 +0,0 @@ -package aws - -import ( - "reflect" - "testing" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/transfer" -) - -// go test -v -run="TestDiffTransferTags" -func TestDiffTransferTags(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", - }, - }, - - // 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 := diffTagsTransfer(tagsFromMapTransfer(tc.Old), tagsFromMapTransfer(tc.New)) - cm := tagsToMapTransfer(c) - rm := tagsToMapTransfer(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) - } - } -} - -// go test -v -run="TestIgnoringTagsTransfer" -func TestIgnoringTagsTransfer(t *testing.T) { - var ignoredTags []*transfer.Tag - ignoredTags = append(ignoredTags, &transfer.Tag{ - Key: aws.String("aws:cloudformation:logical-id"), - Value: aws.String("foo"), - }) - ignoredTags = append(ignoredTags, &transfer.Tag{ - Key: aws.String("aws:foo:bar"), - Value: aws.String("baz"), - }) - for _, tag := range ignoredTags { - if !tagIgnoredTransfer(tag) { - t.Fatalf("Tag %v with value %v not ignored, but should be!", *tag.Key, *tag.Value) - } - } -}