diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 146b75d0176..0b2043fa28f 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -25,6 +25,7 @@ ability to merge PRs and respond to issues. - [Documentation Update](#documentation-update) - [Enhancement/Bugfix to a Resource](#enhancementbugfix-to-a-resource) - [Adding Resource Import Support](#adding-resource-import-support) + - [Adding Resource Tagging Support](#adding-resource-tagging-support) - [New Resource](#new-resource) - [New Service](#new-service) - [New Region](#new-region) @@ -213,6 +214,211 @@ In addition to the below checklist and the items noted in the Extending Terrafor - [ ] _Resource Acceptance Testing Implementation_: In the resource acceptance testing (e.g. `aws/resource_aws_service_thing_test.go`), implementation of `TestStep`s with `ImportState: true` - [ ] _Resource Documentation Implementation_: In the resource documentation (e.g. `website/docs/r/service_thing.html.markdown`), addition of `Import` documentation section at the bottom of the page +#### Adding Resource Tagging Support + +AWS provides key-value metadata across many services and resources, which can be used for a variety of use cases including billing, ownership, and more. See the [AWS Tagging Strategy page](https://aws.amazon.com/answers/account-management/aws-tagging-strategies/) for more information about tagging at a high level. + +Implementing tagging support for Terraform AWS Provider resources requires the following, each with its own section below: + +- [ ] _Generated Service Tagging Code_: In the internal code generators (e.g. `aws/internal/keyvaluetags`), implementation and customization of how a service handles tagging, which is standardized for the resources. +- [ ] _Resource Tagging Code Implementation_: In the resource code (e.g. `aws/resource_aws_service_thing.go`), implementation of `tags` schema attribute, along with handling in `Create`, `Read`, and `Update` functions. +- [ ] _Resource Tagging Acceptance Testing Implementation_: In the resource acceptance testing (e.g. `aws/resource_aws_service_thing_test.go`), implementation of new acceptance test function and configurations to exercise new tagging logic. +- [ ] _Resource Tagging Documentation Implementation_: In the resource documentation (e.g. `website/docs/r/service_thing.html.markdown`), addition of `tags` argument + +See also a [full example pull request for implementing EKS tagging](https://github.com/terraform-providers/terraform-provider-aws/pull/10307). + +##### Adding Service to Tag Generating Code + +This step is only necessary for the first implementation and may have been previously completed. If so, move on to the next section. + +More details about this code generation, including fixes for potential error messages in this process, can be found in the [keyvaluetags documentation](aws/internal/keyvaluetags/README.md). + +- Open the AWS Go SDK documentation for the service, e.g. for [`service/eks`](https://docs.aws.amazon.com/sdk-for-go/api/service/eks/). Note: there can be a delay between the AWS announcement and the updated AWS Go SDK documentation. +- Determine the "type" of tagging implementation. Some services will use a simple map style (`map[string]*string` in Go) while others will have a separate structure shape (`[]service.Tag` struct with `Key` and `Value` fields). + + - If the type is a map, add the AWS Go SDK service name (e.g. `eks`) to `mapServiceNames` in `aws/internal/keyvaluetags/generators/servicetags/main.go` + - Otherwise, if the type is a struct, add the AWS Go SDK service name (e.g. `eks`) to `sliceServiceNames` in `aws/internal/keyvaluetags/generators/servicetags/main.go`. If the struct name is not exactly `Tag`, it can be customized via the `ServiceTagType` function. If the struct key field is not exactly `Key`, it can be customized via the `ServiceTagTypeKeyField` function. If the struct value field is not exactly `Value`, it can be customized via the `ServiceTagTypeValueField` function. + +- Determine if the service API includes functionality for listing tags (usually a `ListTags` or `ListTagsForResource` API call) or updating tags (usually `TagResource` and `UntagResource` API calls). If so, add the AWS Go SDK service client information to `ServiceClientType` (along with the new required import) in `aws/internal/keyvaluetags/service_generation_customizations.go`, e.g. for EKS: + + ```go + case "eks": + funcType = reflect.TypeOf(eks.New) + ``` + + - If the service API includes functionality for listing tags, add the AWS Go SDK service name (e.g. `eks`) to `serviceNames` in `aws/internal/keyvaluetags/generators/listtags/main.go`. + - If the service API includes functionality for updating tags, add the AWS Go SDK service name (e.g. `eks`) to `serviceNames` in `aws/internal/keyvaluetags/generators/updatetags/main.go`. + +- Run `make gen` (`go generate ./...`) and ensure there are no errors via `make test` (`go test ./...`) + +##### Resource Tagging Code Implementation + +- In the resource Go file (e.g. `aws/resource_aws_eks_cluster.go`), add the following Go import: `"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"` +- In the resource schema, add `"tags": tagsSchema(),` +- If the API supports tagging on creation (the `Input` struct accepts a `Tags` field), in the resource `Create` function, implement the logic to convert the configuration tags into the service tags, e.g. with EKS Clusters: + + ```go + input := &eks.CreateClusterInput{ + /* ... other configuration ... */ + Tags: keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().EksTags(), + } + ``` + + If the service API does not allow passing an empty list, the logic can be adjusted similar to: + + ```go + input := &eks.CreateClusterInput{ + /* ... other configuration ... */ + } + + if v := d.Get("tags").(map[string]interface{}); len(v) > 0 { + input.Tags = keyvaluetags.New(v).IgnoreAws().EksTags() + } + ``` + +- Otherwise if the API does not support tagging on creation (the `Input` struct does not accept a `Tags` field), in the resource `Create` function, implement the logic to convert the configuration tags into the service API call to tag a resource, e.g. with CloudHSM v2 Clusters: + + ```go + if v := d.Get("tags").(map[string]interface{}); len(v) > 0 { + if err := keyvaluetags.Cloudhsmv2UpdateTags(conn, d.Id(), nil, v); err != nil { + return fmt.Errorf("error adding CloudHSM v2 Cluster (%s) tags: %s", d.Id(), err) + } + } + ``` + +- In the resource `Read` function, implement the logic to convert the service tags to save them into the Terraform state for drift detection, e.g. with EKS Clusters (which had the tags available in the DescribeCluster API call): + + ```go + if err := d.Set("tags", keyvaluetags.EksKeyValueTags(cluster.Tags).IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + ``` + + If the service API does not return the tags directly from reading the resource and requires a separate API call, its possible to use the `keyvaluetags` functionality like the following, e.g. with Athena Workgroups: + + ```go + tags, err := keyvaluetags.AthenaListTags(conn, arn.String()) + + if err != nil { + return fmt.Errorf("error listing tags for resource (%s): %s", arn, err) + } + + if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + ``` + +- In the resource `Update` function (this may be the first functionality requiring the creation of the `Update` function), implement the logic to handle tagging updates, e.g. with EKS Clusters: + + ```go + if d.HasChange("tags") { + o, n := d.GetChange("tags") + if err := keyvaluetags.EksUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { + return fmt.Errorf("error updating tags: %s", err) + } + } + ``` + +##### Resource Tagging Acceptance Testing Implementation + +- In the resource testing (e.g. `aws/resource_aws_eks_cluster_test.go`), verify that existing resources without tagging are unaffected and do not have tags saved into their Terraform state. This should be done in the `_basic` acceptance test by adding a line similar to `resource.TestCheckResourceAttr(resourceName, "tags.%s", "0"),` +- In the resource testing, implement a new test named `_Tags` with associated configurations, that verifies creating the resource with tags and updating tags. e.g. EKS Clusters: + + ```go + func TestAccAWSEksCluster_Tags(t *testing.T) { + var cluster1, cluster2, cluster3 eks.Cluster + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_eks_cluster.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSEks(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSEksClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSEksClusterConfigTags1(rName, "key1", "value1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEksClusterExists(resourceName, &cluster1), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSEksClusterConfigTags2(rName, "key1", "value1updated", "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEksClusterExists(resourceName, &cluster2), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + Config: testAccAWSEksClusterConfigTags1(rName, "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEksClusterExists(resourceName, &cluster3), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + }, + }) + } + + func testAccAWSEksClusterConfigTags1(rName, tagKey1, tagValue1 string) string { + return testAccAWSEksClusterConfig_Base(rName) + fmt.Sprintf(` + resource "aws_eks_cluster" "test" { + name = %[1]q + role_arn = "${aws_iam_role.test.arn}" + + tags = { + %[2]q = %[3]q + } + + vpc_config { + subnet_ids = ["${aws_subnet.test.*.id[0]}", "${aws_subnet.test.*.id[1]}"] + } + + depends_on = ["aws_iam_role_policy_attachment.test-AmazonEKSClusterPolicy", "aws_iam_role_policy_attachment.test-AmazonEKSServicePolicy"] + } + `, rName, tagKey1, tagValue1) + } + + func testAccAWSEksClusterConfigTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return testAccAWSEksClusterConfig_Base(rName) + fmt.Sprintf(` + resource "aws_eks_cluster" "test" { + name = %[1]q + role_arn = "${aws_iam_role.test.arn}" + + tags = { + %[2]q = %[3]q + %[4]q = %[5]q + } + + vpc_config { + subnet_ids = ["${aws_subnet.test.*.id[0]}", "${aws_subnet.test.*.id[1]}"] + } + + depends_on = ["aws_iam_role_policy_attachment.test-AmazonEKSClusterPolicy", "aws_iam_role_policy_attachment.test-AmazonEKSServicePolicy"] + } + `, rName, tagKey1, tagValue1, tagKey2, tagValue2) + } + ``` + +- Verify all acceptance testing passes for the resource (e.g. `make testacc TESTARGS='-run=TestAccAWSEksCluster_'`) + +#### Resource Tagging Documentation Implementation + +- In the resource documentation (e.g. `website/docs/r/eks_cluster.html.markdown`), add the following to the arguments reference: + + ```markdown + * `tags` - (Optional) Key-value mapping of resource tags + ``` + #### New Resource Implementing a new resource is a good way to learn more about how Terraform diff --git a/CHANGELOG.md b/CHANGELOG.md index 649ee6876a2..47f77524e25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ ## 2.34.0 (Unreleased) + +BUG FIXES: + +* resource/aws_cloudhsm_v2_cluster: Ensure multiple tag configurations are applied correctly [GH-10309] +* resource/aws_cloudhsm_v2_cluster: Perform drift detection with tags [GH-10309] + ## 2.33.0 (October 17, 2019) FEATURES: diff --git a/aws/data_source_aws_ecr_repository.go b/aws/data_source_aws_ecr_repository.go index b2f25e6f569..9bc8e4df698 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-plugin-sdk/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/data_source_aws_subnet.go b/aws/data_source_aws_subnet.go index 04a8cdeb7ea..064d3c25d3b 100644 --- a/aws/data_source_aws_subnet.go +++ b/aws/data_source_aws_subnet.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func dataSourceAwsSubnet() *schema.Resource { @@ -162,7 +163,11 @@ func dataSourceAwsSubnetRead(d *schema.ResourceData, meta interface{}) error { d.Set("cidr_block", subnet.CidrBlock) d.Set("default_for_az", subnet.DefaultForAz) d.Set("state", subnet.State) - d.Set("tags", tagsToMap(subnet.Tags)) + + if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(subnet.Tags).IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + d.Set("assign_ipv6_address_on_creation", subnet.AssignIpv6AddressOnCreation) d.Set("map_public_ip_on_launch", subnet.MapPublicIpOnLaunch) diff --git a/aws/data_source_aws_vpc.go b/aws/data_source_aws_vpc.go index 32c3dc58385..edd13c10c8b 100644 --- a/aws/data_source_aws_vpc.go +++ b/aws/data_source_aws_vpc.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func dataSourceAwsVpc() *schema.Resource { @@ -176,7 +177,11 @@ func dataSourceAwsVpcRead(d *schema.ResourceData, meta interface{}) error { d.Set("instance_tenancy", vpc.InstanceTenancy) d.Set("default", vpc.IsDefault) d.Set("state", vpc.State) - d.Set("tags", tagsToMap(vpc.Tags)) + + if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(vpc.Tags).IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + d.Set("owner_id", vpc.OwnerId) arn := arn.ARN{ diff --git a/aws/resource_aws_cloudhsm2_cluster.go b/aws/resource_aws_cloudhsm2_cluster.go index 1db323e61f7..65dda087432 100644 --- a/aws/resource_aws_cloudhsm2_cluster.go +++ b/aws/resource_aws_cloudhsm2_cluster.go @@ -2,15 +2,15 @@ package aws import ( "fmt" - "github.com/hashicorp/terraform-plugin-sdk/helper/validation" "log" "time" - "github.com/hashicorp/terraform-plugin-sdk/helper/schema" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudhsmv2" "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 resourceAwsCloudHsm2Cluster() *schema.Resource { @@ -212,16 +212,19 @@ func resourceAwsCloudHsm2ClusterCreate(d *schema.ResourceData, meta interface{}) } } - if err := setTagsAwsCloudHsm2Cluster(cloudhsm2, d); err != nil { - return err + if v := d.Get("tags").(map[string]interface{}); len(v) > 0 { + if err := keyvaluetags.Cloudhsmv2UpdateTags(cloudhsm2, d.Id(), nil, v); err != nil { + return fmt.Errorf("error updating tags: %s", err) + } } return resourceAwsCloudHsm2ClusterRead(d, meta) } func resourceAwsCloudHsm2ClusterRead(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).cloudhsmv2conn - cluster, err := describeCloudHsm2Cluster(meta.(*AWSClient).cloudhsmv2conn, d.Id()) + cluster, err := describeCloudHsm2Cluster(conn, d.Id()) if cluster == nil { log.Printf("[WARN] CloudHSMv2 Cluster (%s) not found", d.Id()) @@ -249,14 +252,27 @@ func resourceAwsCloudHsm2ClusterRead(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("Error saving Subnet IDs to state for CloudHSMv2 Cluster (%s): %s", d.Id(), err) } + tags, err := keyvaluetags.Cloudhsmv2ListTags(conn, d.Id()) + + if err != nil { + return fmt.Errorf("error listing tags for resource (%s): %s", d.Id(), err) + } + + if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + return nil } func resourceAwsCloudHsm2ClusterUpdate(d *schema.ResourceData, meta interface{}) error { - cloudhsm2 := meta.(*AWSClient).cloudhsmv2conn + conn := meta.(*AWSClient).cloudhsmv2conn - if err := setTagsAwsCloudHsm2Cluster(cloudhsm2, d); err != nil { - return err + if d.HasChange("tags") { + o, n := d.GetChange("tags") + if err := keyvaluetags.Cloudhsmv2UpdateTags(conn, d.Id(), o, n); err != nil { + return fmt.Errorf("error updating tags: %s", err) + } } return resourceAwsCloudHsm2ClusterRead(d, meta) @@ -296,48 +312,6 @@ func resourceAwsCloudHsm2ClusterDelete(d *schema.ResourceData, meta interface{}) return nil } -func setTagsAwsCloudHsm2Cluster(conn *cloudhsmv2.CloudHSMV2, d *schema.ResourceData) error { - if d.HasChange("tags") { - oraw, nraw := d.GetChange("tags") - create, remove := diffTagsGeneric(oraw.(map[string]interface{}), nraw.(map[string]interface{})) - - if len(remove) > 0 { - log.Printf("[DEBUG] Removing tags: %#v", remove) - keys := make([]*string, 0, len(remove)) - for k := range remove { - keys = append(keys, aws.String(k)) - } - - _, err := conn.UntagResource(&cloudhsmv2.UntagResourceInput{ - ResourceId: aws.String(d.Id()), - TagKeyList: keys, - }) - if err != nil { - return err - } - } - if len(create) > 0 { - log.Printf("[DEBUG] Creating tags: %#v", create) - tagList := make([]*cloudhsmv2.Tag, 0, len(create)) - for k, v := range create { - tagList = append(tagList, &cloudhsmv2.Tag{ - Key: &k, - Value: v, - }) - } - _, err := conn.TagResource(&cloudhsmv2.TagResourceInput{ - ResourceId: aws.String(d.Id()), - TagList: tagList, - }) - if err != nil { - return err - } - } - } - - return nil -} - func readCloudHsm2ClusterCertificates(cluster *cloudhsmv2.Cluster) []map[string]interface{} { certs := map[string]interface{}{} if cluster.Certificates != nil { diff --git a/aws/resource_aws_cloudhsm2_cluster_test.go b/aws/resource_aws_cloudhsm2_cluster_test.go index 2d9c3a4c7e7..6ec1d8f8b13 100644 --- a/aws/resource_aws_cloudhsm2_cluster_test.go +++ b/aws/resource_aws_cloudhsm2_cluster_test.go @@ -8,7 +8,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudhsmv2" - "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" ) @@ -85,32 +84,77 @@ func testSweepCloudhsmv2Clusters(region string) error { } func TestAccAWSCloudHsm2Cluster_basic(t *testing.T) { - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSCloudHsm2ClusterDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSCloudHsm2Cluster(), + Config: testAccAWSCloudHsm2ClusterConfig(), Check: resource.ComposeTestCheckFunc( testAccCheckAWSCloudHsm2ClusterExists("aws_cloudhsm_v2_cluster.cluster"), resource.TestCheckResourceAttrSet("aws_cloudhsm_v2_cluster.cluster", "cluster_id"), resource.TestCheckResourceAttrSet("aws_cloudhsm_v2_cluster.cluster", "vpc_id"), resource.TestCheckResourceAttrSet("aws_cloudhsm_v2_cluster.cluster", "security_group_id"), resource.TestCheckResourceAttrSet("aws_cloudhsm_v2_cluster.cluster", "cluster_state"), + resource.TestCheckResourceAttr("aws_cloudhsm_v2_cluster.cluster", "tags.%", "0"), ), }, { ResourceName: "aws_cloudhsm_v2_cluster.cluster", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"cluster_certificates", "tags"}, + ImportStateVerifyIgnore: []string{"cluster_certificates"}, }, }, }) } -func testAccAWSCloudHsm2Cluster() string { +func TestAccAWSCloudHsm2Cluster_Tags(t *testing.T) { + resourceName := "aws_cloudhsm_v2_cluster.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSCloudHsm2ClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSCloudHsm2ClusterConfigTags2("key1", "value1", "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSCloudHsm2ClusterExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"cluster_certificates"}, + }, + { + Config: testAccAWSCloudHsm2ClusterConfigTags1("key1", "value1updated"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSCloudHsm2ClusterExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + ), + }, + { + Config: testAccAWSCloudHsm2ClusterConfigTags2("key1", "value1updated", "key3", "value3"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSCloudHsm2ClusterExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key3", "value3"), + ), + }, + }, + }) +} + +func testAccAWSCloudHsm2ClusterConfigBase() string { return fmt.Sprintf(` variable "subnets" { default = ["10.0.1.0/24", "10.0.2.0/24"] @@ -138,16 +182,43 @@ resource "aws_subnet" "cloudhsm2_test_subnets" { Name = "tf-acc-aws_cloudhsm_v2_cluster-resource-basic" } } +`) +} +func testAccAWSCloudHsm2ClusterConfig() string { + return testAccAWSCloudHsm2ClusterConfigBase() + fmt.Sprintf(` resource "aws_cloudhsm_v2_cluster" "cluster" { hsm_type = "hsm1.medium" subnet_ids = ["${aws_subnet.cloudhsm2_test_subnets.*.id[0]}", "${aws_subnet.cloudhsm2_test_subnets.*.id[1]}"] +} +`) +} + +func testAccAWSCloudHsm2ClusterConfigTags1(tagKey1, tagValue1 string) string { + return testAccAWSCloudHsm2ClusterConfigBase() + fmt.Sprintf(` +resource "aws_cloudhsm_v2_cluster" "test" { + hsm_type = "hsm1.medium" + subnet_ids = ["${aws_subnet.cloudhsm2_test_subnets.*.id[0]}", "${aws_subnet.cloudhsm2_test_subnets.*.id[1]}"] + + tags = { + %[1]q = %[2]q + } +} +`, tagKey1, tagValue1) +} + +func testAccAWSCloudHsm2ClusterConfigTags2(tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return testAccAWSCloudHsm2ClusterConfigBase() + fmt.Sprintf(` +resource "aws_cloudhsm_v2_cluster" "test" { + hsm_type = "hsm1.medium" + subnet_ids = ["${aws_subnet.cloudhsm2_test_subnets.*.id[0]}", "${aws_subnet.cloudhsm2_test_subnets.*.id[1]}"] tags = { - Name = "tf-acc-aws_cloudhsm_v2_cluster-resource-basic-%d" + %[1]q = %[2]q + %[3]q = %[4]q } } -`, acctest.RandInt()) +`, tagKey1, tagValue1, tagKey2, tagValue2) } func testAccCheckAWSCloudHsm2ClusterDestroy(s *terraform.State) error { diff --git a/aws/resource_aws_ecr_repository.go b/aws/resource_aws_ecr_repository.go index 7a8ea89b349..dca6b4c332b 100644 --- a/aws/resource_aws_ecr_repository.go +++ b/aws/resource_aws_ecr_repository.go @@ -1,16 +1,16 @@ package aws import ( + "fmt" "log" "time" - "fmt" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecr" "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 resourceAwsEcrRepository() *schema.Resource { @@ -65,7 +65,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 +119,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 +150,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/resource_aws_licensemanager_license_configuration.go b/aws/resource_aws_licensemanager_license_configuration.go index 028424dc634..6e94dda4d9f 100644 --- a/aws/resource_aws_licensemanager_license_configuration.go +++ b/aws/resource_aws_licensemanager_license_configuration.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/service/licensemanager" "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 resourceAwsLicenseManagerLicenseConfiguration() *schema.Resource { @@ -89,7 +90,7 @@ func resourceAwsLicenseManagerLicenseConfigurationCreate(d *schema.ResourceData, } if v, ok := d.GetOk("tags"); ok && len(v.(map[string]interface{})) > 0 { - opts.Tags = tagsFromMapLicenseManager(v.(map[string]interface{})) + opts.Tags = keyvaluetags.New(v.(map[string]interface{})).IgnoreAws().LicensemanagerTags() } log.Printf("[DEBUG] License Manager license configuration: %s", opts) @@ -127,7 +128,7 @@ func resourceAwsLicenseManagerLicenseConfigurationRead(d *schema.ResourceData, m } d.Set("name", resp.Name) - if err := d.Set("tags", tagsToMapLicenseManager(resp.Tags)); err != nil { + if err := d.Set("tags", keyvaluetags.LicensemanagerKeyValueTags(resp.Tags).IgnoreAws().Map()); err != nil { return fmt.Errorf("error setting tags: %s", err) } @@ -140,9 +141,12 @@ func resourceAwsLicenseManagerLicenseConfigurationUpdate(d *schema.ResourceData, d.Partial(true) if d.HasChange("tags") { - if err := setTagsLicenseManager(conn, d); err != nil { - return err + o, n := d.GetChange("tags") + + if err := keyvaluetags.LicensemanagerUpdateTags(conn, d.Id(), o, n); err != nil { + return fmt.Errorf("error updating License Manager License Configuration (%s) tags: %s", d.Id(), err) } + d.SetPartial("tags") } diff --git a/aws/resource_aws_quicksight_group.go b/aws/resource_aws_quicksight_group.go index b82a3fcc572..8376a415868 100644 --- a/aws/resource_aws_quicksight_group.go +++ b/aws/resource_aws_quicksight_group.go @@ -82,7 +82,7 @@ func resourceAwsQuickSightGroupCreate(d *schema.ResourceData, meta interface{}) resp, err := conn.CreateGroup(createOpts) if err != nil { - return fmt.Errorf("Error creating Quick Sight Group: %s", err) + return fmt.Errorf("Error creating QuickSight Group: %s", err) } d.SetId(fmt.Sprintf("%s/%s/%s", awsAccountID, namespace, aws.StringValue(resp.Group.GroupName))) @@ -106,12 +106,12 @@ func resourceAwsQuickSightGroupRead(d *schema.ResourceData, meta interface{}) er resp, err := conn.DescribeGroup(descOpts) if isAWSErr(err, quicksight.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] Quick Sight Group %s is already gone", d.Id()) + log.Printf("[WARN] QuickSight Group %s is already gone", d.Id()) d.SetId("") return nil } if err != nil { - return fmt.Errorf("Error describing Quick Sight Group (%s): %s", d.Id(), err) + return fmt.Errorf("Error describing QuickSight Group (%s): %s", d.Id(), err) } d.Set("arn", resp.Group.Arn) @@ -143,12 +143,12 @@ func resourceAwsQuickSightGroupUpdate(d *schema.ResourceData, meta interface{}) _, err = conn.UpdateGroup(updateOpts) if isAWSErr(err, quicksight.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] Quick Sight Group %s is already gone", d.Id()) + log.Printf("[WARN] QuickSight Group %s is already gone", d.Id()) d.SetId("") return nil } if err != nil { - return fmt.Errorf("Error updating Quick Sight Group %s: %s", d.Id(), err) + return fmt.Errorf("Error updating QuickSight Group %s: %s", d.Id(), err) } return resourceAwsQuickSightGroupRead(d, meta) @@ -172,7 +172,7 @@ func resourceAwsQuickSightGroupDelete(d *schema.ResourceData, meta interface{}) if isAWSErr(err, quicksight.ErrCodeResourceNotFoundException, "") { return nil } - return fmt.Errorf("Error deleting Quick Sight Group %s: %s", d.Id(), err) + return fmt.Errorf("Error deleting QuickSight Group %s: %s", d.Id(), err) } return nil diff --git a/aws/resource_aws_quicksight_group_test.go b/aws/resource_aws_quicksight_group_test.go index a699fd30e8e..6d2756f5a93 100644 --- a/aws/resource_aws_quicksight_group_test.go +++ b/aws/resource_aws_quicksight_group_test.go @@ -132,7 +132,7 @@ func testAccCheckQuickSightGroupExists(resourceName string, group *quicksight.Gr } if output == nil || output.Group == nil { - return fmt.Errorf("Quick Sight Group (%s) not found", rs.Primary.ID) + return fmt.Errorf("QuickSight Group (%s) not found", rs.Primary.ID) } *group = *output.Group @@ -166,7 +166,7 @@ func testAccCheckQuickSightGroupDestroy(s *terraform.State) error { return err } - return fmt.Errorf("Quick Sight Group '%s' was not deleted properly", rs.Primary.ID) + return fmt.Errorf("QuickSight Group '%s' was not deleted properly", rs.Primary.ID) } return nil diff --git a/aws/resource_aws_subnet.go b/aws/resource_aws_subnet.go index 2ce8e8fee31..56c8c402d1c 100644 --- a/aws/resource_aws_subnet.go +++ b/aws/resource_aws_subnet.go @@ -10,6 +10,7 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsSubnet() *schema.Resource { @@ -140,7 +141,69 @@ func resourceAwsSubnetCreate(d *schema.ResourceData, meta interface{}) error { d.Id(), err) } - return resourceAwsSubnetUpdate(d, meta) + if v := d.Get("tags").(map[string]interface{}); len(v) > 0 { + // Handle EC2 eventual consistency on creation + err := resource.Retry(5*time.Minute, func() *resource.RetryError { + err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v) + + if isAWSErr(err, "InvalidSubnetID.NotFound", "") { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if isResourceTimeoutError(err) { + err = keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v) + } + + if err != nil { + return fmt.Errorf("error adding tags: %s", err) + } + + d.SetPartial("tags") + } + + // You cannot modify multiple subnet attributes in the same request. + // Reference: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_ModifySubnetAttribute.html + + if d.Get("assign_ipv6_address_on_creation").(bool) { + input := &ec2.ModifySubnetAttributeInput{ + AssignIpv6AddressOnCreation: &ec2.AttributeBooleanValue{ + Value: aws.Bool(true), + }, + SubnetId: aws.String(d.Id()), + } + + if _, err := conn.ModifySubnetAttribute(input); err != nil { + return fmt.Errorf("error enabling EC2 Subnet (%s) assign IPv6 address on creation: %s", d.Id(), err) + } + + d.SetPartial("assign_ipv6_address_on_creation") + } + + if d.Get("map_public_ip_on_launch").(bool) { + input := &ec2.ModifySubnetAttributeInput{ + MapPublicIpOnLaunch: &ec2.AttributeBooleanValue{ + Value: aws.Bool(true), + }, + SubnetId: aws.String(d.Id()), + } + + if _, err := conn.ModifySubnetAttribute(input); err != nil { + return fmt.Errorf("error enabling EC2 Subnet (%s) map public IP on launch: %s", d.Id(), err) + } + + d.SetPartial("map_public_ip_on_launch") + } + + d.Partial(false) + + return resourceAwsSubnetRead(d, meta) } func resourceAwsSubnetRead(d *schema.ResourceData, meta interface{}) error { @@ -184,7 +247,11 @@ func resourceAwsSubnetRead(d *schema.ResourceData, meta interface{}) error { } d.Set("arn", subnet.SubnetArn) - d.Set("tags", tagsToMap(subnet.Tags)) + + if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(subnet.Tags).IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } + d.Set("owner_id", subnet.OwnerId) return nil @@ -195,9 +262,13 @@ func resourceAwsSubnetUpdate(d *schema.ResourceData, meta interface{}) error { d.Partial(true) - if err := setTags(conn, d); err != nil { - return err - } else { + if d.HasChange("tags") { + o, n := d.GetChange("tags") + + if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil { + return fmt.Errorf("error updating EC2 Subnet (%s) tags: %s", d.Id(), err) + } + d.SetPartial("tags") } @@ -220,9 +291,7 @@ func resourceAwsSubnetUpdate(d *schema.ResourceData, meta interface{}) error { } } - // We have to be careful here to not go through a change of association if this is a new resource - // A New resource here would denote that the Update func is called by the Create func - if d.HasChange("ipv6_cidr_block") && !d.IsNewResource() { + if d.HasChange("ipv6_cidr_block") { // We need to handle that we disassociate the IPv6 CIDR block before we try and associate the new one // This could be an issue as, we could error out when we try and add the new one // We may need to roll back the state and reattach the old one if this is the case diff --git a/aws/tagsECR.go b/aws/tagsECR.go deleted file mode 100644 index b8b5a800d24..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-plugin-sdk/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) - } - } -} diff --git a/aws/tagsLicenseManager.go b/aws/tagsLicenseManager.go deleted file mode 100644 index 867a82c51c7..00000000000 --- a/aws/tagsLicenseManager.go +++ /dev/null @@ -1,115 +0,0 @@ -package aws - -import ( - "log" - "regexp" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/licensemanager" - "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 setTagsLicenseManager(conn *licensemanager.LicenseManager, 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 := diffTagsLicenseManager(tagsFromMapLicenseManager(o), tagsFromMapLicenseManager(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(&licensemanager.UntagResourceInput{ - ResourceArn: aws.String(d.Id()), - TagKeys: k, - }) - if err != nil { - return err - } - } - if len(create) > 0 { - log.Printf("[DEBUG] Creating tags: %#v", create) - _, err := conn.TagResource(&licensemanager.TagResourceInput{ - ResourceArn: aws.String(d.Id()), - 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 diffTagsLicenseManager(oldTags, newTags []*licensemanager.Tag) ([]*licensemanager.Tag, []*licensemanager.Tag) { - // 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 []*licensemanager.Tag - for _, t := range oldTags { - old, ok := create[*t.Key] - if !ok || old != *t.Value { - // Delete it! - remove = append(remove, t) - } - } - - return tagsFromMapLicenseManager(create), remove -} - -// tagsFromMap returns the tags for the given map of data. -func tagsFromMapLicenseManager(m map[string]interface{}) []*licensemanager.Tag { - result := make([]*licensemanager.Tag, 0, len(m)) - for k, v := range m { - t := &licensemanager.Tag{ - Key: aws.String(k), - Value: aws.String(v.(string)), - } - if !tagIgnoredLicenseManager(t) { - result = append(result, t) - } - } - - return result -} - -// tagsToMap turns the list of tags into a map. -func tagsToMapLicenseManager(ts []*licensemanager.Tag) map[string]string { - result := make(map[string]string) - for _, t := range ts { - if !tagIgnoredLicenseManager(t) { - result[*t.Key] = *t.Value - } - } - - return result -} - -// compare a tag against a list of strings and checks if it should -// be ignored or not -func tagIgnoredLicenseManager(t *licensemanager.Tag) bool { - filter := []string{"^aws:"} - for _, v := range filter { - log.Printf("[DEBUG] Matching %v with %v\n", v, *t.Key) - if r, _ := regexp.MatchString(v, *t.Key); r { - log.Printf("[DEBUG] Found AWS specific tag %s (val: %s), ignoring.\n", *t.Key, *t.Value) - return true - } - } - return false -} diff --git a/go.mod b/go.mod index 2ec1ac707c0..fc50985d49d 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/hashicorp/go-cleanhttp v0.5.1 github.com/hashicorp/go-multierror v1.0.0 github.com/hashicorp/go-version v1.2.0 - github.com/hashicorp/terraform-plugin-sdk v1.1.1 + github.com/hashicorp/terraform-plugin-sdk v1.2.0 github.com/hashicorp/vault v0.10.4 github.com/jen20/awspolicyequivalence v1.0.0 github.com/json-iterator/go v1.1.5 // indirect diff --git a/go.sum b/go.sum index 5aab7030588..41ae3465780 100644 --- a/go.sum +++ b/go.sum @@ -237,8 +237,8 @@ github.com/hashicorp/terraform-config-inspect v0.0.0-20190821133035-82a99dc22ef4 github.com/hashicorp/terraform-config-inspect v0.0.0-20190821133035-82a99dc22ef4/go.mod h1:JDmizlhaP5P0rYTTZB0reDMefAiJyfWPEtugV4in1oI= github.com/hashicorp/terraform-plugin-sdk v1.0.0 h1:3AjuuV1LJKs1NlG+heUgqWN6/QCSx2kDhyS6K7F0fTw= github.com/hashicorp/terraform-plugin-sdk v1.0.0/go.mod h1:NuwtLpEpPsFaKJPJNGtMcn9vlhe6Ofe+Y6NqXhJgV2M= -github.com/hashicorp/terraform-plugin-sdk v1.1.1 h1:wQ2HtvOE8K4QYcm2JB8YFw9u7OplFJ2HMV5WOpMRL2Y= -github.com/hashicorp/terraform-plugin-sdk v1.1.1/go.mod h1:NuwtLpEpPsFaKJPJNGtMcn9vlhe6Ofe+Y6NqXhJgV2M= +github.com/hashicorp/terraform-plugin-sdk v1.2.0 h1:OASbbPmJHbajCSXelwWLSw/HkvnakG4rd/GcrqD2k/g= +github.com/hashicorp/terraform-plugin-sdk v1.2.0/go.mod h1:NuwtLpEpPsFaKJPJNGtMcn9vlhe6Ofe+Y6NqXhJgV2M= github.com/hashicorp/vault v0.10.4 h1:4x0lHxui/ZRp/B3E0Auv1QNBJpzETqHR2kQD3mHSBJU= github.com/hashicorp/vault v0.10.4/go.mod h1:KfSyffbKxoVyspOdlaGVjIuwLobi07qD1bAbosPMpP0= github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb h1:b5rjCoWHc7eqmAS4/qyk21ZsHyb6Mxv/jykxvNTkU4M= diff --git a/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/resource/testing.go b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/resource/testing.go index b2a323cec78..eb0b58c220e 100644 --- a/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/resource/testing.go +++ b/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/resource/testing.go @@ -2,6 +2,7 @@ package resource import ( "bytes" + "errors" "flag" "fmt" "io" @@ -54,13 +55,10 @@ import ( // destroyed. var flagSweep = flag.String("sweep", "", "List of Regions to run available Sweepers") +var flagSweepAllowFailures = flag.Bool("sweep-allow-failures", false, "Enable to allow Sweeper Tests to continue after failures") var flagSweepRun = flag.String("sweep-run", "", "Comma seperated list of Sweeper Tests to run") var sweeperFuncs map[string]*Sweeper -// map of sweepers that have ran, and the success/fail status based on any error -// raised -var sweeperRunList map[string]bool - // type SweeperFunc is a signature for a function that acts as a sweeper. It // accepts a string for the region that the sweeper is to be ran in. This // function must be able to construct a valid client for that region. @@ -105,26 +103,64 @@ func TestMain(m *testing.M) { // get filtered list of sweepers to run based on sweep-run flag sweepers := filterSweepers(*flagSweepRun, sweeperFuncs) - for _, region := range regions { - region = strings.TrimSpace(region) - // reset sweeperRunList for each region - sweeperRunList = map[string]bool{} - - log.Printf("[DEBUG] Running Sweepers for region (%s):\n", region) - for _, sweeper := range sweepers { - if err := runSweeperWithRegion(region, sweeper); err != nil { - log.Fatalf("[ERR] error running (%s): %s", sweeper.Name, err) + + if _, err := runSweepers(regions, sweepers, *flagSweepAllowFailures); err != nil { + os.Exit(1) + } + } else { + os.Exit(m.Run()) + } +} + +func runSweepers(regions []string, sweepers map[string]*Sweeper, allowFailures bool) (map[string]map[string]error, error) { + var sweeperErrorFound bool + sweeperRunList := make(map[string]map[string]error) + + for _, region := range regions { + region = strings.TrimSpace(region) + + var regionSweeperErrorFound bool + regionSweeperRunList := make(map[string]error) + + log.Printf("[DEBUG] Running Sweepers for region (%s):\n", region) + for _, sweeper := range sweepers { + if err := runSweeperWithRegion(region, sweeper, sweepers, regionSweeperRunList, allowFailures); err != nil { + if allowFailures { + continue } + + sweeperRunList[region] = regionSweeperRunList + return sweeperRunList, fmt.Errorf("sweeper (%s) for region (%s) failed: %s", sweeper.Name, region, err) } + } - log.Printf("Sweeper Tests ran:\n") - for s, _ := range sweeperRunList { - fmt.Printf("\t- %s\n", s) + log.Printf("Sweeper Tests ran successfully:\n") + for sweeper, sweeperErr := range regionSweeperRunList { + if sweeperErr == nil { + fmt.Printf("\t- %s\n", sweeper) + } else { + regionSweeperErrorFound = true } } - } else { - os.Exit(m.Run()) + + if regionSweeperErrorFound { + sweeperErrorFound = true + log.Printf("Sweeper Tests ran unsuccessfully:\n") + for sweeper, sweeperErr := range regionSweeperRunList { + if sweeperErr != nil { + fmt.Printf("\t- %s: %s\n", sweeper, sweeperErr) + } + } + } + + sweeperRunList[region] = regionSweeperRunList + } + + if sweeperErrorFound { + return sweeperRunList, errors.New("at least one sweeper failed") } + + return sweeperRunList, nil } // filterSweepers takes a comma seperated string listing the names of sweepers @@ -153,11 +189,18 @@ func filterSweepers(f string, source map[string]*Sweeper) map[string]*Sweeper { // itself with that region for every dependency found for that sweeper. If there // are no dependencies, invoke the contained sweeper fun with the region, and // add the success/fail status to the sweeperRunList. -func runSweeperWithRegion(region string, s *Sweeper) error { +func runSweeperWithRegion(region string, s *Sweeper, sweepers map[string]*Sweeper, sweeperRunList map[string]error, allowFailures bool) error { for _, dep := range s.Dependencies { - if depSweeper, ok := sweeperFuncs[dep]; ok { + if depSweeper, ok := sweepers[dep]; ok { log.Printf("[DEBUG] Sweeper (%s) has dependency (%s), running..", s.Name, dep) - if err := runSweeperWithRegion(region, depSweeper); err != nil { + err := runSweeperWithRegion(region, depSweeper, sweepers, sweeperRunList, allowFailures) + + if err != nil { + if allowFailures { + log.Printf("[ERROR] Error running Sweeper (%s) in region (%s): %s", depSweeper.Name, region, err) + continue + } + return err } } else { @@ -170,11 +213,14 @@ func runSweeperWithRegion(region string, s *Sweeper) error { return nil } + log.Printf("[DEBUG] Running Sweeper (%s) in region (%s)", s.Name, region) + runE := s.F(region) - if runE == nil { - sweeperRunList[s.Name] = true - } else { - sweeperRunList[s.Name] = false + + sweeperRunList[s.Name] = runE + + if runE != nil { + log.Printf("[ERROR] Error running Sweeper (%s) in region (%s): %s", s.Name, region, runE) } return runE diff --git a/vendor/modules.txt b/vendor/modules.txt index a49fb0f395e..0cd5db71551 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -417,7 +417,7 @@ github.com/hashicorp/hil/ast github.com/hashicorp/logutils # github.com/hashicorp/terraform-config-inspect v0.0.0-20190821133035-82a99dc22ef4 github.com/hashicorp/terraform-config-inspect/tfconfig -# github.com/hashicorp/terraform-plugin-sdk v1.1.1 +# github.com/hashicorp/terraform-plugin-sdk v1.2.0 github.com/hashicorp/terraform-plugin-sdk/helper/acctest github.com/hashicorp/terraform-plugin-sdk/helper/customdiff github.com/hashicorp/terraform-plugin-sdk/helper/encryption diff --git a/website/docs/r/codebuild_project.html.markdown b/website/docs/r/codebuild_project.html.markdown index d20bf6d43cb..2821f026974 100755 --- a/website/docs/r/codebuild_project.html.markdown +++ b/website/docs/r/codebuild_project.html.markdown @@ -161,7 +161,7 @@ resource "aws_codebuild_project" "example" { security_group_ids = [ "${aws_security_group.example1.id}", - "${aws_security_gorup.example2.id}", + "${aws_security_group.example2.id}", ] } diff --git a/website/docs/r/quicksight_group.html.markdown b/website/docs/r/quicksight_group.html.markdown index 388edd8ce73..b17533d9d87 100644 --- a/website/docs/r/quicksight_group.html.markdown +++ b/website/docs/r/quicksight_group.html.markdown @@ -2,12 +2,12 @@ layout: "aws" page_title: "AWS: aws_quicksight_group" description: |- - Manages a Resource Quick Sight Group. + Manages a Resource QuickSight Group. --- # Resource: aws_quicksight_group -Resource for managing Quick Sight Group +Resource for managing QuickSight Group ## Example Usage @@ -34,7 +34,7 @@ In addition to all arguments above, the following attributes are exported: ## Import -Quick Sight Group can be imported using the aws account id, namespace and group name separated by `/`. +QuickSight Group can be imported using the aws account id, namespace and group name separated by `/`. ``` $ terraform import aws_quicksight_group.example 123456789123/default/tf-example diff --git a/website/docs/r/ses_identity_policy.html.markdown b/website/docs/r/ses_identity_policy.html.markdown index cb6847c65aa..5dad2a34a50 100644 --- a/website/docs/r/ses_identity_policy.html.markdown +++ b/website/docs/r/ses_identity_policy.html.markdown @@ -19,7 +19,7 @@ resource "aws_ses_domain_identity" "example" { data "aws_iam_policy_document" "example" { statement { actions = ["SES:SendEmail", "SES:SendRawEmail"] - resources = ["${aws_ses_domain_identity.test.arn}"] + resources = ["${aws_ses_domain_identity.example.arn}"] principals { identifiers = ["*"] @@ -48,5 +48,5 @@ The following arguments are supported: SES Identity Policies can be imported using the identity and policy name, separated by a pipe character (`|`), e.g. ``` -$ terraform import aws_ses_identity_policy.test 'example.com|example' +$ terraform import aws_ses_identity_policy.example 'example.com|example' ```