From 69ce4872e0af657e19ccfa721aac94e34dea49b5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 18 Dec 2018 15:18:12 -0500 Subject: [PATCH 1/2] Amazon ECR Repository tags. --- aws/data_source_aws_ecr_repository.go | 16 ++- aws/data_source_aws_ecr_repository_test.go | 26 ++-- aws/resource_aws_ecr_repository.go | 51 ++++---- aws/resource_aws_ecr_repository_test.go | 58 ++++++++- aws/tagsECR.go | 133 ++++++++++++++++++++ aws/tagsECR_test.go | 79 ++++++++++++ website/docs/d/ecr_repository.html.markdown | 1 + website/docs/r/ecr_repository.html.markdown | 1 + 8 files changed, 329 insertions(+), 36 deletions(-) create mode 100644 aws/tagsECR.go create 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 e2d25a586aa1..8b93434ac154 100644 --- a/aws/data_source_aws_ecr_repository.go +++ b/aws/data_source_aws_ecr_repository.go @@ -1,10 +1,10 @@ package aws import ( + "fmt" "log" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ecr" "github.com/hashicorp/terraform/helper/schema" ) @@ -31,6 +31,7 @@ func dataSourceAwsEcrRepository() *schema.Resource { Type: schema.TypeString, Computed: true, }, + "tags": tagsSchemaComputed(), }, } } @@ -38,30 +39,33 @@ func dataSourceAwsEcrRepository() *schema.Resource { func dataSourceAwsEcrRepositoryRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ecrconn - repositoryName := d.Get("name").(string) params := &ecr.DescribeRepositoriesInput{ - RepositoryNames: []*string{aws.String(repositoryName)}, + RepositoryNames: aws.StringSlice([]string{d.Get("name").(string)}), } log.Printf("[DEBUG] Reading ECR repository: %s", params) out, err := conn.DescribeRepositories(params) if err != nil { - if ecrerr, ok := err.(awserr.Error); ok && ecrerr.Code() == "RepositoryNotFoundException" { + if isAWSErr(err, ecr.ErrCodeRepositoryNotFoundException, "") { log.Printf("[WARN] ECR Repository %s not found, removing from state", d.Id()) d.SetId("") return nil } - return err + return fmt.Errorf("error reading ECR repository: %s", err) } repository := out.Repositories[0] log.Printf("[DEBUG] Received ECR repository %s", out) - d.SetId(*repository.RepositoryName) + d.SetId(aws.StringValue(repository.RepositoryName)) d.Set("arn", repository.RepositoryArn) 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) + } + return nil } diff --git a/aws/data_source_aws_ecr_repository_test.go b/aws/data_source_aws_ecr_repository_test.go index 741058149ca8..e862a02f5d64 100644 --- a/aws/data_source_aws_ecr_repository_test.go +++ b/aws/data_source_aws_ecr_repository_test.go @@ -10,28 +10,40 @@ import ( ) func TestAccAWSEcrDataSource_ecrRepository(t *testing.T) { + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "data.aws_ecr_repository.default" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccCheckAwsEcrRepositoryDataSourceConfig, + Config: testAccCheckAwsEcrRepositoryDataSourceConfig(rName), Check: resource.ComposeTestCheckFunc( - resource.TestMatchResourceAttr("data.aws_ecr_repository.default", "arn", regexp.MustCompile(`^arn:aws:ecr:[a-zA-Z]+-[a-zA-Z]+-\d+:\d+:repository/foo-repository-terraform-\d+$`)), - resource.TestCheckResourceAttrSet("data.aws_ecr_repository.default", "registry_id"), - resource.TestMatchResourceAttr("data.aws_ecr_repository.default", "repository_url", regexp.MustCompile(`^\d+\.dkr\.ecr\.[a-zA-Z]+-[a-zA-Z]+-\d+\.amazonaws\.com/foo-repository-terraform-\d+$`)), + resource.TestMatchResourceAttr(resourceName, "arn", regexp.MustCompile(`^arn:aws:ecr:[a-zA-Z]+-[a-zA-Z]+-\d+:\d+:repository/tf-acc-test-\d+$`)), + resource.TestCheckResourceAttrSet(resourceName, "registry_id"), + resource.TestMatchResourceAttr(resourceName, "repository_url", regexp.MustCompile(`^\d+\.dkr\.ecr\.[a-zA-Z]+-[a-zA-Z]+-\d+\.amazonaws\.com/tf-acc-test-\d+$`)), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.Usage", "original"), ), }, }, }) } -var testAccCheckAwsEcrRepositoryDataSourceConfig = fmt.Sprintf(` +func testAccCheckAwsEcrRepositoryDataSourceConfig(rName string) string { + return fmt.Sprintf(` resource "aws_ecr_repository" "default" { - name = "foo-repository-terraform-%d" + name = %q + + tags = { + Environment = "production" + Usage = "original" + } } data "aws_ecr_repository" "default" { name = "${aws_ecr_repository.default.name}" } -`, acctest.RandInt()) +`, rName) +} diff --git a/aws/resource_aws_ecr_repository.go b/aws/resource_aws_ecr_repository.go index 8b540432b093..7fa058002ec6 100644 --- a/aws/resource_aws_ecr_repository.go +++ b/aws/resource_aws_ecr_repository.go @@ -7,7 +7,6 @@ import ( "fmt" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ecr" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" @@ -17,6 +16,7 @@ func resourceAwsEcrRepository() *schema.Resource { return &schema.Resource{ Create: resourceAwsEcrRepositoryCreate, Read: resourceAwsEcrRepositoryRead, + Update: resourceAwsEcrRepositoryUpdate, Delete: resourceAwsEcrRepositoryDelete, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, @@ -32,6 +32,7 @@ func resourceAwsEcrRepository() *schema.Resource { Required: true, ForceNew: true, }, + "tags": tagsSchema(), "arn": { Type: schema.TypeString, Computed: true, @@ -55,30 +56,29 @@ func resourceAwsEcrRepositoryCreate(d *schema.ResourceData, meta interface{}) er RepositoryName: aws.String(d.Get("name").(string)), } - log.Printf("[DEBUG] Creating ECR resository: %s", input) + log.Printf("[DEBUG] Creating ECR repository: %#v", input) out, err := conn.CreateRepository(&input) if err != nil { - return err + return fmt.Errorf("error creating ECR repository: %s", err) } repository := *out.Repository log.Printf("[DEBUG] ECR repository created: %q", *repository.RepositoryArn) - d.SetId(*repository.RepositoryName) + d.SetId(aws.StringValue(repository.RepositoryName)) + // ARN required for setting any tags. d.Set("arn", repository.RepositoryArn) - d.Set("registry_id", repository.RegistryId) - - return resourceAwsEcrRepositoryRead(d, meta) + return resourceAwsEcrRepositoryUpdate(d, meta) } func resourceAwsEcrRepositoryRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ecrconn - log.Printf("[DEBUG] Reading repository %s", d.Id()) + log.Printf("[DEBUG] Reading ECR repository %s", d.Id()) var out *ecr.DescribeRepositoriesOutput input := &ecr.DescribeRepositoriesInput{ - RepositoryNames: []*string{aws.String(d.Id())}, + RepositoryNames: aws.StringSlice([]string{d.Id()}), } err := resource.Retry(1*time.Minute, func() *resource.RetryError { @@ -100,7 +100,7 @@ func resourceAwsEcrRepositoryRead(d *schema.ResourceData, meta interface{}) erro } if err != nil { - return err + return fmt.Errorf("error reading ECR repository: %s", err) } repository := out.Repositories[0] @@ -110,9 +110,23 @@ func resourceAwsEcrRepositoryRead(d *schema.ResourceData, meta interface{}) erro d.Set("registry_id", repository.RegistryId) d.Set("repository_url", repository.RepositoryUri) + if err := getTagsECR(conn, d); err != nil { + return fmt.Errorf("error getting ECR repository tags: %s", err) + } + return nil } +func resourceAwsEcrRepositoryUpdate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).ecrconn + + if err := setTagsECR(conn, d); err != nil { + return fmt.Errorf("error setting ECR repository tags: %s", err) + } + + return resourceAwsEcrRepositoryRead(d, meta) +} + func resourceAwsEcrRepositoryDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ecrconn @@ -122,28 +136,21 @@ func resourceAwsEcrRepositoryDelete(d *schema.ResourceData, meta interface{}) er Force: aws.Bool(true), }) if err != nil { - if ecrerr, ok := err.(awserr.Error); ok && ecrerr.Code() == "RepositoryNotFoundException" { + if isAWSErr(err, ecr.ErrCodeRepositoryNotFoundException, "") { return nil } - return err + return fmt.Errorf("error deleting ECR repository: %s", err) } log.Printf("[DEBUG] Waiting for ECR Repository %q to be deleted", d.Id()) err = resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError { _, err := conn.DescribeRepositories(&ecr.DescribeRepositoriesInput{ - RepositoryNames: []*string{aws.String(d.Id())}, + RepositoryNames: aws.StringSlice([]string{d.Id()}), }) - if err != nil { - awsErr, ok := err.(awserr.Error) - if !ok { - return resource.NonRetryableError(err) - } - - if awsErr.Code() == "RepositoryNotFoundException" { + if isAWSErr(err, ecr.ErrCodeRepositoryNotFoundException, "") { return nil } - return resource.NonRetryableError(err) } @@ -151,7 +158,7 @@ func resourceAwsEcrRepositoryDelete(d *schema.ResourceData, meta interface{}) er fmt.Errorf("%q: Timeout while waiting for the ECR Repository to be deleted", d.Id())) }) if err != nil { - return err + return fmt.Errorf("error deleting ECR repository: %s", err) } log.Printf("[DEBUG] repository %q deleted.", d.Get("name").(string)) diff --git a/aws/resource_aws_ecr_repository_test.go b/aws/resource_aws_ecr_repository_test.go index 85dbfa2a616f..b4cdda20c5a4 100644 --- a/aws/resource_aws_ecr_repository_test.go +++ b/aws/resource_aws_ecr_repository_test.go @@ -39,6 +39,37 @@ func TestAccAWSEcrRepository_basic(t *testing.T) { }) } +func TestAccAWSEcrRepository_tags(t *testing.T) { + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_ecr_repository.default" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSEcrRepositoryDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSEcrRepositoryConfig_tags(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEcrRepositoryExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.Usage", "original"), + ), + }, + { + Config: testAccAWSEcrRepositoryConfig_tagsChanged(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEcrRepositoryExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.Usage", "changed"), + ), + }, + }, + }) +} + func testAccCheckAWSEcrRepositoryDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).ecrconn @@ -99,7 +130,32 @@ func testAccCheckAWSEcrRepositoryRepositoryURL(resourceName, repositoryName stri func testAccAWSEcrRepositoryConfig(rName string) string { return fmt.Sprintf(` resource "aws_ecr_repository" "default" { - name = %q + name = %q +} +`, rName) +} + +func testAccAWSEcrRepositoryConfig_tags(rName string) string { + return fmt.Sprintf(` +resource "aws_ecr_repository" "default" { + name = %q + + tags = { + Environment = "production" + Usage = "original" + } +} +`, rName) +} + +func testAccAWSEcrRepositoryConfig_tagsChanged(rName string) string { + return fmt.Sprintf(` +resource "aws_ecr_repository" "default" { + name = %q + + tags = { + Usage = "changed" + } } `, rName) } diff --git a/aws/tagsECR.go b/aws/tagsECR.go new file mode 100644 index 000000000000..a551f594a340 --- /dev/null +++ b/aws/tagsECR.go @@ -0,0 +1,133 @@ +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) { + // Delete it! + remove = append(remove, t) + } + } + + 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 new file mode 100644 index 000000000000..2c92bdaa4f69 --- /dev/null +++ b/aws/tagsECR_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/ecr" +) + +// go test -v -run="TestDiffECRTags" +func TestDiffECRTags(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 := 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) + } + } +} diff --git a/website/docs/d/ecr_repository.html.markdown b/website/docs/d/ecr_repository.html.markdown index 54b88f5760dc..37accb55cf1f 100644 --- a/website/docs/d/ecr_repository.html.markdown +++ b/website/docs/d/ecr_repository.html.markdown @@ -31,3 +31,4 @@ In addition to all arguments above, the following attributes are exported: * `arn` - Full ARN of the repository. * `registry_id` - The registry ID where the repository was created. * `repository_url` - The URL of the repository (in the form `aws_account_id.dkr.ecr.region.amazonaws.com/repositoryName`). +* `tags` - A mapping of tags assigned to the resource. diff --git a/website/docs/r/ecr_repository.html.markdown b/website/docs/r/ecr_repository.html.markdown index a9c173a61252..865aab8d5d3c 100644 --- a/website/docs/r/ecr_repository.html.markdown +++ b/website/docs/r/ecr_repository.html.markdown @@ -27,6 +27,7 @@ resource "aws_ecr_repository" "foo" { The following arguments are supported: * `name` - (Required) Name of the repository. +* `tags` - (Optional) A mapping of tags to assign to the resource. ## Attributes Reference From a49d29f374b50a19ff0477dee09b9d6972d8d04b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 18 Dec 2018 16:02:29 -0500 Subject: [PATCH 2/2] Changes after code review. --- aws/resource_aws_ecr_repository.go | 7 +++++- aws/tagsECR.go | 4 ++- aws/tagsECR_test.go | 40 +++++++++++++++++++++++++++--- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/aws/resource_aws_ecr_repository.go b/aws/resource_aws_ecr_repository.go index 7fa058002ec6..40da39502329 100644 --- a/aws/resource_aws_ecr_repository.go +++ b/aws/resource_aws_ecr_repository.go @@ -69,7 +69,12 @@ func resourceAwsEcrRepositoryCreate(d *schema.ResourceData, meta interface{}) er d.SetId(aws.StringValue(repository.RepositoryName)) // ARN required for setting any tags. d.Set("arn", repository.RepositoryArn) - return resourceAwsEcrRepositoryUpdate(d, meta) + + if err := setTagsECR(conn, d); err != nil { + return fmt.Errorf("error setting ECR repository tags: %s", err) + } + + return resourceAwsEcrRepositoryRead(d, meta) } func resourceAwsEcrRepositoryRead(d *schema.ResourceData, meta interface{}) error { diff --git a/aws/tagsECR.go b/aws/tagsECR.go index a551f594a340..4bedcccb64bd 100644 --- a/aws/tagsECR.go +++ b/aws/tagsECR.go @@ -81,8 +81,10 @@ func diffTagsECR(oldTags, newTags []*ecr.Tag) ([]*ecr.Tag, []*ecr.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)) } } diff --git a/aws/tagsECR_test.go b/aws/tagsECR_test.go index 2c92bdaa4f69..3cba90bf5be1 100644 --- a/aws/tagsECR_test.go +++ b/aws/tagsECR_test.go @@ -14,20 +14,19 @@ func TestDiffECRTags(t *testing.T) { Old, New map[string]interface{} Create, Remove map[string]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: map[string]string{ - "foo": "bar", - }, + Remove: map[string]string{}, }, // Modify @@ -45,6 +44,39 @@ func TestDiffECRTags(t *testing.T) { "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 {