From e5bf34e8331473d53c2a92a53896921c2266c8ff Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 30 Sep 2019 16:36:59 -0400 Subject: [PATCH 1/2] docs/provider: Add Adding Resource Tagging Support section Includes information how to work with the new `internal/keyvaluetags` generators. --- .github/CONTRIBUTING.md | 196 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index e0f65a9e9548..d93523a04c75 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,201 @@ 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 Stategy 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(),` +- 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() + } + ``` + +- 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 From a732852d4e30eec3cc7923a5e089e6dbe85a35bf Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Sun, 6 Oct 2019 13:35:00 -0400 Subject: [PATCH 2/2] docs/provider: Update Resource Tagging Code Implementation section to show tagging on creation versus not, fix typo Reference: https://github.com/terraform-providers/terraform-provider-aws/pull/10313 --- .github/CONTRIBUTING.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index d93523a04c75..f3860eac918f 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -216,7 +216,7 @@ In addition to the below checklist and the items noted in the Extending Terrafor #### 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 Stategy page](https://aws.amazon.com/answers/account-management/aws-tagging-strategies/) for more information about tagging at a high level. +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: @@ -255,7 +255,7 @@ More details about this code generation, including fixes for potential error mes - 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(),` -- In the resource `Create` function, implement the logic to convert the configuration tags into the service tags, e.g. with EKS Clusters: +- 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{ @@ -276,6 +276,16 @@ More details about this code generation, including fixes for potential error mes } ``` +- 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