From 50f0f6ec784799398b124b1cb8bfaa0b0cfcea37 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Sat, 13 Jun 2020 14:22:02 -0400 Subject: [PATCH] internal/keyvaluetags: Create {SERVICE}GetTag generator, support EC2 list/get, use in aws_ec2_tag implementation Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/9061 The GetTag generator simplifies the creation of the new individual service tag resources into a consistent implementation. This consistent implementation can be used to automatically generate the service tag resources themselves in the future. Output from acceptance testing: ``` --- PASS: TestAccAWSEc2Tag_basic (485.52s) --- PASS: TestAccAWSEc2Tag_disappears (529.67s) --- PASS: TestAccAWSEc2Tag_Value (588.49s) ``` --- aws/internal/keyvaluetags/README.md | 2 + .../keyvaluetags/generators/gettag/main.go | 133 ++++++++++++++++++ .../keyvaluetags/generators/listtags/main.go | 11 ++ .../generators/servicetags/main.go | 27 ++++ aws/internal/keyvaluetags/get_tag_gen.go | 86 +++++++++++ aws/internal/keyvaluetags/key_value_tags.go | 23 +++ .../keyvaluetags/key_value_tags_test.go | 96 +++++++++++++ aws/internal/keyvaluetags/list_tags_gen.go | 23 +++ .../service_generation_customizations.go | 24 ++++ aws/internal/keyvaluetags/service_tags_gen.go | 26 +++- aws/resource_aws_ec2_tag.go | 34 +---- aws/resource_aws_ec2_tag_test.go | 70 ++------- 12 files changed, 458 insertions(+), 97 deletions(-) create mode 100644 aws/internal/keyvaluetags/generators/gettag/main.go create mode 100644 aws/internal/keyvaluetags/get_tag_gen.go diff --git a/aws/internal/keyvaluetags/README.md b/aws/internal/keyvaluetags/README.md index 681ddad9e73..0b7930f539a 100644 --- a/aws/internal/keyvaluetags/README.md +++ b/aws/internal/keyvaluetags/README.md @@ -19,6 +19,8 @@ Any tagging functions that cannot be generated should be hand implemented in a s ```text aws/internal/keyvaluetags ├── generators +│ ├── createtags (generates create_tags_gen.go) +│ ├── gettag (generates get_tag_gen.go) │ ├── listtags (generates list_tags_gen.go) │ ├── servicetags (generates service_tags_gen.go) │ └── updatetags (generates update_tags_gen.go) diff --git a/aws/internal/keyvaluetags/generators/gettag/main.go b/aws/internal/keyvaluetags/generators/gettag/main.go new file mode 100644 index 00000000000..b01858bcf50 --- /dev/null +++ b/aws/internal/keyvaluetags/generators/gettag/main.go @@ -0,0 +1,133 @@ +// +build ignore + +package main + +import ( + "bytes" + "go/format" + "log" + "os" + "sort" + "strings" + "text/template" + + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" +) + +const filename = `get_tag_gen.go` + +var serviceNames = []string{ + "dynamodb", + "ec2", + "ecs", + "route53resolver", +} + +type TemplateData struct { + ServiceNames []string +} + +func main() { + // Always sort to reduce any potential generation churn + sort.Strings(serviceNames) + + templateData := TemplateData{ + ServiceNames: serviceNames, + } + templateFuncMap := template.FuncMap{ + "ClientType": keyvaluetags.ServiceClientType, + "ListTagsFunction": keyvaluetags.ServiceListTagsFunction, + "ListTagsInputFilterIdentifierName": keyvaluetags.ServiceListTagsInputFilterIdentifierName, + "ListTagsInputResourceTypeField": keyvaluetags.ServiceListTagsInputResourceTypeField, + "ListTagsOutputTagsField": keyvaluetags.ServiceListTagsOutputTagsField, + "TagPackage": keyvaluetags.ServiceTagPackage, + "Title": strings.Title, + } + + tmpl, err := template.New("gettag").Funcs(templateFuncMap).Parse(templateBody) + + if err != nil { + log.Fatalf("error parsing template: %s", err) + } + + var buffer bytes.Buffer + err = tmpl.Execute(&buffer, templateData) + + if err != nil { + log.Fatalf("error executing template: %s", err) + } + + generatedFileContents, err := format.Source(buffer.Bytes()) + + if err != nil { + log.Fatalf("error formatting generated file: %s", err) + } + + f, err := os.Create(filename) + + if err != nil { + log.Fatalf("error creating file (%s): %s", filename, err) + } + + defer f.Close() + + _, err = f.Write(generatedFileContents) + + if err != nil { + log.Fatalf("error writing to file (%s): %s", filename, err) + } +} + +var templateBody = ` +// Code generated by generators/gettag/main.go; DO NOT EDIT. + +package keyvaluetags + +import ( + "github.com/aws/aws-sdk-go/aws" +{{- range .ServiceNames }} + "github.com/aws/aws-sdk-go/service/{{ . }}" +{{- end }} +) + +{{- range .ServiceNames }} + +// {{ . | Title }}GetTag fetches an individual {{ . }} service tag for a resource. +// Returns whether the key exists, the key value, and any errors. +// This function will optimise the handling over {{ . | Title }}ListTags, if possible. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func {{ . | Title }}GetTag(conn {{ . | ClientType }}, identifier string{{ if . | ListTagsInputResourceTypeField }}, resourceType string{{ end }}, key string) (bool, *string, error) { + {{- if . | ListTagsInputFilterIdentifierName }} + input := &{{ . | TagPackage }}.{{ . | ListTagsFunction }}Input{ + Filters: []*{{ . | TagPackage }}.Filter{ + { + Name: aws.String("{{ . | ListTagsInputFilterIdentifierName }}"), + Values: []*string{aws.String(identifier)}, + }, + { + Name: aws.String("key"), + Values: []*string{aws.String(key)}, + }, + }, + } + + output, err := conn.{{ . | ListTagsFunction }}(input) + + if err != nil { + return false, nil, err + } + + listTags := {{ . | Title }}KeyValueTags(output.{{ . | ListTagsOutputTagsField }}) + {{- else }} + listTags, err := {{ . | Title }}ListTags(conn, identifier{{ if . | ListTagsInputResourceTypeField }}, resourceType{{ end }}) + + if err != nil { + return false, nil, err + } + {{- end }} + + return listTags.KeyExists(key), listTags.KeyValue(key), nil +} +{{- end }} +` diff --git a/aws/internal/keyvaluetags/generators/listtags/main.go b/aws/internal/keyvaluetags/generators/listtags/main.go index e193e2515fb..9ce6dd2867b 100644 --- a/aws/internal/keyvaluetags/generators/listtags/main.go +++ b/aws/internal/keyvaluetags/generators/listtags/main.go @@ -51,6 +51,7 @@ var serviceNames = []string{ "dlm", "docdb", "dynamodb", + "ec2", "ecr", "ecs", "efs", @@ -129,6 +130,7 @@ func main() { templateFuncMap := template.FuncMap{ "ClientType": keyvaluetags.ServiceClientType, "ListTagsFunction": keyvaluetags.ServiceListTagsFunction, + "ListTagsInputFilterIdentifierName": keyvaluetags.ServiceListTagsInputFilterIdentifierName, "ListTagsInputIdentifierField": keyvaluetags.ServiceListTagsInputIdentifierField, "ListTagsInputIdentifierRequiresSlice": keyvaluetags.ServiceListTagsInputIdentifierRequiresSlice, "ListTagsInputResourceTypeField": keyvaluetags.ServiceListTagsInputResourceTypeField, @@ -189,6 +191,14 @@ import ( // it may also be a different identifier depending on the service. func {{ . | Title }}ListTags(conn {{ . | ClientType }}, identifier string{{ if . | ListTagsInputResourceTypeField }}, resourceType string{{ end }}) (KeyValueTags, error) { input := &{{ . | TagPackage }}.{{ . | ListTagsFunction }}Input{ + {{- if . | ListTagsInputFilterIdentifierName }} + Filters: []*{{ . | TagPackage }}.Filter{ + { + Name: aws.String("{{ . | ListTagsInputFilterIdentifierName }}"), + Values: []*string{aws.String(identifier)}, + }, + }, + {{- else }} {{- if . | ListTagsInputIdentifierRequiresSlice }} {{ . | ListTagsInputIdentifierField }}: aws.StringSlice([]string{identifier}), {{- else }} @@ -197,6 +207,7 @@ func {{ . | Title }}ListTags(conn {{ . | ClientType }}, identifier string{{ if . {{- if . | ListTagsInputResourceTypeField }} {{ . | ListTagsInputResourceTypeField }}: aws.String(resourceType), {{- end }} + {{- end }} } output, err := conn.{{ . | ListTagsFunction }}(input) diff --git a/aws/internal/keyvaluetags/generators/servicetags/main.go b/aws/internal/keyvaluetags/generators/servicetags/main.go index 2af089f6787..3caee9f25a9 100644 --- a/aws/internal/keyvaluetags/generators/servicetags/main.go +++ b/aws/internal/keyvaluetags/generators/servicetags/main.go @@ -155,6 +155,7 @@ func main() { "TagKeyType": keyvaluetags.ServiceTagKeyType, "TagPackage": keyvaluetags.ServiceTagPackage, "TagType": keyvaluetags.ServiceTagType, + "TagType2": keyvaluetags.ServiceTagType2, "TagTypeKeyField": keyvaluetags.ServiceTagTypeKeyField, "TagTypeValueField": keyvaluetags.ServiceTagTypeValueField, "Title": strings.Title, @@ -259,6 +260,31 @@ func (tags KeyValueTags) {{ . | Title }}Tags() []*{{ . | TagPackage }}.{{ . | Ta } // {{ . | Title }}KeyValueTags creates KeyValueTags from {{ . }} service tags. +{{- if . | TagType2 }} +// Accepts []*{{ . | TagPackage }}.{{ . | TagType }} and []*{{ . | TagPackage }}.{{ . | TagType2 }}. +func {{ . | Title }}KeyValueTags(tags interface{}) KeyValueTags { + switch tags := tags.(type) { + case []*{{ . | TagPackage }}.{{ . | TagType }}: + m := make(map[string]*string, len(tags)) + + for _, tag := range tags { + m[aws.StringValue(tag.{{ . | TagTypeKeyField }})] = tag.{{ . | TagTypeValueField }} + } + + return New(m) + case []*{{ . | TagPackage }}.{{ . | TagType2 }}: + m := make(map[string]*string, len(tags)) + + for _, tag := range tags { + m[aws.StringValue(tag.{{ . | TagTypeKeyField }})] = tag.{{ . | TagTypeValueField }} + } + + return New(m) + default: + return New(nil) + } +} +{{- else }} func {{ . | Title }}KeyValueTags(tags []*{{ . | TagPackage }}.{{ . | TagType }}) KeyValueTags { m := make(map[string]*string, len(tags)) @@ -269,4 +295,5 @@ func {{ . | Title }}KeyValueTags(tags []*{{ . | TagPackage }}.{{ . | TagType }}) return New(m) } {{- end }} +{{- end }} ` diff --git a/aws/internal/keyvaluetags/get_tag_gen.go b/aws/internal/keyvaluetags/get_tag_gen.go new file mode 100644 index 00000000000..f2bf57afd23 --- /dev/null +++ b/aws/internal/keyvaluetags/get_tag_gen.go @@ -0,0 +1,86 @@ +// Code generated by generators/gettag/main.go; DO NOT EDIT. + +package keyvaluetags + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/dynamodb" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/ecs" + "github.com/aws/aws-sdk-go/service/route53resolver" +) + +// DynamodbGetTag fetches an individual dynamodb service tag for a resource. +// Returns whether the key exists, the key value, and any errors. +// This function will optimise the handling over DynamodbListTags, if possible. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func DynamodbGetTag(conn *dynamodb.DynamoDB, identifier string, key string) (bool, *string, error) { + listTags, err := DynamodbListTags(conn, identifier) + + if err != nil { + return false, nil, err + } + + return listTags.KeyExists(key), listTags.KeyValue(key), nil +} + +// Ec2GetTag fetches an individual ec2 service tag for a resource. +// Returns whether the key exists, the key value, and any errors. +// This function will optimise the handling over Ec2ListTags, if possible. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func Ec2GetTag(conn *ec2.EC2, identifier string, key string) (bool, *string, error) { + input := &ec2.DescribeTagsInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("resource-id"), + Values: []*string{aws.String(identifier)}, + }, + { + Name: aws.String("key"), + Values: []*string{aws.String(key)}, + }, + }, + } + + output, err := conn.DescribeTags(input) + + if err != nil { + return false, nil, err + } + + listTags := Ec2KeyValueTags(output.Tags) + + return listTags.KeyExists(key), listTags.KeyValue(key), nil +} + +// EcsGetTag fetches an individual ecs service tag for a resource. +// Returns whether the key exists, the key value, and any errors. +// This function will optimise the handling over EcsListTags, if possible. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func EcsGetTag(conn *ecs.ECS, identifier string, key string) (bool, *string, error) { + listTags, err := EcsListTags(conn, identifier) + + if err != nil { + return false, nil, err + } + + return listTags.KeyExists(key), listTags.KeyValue(key), nil +} + +// Route53resolverGetTag fetches an individual route53resolver service tag for a resource. +// Returns whether the key exists, the key value, and any errors. +// This function will optimise the handling over Route53resolverListTags, if possible. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func Route53resolverGetTag(conn *route53resolver.Route53Resolver, identifier string, key string) (bool, *string, error) { + listTags, err := Route53resolverListTags(conn, identifier) + + if err != nil { + return false, nil, err + } + + return listTags.KeyExists(key), listTags.KeyValue(key), nil +} diff --git a/aws/internal/keyvaluetags/key_value_tags.go b/aws/internal/keyvaluetags/key_value_tags.go index 58c27c34d73..71370437b17 100644 --- a/aws/internal/keyvaluetags/key_value_tags.go +++ b/aws/internal/keyvaluetags/key_value_tags.go @@ -1,5 +1,6 @@ //go:generate go run -tags generate generators/servicetags/main.go //go:generate go run -tags generate generators/listtags/main.go +//go:generate go run -tags generate generators/gettag/main.go //go:generate go run -tags generate generators/createtags/main.go //go:generate go run -tags generate generators/updatetags/main.go @@ -138,6 +139,28 @@ func (tags KeyValueTags) Ignore(ignoreTags KeyValueTags) KeyValueTags { return result } +// KeyExists returns true if a tag key exists. +// If the key is not found, returns nil. +// Use KeyExists to determine if key is present. +func (tags KeyValueTags) KeyExists(key string) bool { + if _, ok := tags[key]; ok { + return true + } + + return false +} + +// KeyValue returns a tag key value. +// If the key is not found, returns nil. +// Use KeyExists to determine if key is present. +func (tags KeyValueTags) KeyValue(key string) *string { + if v, ok := tags[key]; ok { + return v + } + + return nil +} + // Keys returns tag keys. func (tags KeyValueTags) Keys() []string { result := make([]string, 0, len(tags)) diff --git a/aws/internal/keyvaluetags/key_value_tags_test.go b/aws/internal/keyvaluetags/key_value_tags_test.go index dedcc5acaae..789829f8a68 100644 --- a/aws/internal/keyvaluetags/key_value_tags_test.go +++ b/aws/internal/keyvaluetags/key_value_tags_test.go @@ -533,6 +533,102 @@ func TestKeyValueTagsIgnore(t *testing.T) { } } +func TestKeyValueTagsKeyExists(t *testing.T) { + testCases := []struct { + name string + tags KeyValueTags + key string + want bool + }{ + { + name: "empty", + tags: New(map[string]*string{}), + key: "key1", + want: false, + }, + { + name: "non-existent", + tags: New(map[string]*string{"key1": testStringPtr("value1")}), + key: "key2", + want: false, + }, + { + name: "matching with string value", + tags: New(map[string]*string{"key1": testStringPtr("value1")}), + key: "key1", + want: true, + }, + { + name: "matching with nil value", + tags: New(map[string]*string{"key1": nil}), + key: "key1", + want: true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + got := testCase.tags.KeyExists(testCase.key) + + if got != testCase.want { + t.Fatalf("expected: %t, got: %t", testCase.want, got) + } + }) + } +} + +func TestKeyValueTagsKeyValues(t *testing.T) { + testCases := []struct { + name string + tags KeyValueTags + key string + want *string + }{ + { + name: "empty", + tags: New(map[string]*string{}), + key: "key1", + want: nil, + }, + { + name: "non-existent", + tags: New(map[string]*string{"key1": testStringPtr("value1")}), + key: "key2", + want: nil, + }, + { + name: "matching with string value", + tags: New(map[string]*string{"key1": testStringPtr("value1")}), + key: "key1", + want: testStringPtr("value1"), + }, + { + name: "matching with nil value", + tags: New(map[string]*string{"key1": nil}), + key: "key1", + want: nil, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + got := testCase.tags.KeyValue(testCase.key) + + if testCase.want == nil && got != nil { + t.Fatalf("expected: nil, got: %s", *got) + } + + if testCase.want != nil && got == nil { + t.Fatalf("expected: %s, got: nil", *testCase.want) + } + + if testCase.want != nil && got != nil && *testCase.want != *got { + t.Fatalf("expected: %s, got: %s", *testCase.want, *got) + } + }) + } +} + func TestKeyValueTagsKeys(t *testing.T) { testCases := []struct { name string diff --git a/aws/internal/keyvaluetags/list_tags_gen.go b/aws/internal/keyvaluetags/list_tags_gen.go index 100c4080937..bff556eaeac 100644 --- a/aws/internal/keyvaluetags/list_tags_gen.go +++ b/aws/internal/keyvaluetags/list_tags_gen.go @@ -38,6 +38,7 @@ import ( "github.com/aws/aws-sdk-go/service/dlm" "github.com/aws/aws-sdk-go/service/docdb" "github.com/aws/aws-sdk-go/service/dynamodb" + "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ecr" "github.com/aws/aws-sdk-go/service/ecs" "github.com/aws/aws-sdk-go/service/efs" @@ -680,6 +681,28 @@ func DynamodbListTags(conn *dynamodb.DynamoDB, identifier string) (KeyValueTags, return DynamodbKeyValueTags(output.Tags), nil } +// Ec2ListTags lists ec2 service tags. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func Ec2ListTags(conn *ec2.EC2, identifier string) (KeyValueTags, error) { + input := &ec2.DescribeTagsInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("resource-id"), + Values: []*string{aws.String(identifier)}, + }, + }, + } + + output, err := conn.DescribeTags(input) + + if err != nil { + return New(nil), err + } + + return Ec2KeyValueTags(output.Tags), nil +} + // EcrListTags lists ecr service tags. // The identifier is typically the Amazon Resource Name (ARN), although // it may also be a different identifier depending on the service. diff --git a/aws/internal/keyvaluetags/service_generation_customizations.go b/aws/internal/keyvaluetags/service_generation_customizations.go index 20786db583d..8bbdaf2936e 100644 --- a/aws/internal/keyvaluetags/service_generation_customizations.go +++ b/aws/internal/keyvaluetags/service_generation_customizations.go @@ -360,6 +360,8 @@ func ServiceListTagsFunction(serviceName string) string { return "DescribeTags" case "dynamodb": return "ListTagsOfResource" + case "ec2": + return "DescribeTags" case "efs": return "DescribeTags" case "elasticsearchservice": @@ -401,6 +403,17 @@ func ServiceListTagsFunction(serviceName string) string { } } +// ServiceListTagsInputFilterIdentifierName determines the service list tag filter identifier field. +// This causes the implementation to use the Filters field with the Input struct. +func ServiceListTagsInputFilterIdentifierName(serviceName string) string { + switch serviceName { + case "ec2": + return "resource-id" + default: + return "" + } +} + // ServiceListTagsInputIdentifierField determines the service list tag identifier field. func ServiceListTagsInputIdentifierField(serviceName string) string { switch serviceName { @@ -818,6 +831,17 @@ func ServiceTagType(serviceName string) string { } } +// ServiceTagType2 determines if the service tagging has a second tag type. +// The two types must be equivalent. +func ServiceTagType2(serviceName string) string { + switch serviceName { + case "ec2": + return "TagDescription" + default: + return "" + } +} + // ServiceTagTypeKeyField determines the service tagging tag type key field. func ServiceTagTypeKeyField(serviceName string) string { switch serviceName { diff --git a/aws/internal/keyvaluetags/service_tags_gen.go b/aws/internal/keyvaluetags/service_tags_gen.go index 8e7a0432a67..e21b971836e 100644 --- a/aws/internal/keyvaluetags/service_tags_gen.go +++ b/aws/internal/keyvaluetags/service_tags_gen.go @@ -1120,14 +1120,28 @@ func (tags KeyValueTags) Ec2Tags() []*ec2.Tag { } // Ec2KeyValueTags creates KeyValueTags from ec2 service tags. -func Ec2KeyValueTags(tags []*ec2.Tag) KeyValueTags { - m := make(map[string]*string, len(tags)) +// Accepts []*ec2.Tag and []*ec2.TagDescription. +func Ec2KeyValueTags(tags interface{}) KeyValueTags { + switch tags := tags.(type) { + case []*ec2.Tag: + m := make(map[string]*string, len(tags)) - for _, tag := range tags { - m[aws.StringValue(tag.Key)] = tag.Value - } + for _, tag := range tags { + m[aws.StringValue(tag.Key)] = tag.Value + } - return New(m) + return New(m) + case []*ec2.TagDescription: + m := make(map[string]*string, len(tags)) + + for _, tag := range tags { + m[aws.StringValue(tag.Key)] = tag.Value + } + + return New(m) + default: + return New(nil) + } } // EcrTags returns ecr service tags. diff --git a/aws/resource_aws_ec2_tag.go b/aws/resource_aws_ec2_tag.go index b8218847634..48c27516152 100644 --- a/aws/resource_aws_ec2_tag.go +++ b/aws/resource_aws_ec2_tag.go @@ -5,8 +5,6 @@ import ( "log" "strings" - "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" ) @@ -74,39 +72,13 @@ func resourceAwsEc2TagRead(d *schema.ResourceData, meta interface{}) error { return err } - input := &ec2.DescribeTagsInput{ - Filters: []*ec2.Filter{ - { - Name: aws.String("resource-id"), - Values: []*string{aws.String(resourceID)}, - }, - { - Name: aws.String("key"), - Values: []*string{aws.String(key)}, - }, - }, - } - - output, err := conn.DescribeTags(input) + exists, value, err := keyvaluetags.Ec2GetTag(conn, resourceID, key) if err != nil { return fmt.Errorf("error reading EC2 Tag (%s) for resource (%s): %w", key, resourceID, err) } - if output == nil { - return fmt.Errorf("error reading EC2 Tag (%s) for resource (%s): empty response", key, resourceID) - } - - var tag *ec2.TagDescription - - for _, outputTag := range output.Tags { - if aws.StringValue(outputTag.Key) == key { - tag = outputTag - break - } - } - - if tag == nil { + if !exists { log.Printf("[WARN] EC2 Tag (%s) for resource (%s) not found, removing from state", key, resourceID) d.SetId("") return nil @@ -114,7 +86,7 @@ func resourceAwsEc2TagRead(d *schema.ResourceData, meta interface{}) error { d.Set("key", key) d.Set("resource_id", resourceID) - d.Set("value", tag.Value) + d.Set("value", value) return nil } diff --git a/aws/resource_aws_ec2_tag_test.go b/aws/resource_aws_ec2_tag_test.go index 9f066903ac4..8aae8f54554 100644 --- a/aws/resource_aws_ec2_tag_test.go +++ b/aws/resource_aws_ec2_tag_test.go @@ -4,14 +4,12 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func TestAccAWSEc2Tag_basic(t *testing.T) { - var tag ec2.TagDescription rBgpAsn := randIntRange(64512, 65534) resourceName := "aws_ec2_tag.test" @@ -23,7 +21,7 @@ func TestAccAWSEc2Tag_basic(t *testing.T) { { Config: testAccEc2TagConfig(rBgpAsn, "key1", "value1"), Check: resource.ComposeTestCheckFunc( - testAccCheckEc2TagExists(resourceName, &tag), + testAccCheckEc2TagExists(resourceName), resource.TestCheckResourceAttr(resourceName, "key", "key1"), resource.TestCheckResourceAttr(resourceName, "value", "value1"), ), @@ -38,7 +36,6 @@ func TestAccAWSEc2Tag_basic(t *testing.T) { } func TestAccAWSEc2Tag_disappears(t *testing.T) { - var tag ec2.TagDescription rBgpAsn := randIntRange(64512, 65534) resourceName := "aws_ec2_tag.test" @@ -50,7 +47,7 @@ func TestAccAWSEc2Tag_disappears(t *testing.T) { { Config: testAccEc2TagConfig(rBgpAsn, "key1", "value1"), Check: resource.ComposeTestCheckFunc( - testAccCheckEc2TagExists(resourceName, &tag), + testAccCheckEc2TagExists(resourceName), testAccCheckResourceDisappears(testAccProvider, resourceAwsEc2Tag(), resourceName), ), ExpectNonEmptyPlan: true, @@ -60,7 +57,6 @@ func TestAccAWSEc2Tag_disappears(t *testing.T) { } func TestAccAWSEc2Tag_Value(t *testing.T) { - var tag ec2.TagDescription rBgpAsn := randIntRange(64512, 65534) resourceName := "aws_ec2_tag.test" @@ -72,7 +68,7 @@ func TestAccAWSEc2Tag_Value(t *testing.T) { { Config: testAccEc2TagConfig(rBgpAsn, "key1", "value1"), Check: resource.ComposeTestCheckFunc( - testAccCheckEc2TagExists(resourceName, &tag), + testAccCheckEc2TagExists(resourceName), resource.TestCheckResourceAttr(resourceName, "key", "key1"), resource.TestCheckResourceAttr(resourceName, "value", "value1"), ), @@ -85,7 +81,7 @@ func TestAccAWSEc2Tag_Value(t *testing.T) { { Config: testAccEc2TagConfig(rBgpAsn, "key1", "value1updated"), Check: resource.ComposeTestCheckFunc( - testAccCheckEc2TagExists(resourceName, &tag), + testAccCheckEc2TagExists(resourceName), resource.TestCheckResourceAttr(resourceName, "key", "key1"), resource.TestCheckResourceAttr(resourceName, "value", "value1updated"), ), @@ -108,35 +104,13 @@ func testAccCheckEc2TagDestroy(s *terraform.State) error { return err } - input := &ec2.DescribeTagsInput{ - Filters: []*ec2.Filter{ - { - Name: aws.String("resource-id"), - Values: []*string{aws.String(resourceID)}, - }, - { - Name: aws.String("key"), - Values: []*string{aws.String(key)}, - }, - }, - } - - output, err := conn.DescribeTags(input) + exists, _, err := keyvaluetags.Ec2GetTag(conn, resourceID, key) if err != nil { return err } - var tag *ec2.TagDescription - - for _, outputTag := range output.Tags { - if aws.StringValue(outputTag.Key) == key { - tag = outputTag - break - } - } - - if tag != nil { + if exists { return fmt.Errorf("Tag (%s) for resource (%s) still exists", key, resourceID) } } @@ -144,7 +118,7 @@ func testAccCheckEc2TagDestroy(s *terraform.State) error { return nil } -func testAccCheckEc2TagExists(n string, tag *ec2.TagDescription) resource.TestCheckFunc { +func testAccCheckEc2TagExists(n string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -163,33 +137,9 @@ func testAccCheckEc2TagExists(n string, tag *ec2.TagDescription) resource.TestCh conn := testAccProvider.Meta().(*AWSClient).ec2conn - input := &ec2.DescribeTagsInput{ - Filters: []*ec2.Filter{ - { - Name: aws.String("resource-id"), - Values: []*string{aws.String(resourceID)}, - }, - { - Name: aws.String("key"), - Values: []*string{aws.String(key)}, - }, - }, - } - - output, err := conn.DescribeTags(input) - - if err != nil { - return err - } - - for _, outputTag := range output.Tags { - if aws.StringValue(outputTag.Key) == key { - *tag = *outputTag - break - } - } + exists, _, err := keyvaluetags.Ec2GetTag(conn, resourceID, key) - if tag == nil { + if !exists { return fmt.Errorf("Tag (%s) for resource (%s) not found", key, resourceID) }