From b5256fc415f91a7222853d128b18040f2fd29873 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 1 Oct 2019 10:46:12 -0400 Subject: [PATCH] service/ecr: Refactor to use keyvaluetags library Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/7926 Output from acceptance testing: ``` --- PASS: TestAccAWSEcrDataSource_ecrImage (13.77s) --- PASS: TestAccAWSEcrRepository_basic (14.42s) --- PASS: TestAccAWSEcrRepository_immutability (14.53s) --- PASS: TestAccAWSEcrDataSource_ecrRepository (16.21s) --- PASS: TestAccAWSEcrRepository_tags (20.90s) ``` --- aws/data_source_aws_ecr_repository.go | 16 ++- aws/resource_aws_ecr_repository.go | 25 +++-- aws/tagsECR.go | 135 -------------------------- aws/tagsECR_test.go | 111 --------------------- 4 files changed, 30 insertions(+), 257 deletions(-) delete mode 100644 aws/tagsECR.go delete mode 100644 aws/tagsECR_test.go diff --git a/aws/data_source_aws_ecr_repository.go b/aws/data_source_aws_ecr_repository.go index 8b93434ac15..7ebb1fab4fc 100644 --- a/aws/data_source_aws_ecr_repository.go +++ b/aws/data_source_aws_ecr_repository.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecr" "github.com/hashicorp/terraform/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func dataSourceAwsEcrRepository() *schema.Resource { @@ -54,17 +55,22 @@ func dataSourceAwsEcrRepositoryRead(d *schema.ResourceData, meta interface{}) er } repository := out.Repositories[0] - - log.Printf("[DEBUG] Received ECR repository %s", out) + arn := aws.StringValue(repository.RepositoryArn) d.SetId(aws.StringValue(repository.RepositoryName)) - d.Set("arn", repository.RepositoryArn) + d.Set("arn", arn) d.Set("registry_id", repository.RegistryId) d.Set("name", repository.RepositoryName) d.Set("repository_url", repository.RepositoryUri) - if err := getTagsECR(conn, d); err != nil { - return fmt.Errorf("error getting ECR repository tags: %s", err) + tags, err := keyvaluetags.EcrListTags(conn, arn) + + if err != nil { + return fmt.Errorf("error listing tags for ECR Repository (%s): %s", arn, err) + } + + if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) } return nil diff --git a/aws/resource_aws_ecr_repository.go b/aws/resource_aws_ecr_repository.go index 571b7d627d8..e52e24e66d3 100644 --- a/aws/resource_aws_ecr_repository.go +++ b/aws/resource_aws_ecr_repository.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsEcrRepository() *schema.Resource { @@ -65,7 +66,7 @@ func resourceAwsEcrRepositoryCreate(d *schema.ResourceData, meta interface{}) er input := ecr.CreateRepositoryInput{ ImageTagMutability: aws.String(d.Get("image_tag_mutability").(string)), RepositoryName: aws.String(d.Get("name").(string)), - Tags: tagsFromMapECR(d.Get("tags").(map[string]interface{})), + Tags: keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().EcrTags(), } log.Printf("[DEBUG] Creating ECR repository: %#v", input) @@ -119,21 +120,29 @@ func resourceAwsEcrRepositoryRead(d *schema.ResourceData, meta interface{}) erro } repository := out.Repositories[0] + arn := aws.StringValue(repository.RepositoryArn) - d.Set("arn", repository.RepositoryArn) + d.Set("arn", arn) d.Set("name", repository.RepositoryName) d.Set("registry_id", repository.RegistryId) d.Set("repository_url", repository.RepositoryUri) d.Set("image_tag_mutability", repository.ImageTagMutability) - if err := getTagsECR(conn, d); err != nil { - return fmt.Errorf("error getting ECR repository tags: %s", err) + tags, err := keyvaluetags.EcrListTags(conn, arn) + + if err != nil { + return fmt.Errorf("error listing tags for ECR Repository (%s): %s", arn, err) + } + + if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) } return nil } func resourceAwsEcrRepositoryUpdate(d *schema.ResourceData, meta interface{}) error { + arn := d.Get("arn").(string) conn := meta.(*AWSClient).ecrconn if d.HasChange("image_tag_mutability") { @@ -142,8 +151,12 @@ func resourceAwsEcrRepositoryUpdate(d *schema.ResourceData, meta interface{}) er } } - if err := setTagsECR(conn, d); err != nil { - return fmt.Errorf("error setting ECR repository tags: %s", err) + if d.HasChange("tags") { + o, n := d.GetChange("tags") + + if err := keyvaluetags.EcrUpdateTags(conn, arn, o, n); err != nil { + return fmt.Errorf("error updating ECR Repository (%s) tags: %s", arn, err) + } } return resourceAwsEcrRepositoryRead(d, meta) diff --git a/aws/tagsECR.go b/aws/tagsECR.go deleted file mode 100644 index 4bedcccb64b..00000000000 --- a/aws/tagsECR.go +++ /dev/null @@ -1,135 +0,0 @@ -package aws - -import ( - "log" - "regexp" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ecr" - "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 getTagsECR(conn *ecr.ECR, d *schema.ResourceData) error { - resp, err := conn.ListTagsForResource(&ecr.ListTagsForResourceInput{ - ResourceArn: aws.String(d.Get("arn").(string)), - }) - if err != nil { - return err - } - - if err := d.Set("tags", tagsToMapECR(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 setTagsECR(conn *ecr.ECR, 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 := diffTagsECR(tagsFromMapECR(o), tagsFromMapECR(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(&ecr.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(&ecr.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 diffTagsECR(oldTags, newTags []*ecr.Tag) ([]*ecr.Tag, []*ecr.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 []*ecr.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 tagsFromMapECR(create), remove -} - -// tagsFromMap returns the tags for the given map of data. -func tagsFromMapECR(m map[string]interface{}) []*ecr.Tag { - result := make([]*ecr.Tag, 0, len(m)) - for k, v := range m { - t := &ecr.Tag{ - Key: aws.String(k), - Value: aws.String(v.(string)), - } - if !tagIgnoredECR(t) { - result = append(result, t) - } - } - - return result -} - -// tagsToMap turns the list of tags into a map. -func tagsToMapECR(ts []*ecr.Tag) map[string]string { - result := make(map[string]string) - for _, t := range ts { - if !tagIgnoredECR(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 tagIgnoredECR(t *ecr.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/tagsECR_test.go b/aws/tagsECR_test.go deleted file mode 100644 index 3cba90bf5be..00000000000 --- a/aws/tagsECR_test.go +++ /dev/null @@ -1,111 +0,0 @@ -package aws - -import ( - "reflect" - "testing" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ecr" -) - -// go test -v -run="TestDiffECRTags" -func TestDiffECRTags(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 := diffTagsECR(tagsFromMapECR(tc.Old), tagsFromMapECR(tc.New)) - cm := tagsToMapECR(c) - rm := tagsToMapECR(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="TestIgnoringTagsECR" -func TestIgnoringTagsECR(t *testing.T) { - var ignoredTags []*ecr.Tag - ignoredTags = append(ignoredTags, &ecr.Tag{ - Key: aws.String("aws:cloudformation:logical-id"), - Value: aws.String("foo"), - }) - ignoredTags = append(ignoredTags, &ecr.Tag{ - Key: aws.String("aws:foo:bar"), - Value: aws.String("baz"), - }) - for _, tag := range ignoredTags { - if !tagIgnoredECR(tag) { - t.Fatalf("Tag %v with value %v not ignored, but should be!", *tag.Key, *tag.Value) - } - } -}