Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/aws_kms_key: Fix tag propagation timeout errors when ignore_tags is configured #33167

Merged
merged 15 commits into from
Aug 24, 2023
Merged
3 changes: 3 additions & 0 deletions .changelog/33167.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_kms_key: Fix `tag propagation: timeout while waiting for state to become 'TRUE'` errors when [`ignore_tags`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#ignore_tags) has been configured
```
16 changes: 8 additions & 8 deletions internal/acctest/acctest.go
Original file line number Diff line number Diff line change
Expand Up @@ -1193,11 +1193,11 @@ func ConfigDefaultAndIgnoreTagsKeyPrefixes1(key1, value1, keyPrefix1 string) str
provider "aws" {
default_tags {
tags = {
%q = %q
%[1]q = %[2]q
}
}
ignore_tags {
key_prefixes = [%q]
key_prefixes = [%[3]q]
}
}
`, key1, value1, keyPrefix1)
Expand All @@ -1209,7 +1209,7 @@ func ConfigDefaultAndIgnoreTagsKeys1(key1, value1 string) string {
provider "aws" {
default_tags {
tags = {
%[1]q = %q
%[1]q = %[2]q
}
}
ignore_tags {
Expand Down Expand Up @@ -1569,7 +1569,7 @@ func ConfigDefaultTags_Tags1(tag1, value1 string) string {
provider "aws" {
default_tags {
tags = {
%q = %q
%[1]q = %[2]q
}
}

Expand All @@ -1588,8 +1588,8 @@ func ConfigDefaultTags_Tags2(tag1, value1, tag2, value2 string) string {
provider "aws" {
default_tags {
tags = {
%q = %q
%q = %q
%[1]q = %[2]q
%[3]q = %[4]q
}
}

Expand All @@ -1609,8 +1609,8 @@ func ConfigAssumeRolePolicy(policy string) string {
return fmt.Sprintf(`
provider "aws" {
assume_role {
role_arn = %q
policy = %q
role_arn = %[1]q
policy = %[2]q
}
}
`, os.Getenv(envvar.AccAssumeRoleARN), policy)
Expand Down
3 changes: 3 additions & 0 deletions internal/generate/tags/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ var (
listTagsInIDElem = flag.String("ListTagsInIDElem", "ResourceArn", "listTagsInIDElem")
listTagsInIDNeedSlice = flag.String("ListTagsInIDNeedSlice", "", "listTagsInIDNeedSlice")
listTagsOp = flag.String("ListTagsOp", "ListTagsForResource", "listTagsOp")
listTagsOpPaginated = flag.Bool("ListTagsOpPaginated", false, "whether ListTagsOp is paginated")
listTagsOutTagsElem = flag.String("ListTagsOutTagsElem", "Tags", "listTagsOutTagsElem")
setTagsOutFunc = flag.String("SetTagsOutFunc", "setTagsOut", "setTagsOutFunc")
tagInCustomVal = flag.String("TagInCustomVal", "", "tagInCustomVal")
Expand Down Expand Up @@ -164,6 +165,7 @@ type TemplateData struct {
ListTagsInIDElem string
ListTagsInIDNeedSlice string
ListTagsOp string
ListTagsOpPaginated bool
ListTagsOutTagsElem string
ParentNotFoundErrCode string
ParentNotFoundErrMsg string
Expand Down Expand Up @@ -318,6 +320,7 @@ func main() {
ListTagsInIDElem: *listTagsInIDElem,
ListTagsInIDNeedSlice: *listTagsInIDNeedSlice,
ListTagsOp: *listTagsOp,
ListTagsOpPaginated: *listTagsOpPaginated,
ListTagsOutTagsElem: *listTagsOutTagsElem,
ParentNotFoundErrCode: *parentNotFoundErrCode,
ParentNotFoundErrMsg: *parentNotFoundErrMsg,
Expand Down
22 changes: 22 additions & 0 deletions internal/generate/tags/templates/v1/list_tags_body.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,26 @@ func {{ .ListTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifier
{{- end }}
{{- end }}
}
{{- if .ListTagsOpPaginated }}
var output []*{{ .TagPackage }}.{{ .TagType }}

err := conn.{{ .ListTagsOp }}PagesWithContext(ctx, input, func(page *{{ .TagPackage }}.{{ .ListTagsOp }}Output, lastPage bool) bool {
if page == nil {
return !lastPage
}

for _, v := range page.{{ .ListTagsOutTagsElem }} {
if v != nil {
output = append(output, v)
}
}

return !lastPage
})
{{ else }}

output, err := conn.{{ .ListTagsOp }}WithContext(ctx, input)
{{- end }}

{{ if and ( .ParentNotFoundErrCode ) ( .ParentNotFoundErrMsg ) }}
if tfawserr.ErrMessageContains(err, "{{ .ParentNotFoundErrCode }}", "{{ .ParentNotFoundErrMsg }}") {
Expand All @@ -44,7 +62,11 @@ func {{ .ListTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifier
return tftags.New(ctx, nil), err
}

{{ if .ListTagsOpPaginated }}
return {{ .KeyValueTagsFunc }}(ctx, output{{ if .TagTypeIDElem }}, identifier{{ if .TagResTypeElem }}, resourceType{{ end }}{{ end }}), nil
{{- else }}
return {{ .KeyValueTagsFunc }}(ctx, output.{{ .ListTagsOutTagsElem }}{{ if .TagTypeIDElem }}, identifier{{ if .TagResTypeElem }}, resourceType{{ end }}{{ end }}), nil
{{- end }}
}

{{- if .IsDefaultListTags }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ func {{ .WaitTagsPropagatedFunc }}(ctx context.Context, conn {{ .ClientType }},
})

checkFunc := func() (bool, error) {
output, err := listTags(ctx, conn, id)
output, err := {{ .ListTagsFunc }}(ctx, conn, id)

if tfresource.NotFound(err) {
return false, nil
Expand All @@ -17,6 +17,10 @@ func {{ .WaitTagsPropagatedFunc }}(ctx context.Context, conn {{ .ClientType }},
return false, err
}

if inContext, ok := tftags.FromContext(ctx); ok {
output = output.IgnoreConfig(inContext.IgnoreConfig)
}

return output.Equal(tags), nil
}
opts := tfresource.WaitOpts{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ func {{ .WaitTagsPropagatedFunc }}(ctx context.Context, conn {{ .ClientType }},
})

checkFunc := func() (bool, error) {
output, err := listTags(ctx, conn, id)
output, err := {{ .ListTagsFunc }}(ctx, conn, id)

if tfresource.NotFound(err) {
return false, nil
Expand All @@ -17,6 +17,10 @@ func {{ .WaitTagsPropagatedFunc }}(ctx context.Context, conn {{ .ClientType }},
return false, err
}

if inContext, ok := tftags.FromContext(ctx); ok {
output = output.IgnoreConfig(inContext.IgnoreConfig)
}

return output.Equal(tags), nil
}
opts := tfresource.WaitOpts{
Expand Down
2 changes: 1 addition & 1 deletion internal/service/kms/generate.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

//go:generate go run ../../generate/tags/main.go -ListTags -ListTagsOp=ListResourceTags -ListTagsInIDElem=KeyId -ServiceTagsSlice -TagInIDElem=KeyId -TagTypeKeyElem=TagKey -TagTypeValElem=TagValue -UpdateTags -Wait -WaitContinuousOccurence 5 -WaitMinTimeout 1s -WaitTimeout 10m -ParentNotFoundErrCode=NotFoundException
//go:generate go run ../../generate/tags/main.go -ListTags -ListTagsOp=ListResourceTags -ListTagsOpPaginated -ListTagsInIDElem=KeyId -ServiceTagsSlice -TagInIDElem=KeyId -TagTypeKeyElem=TagKey -TagTypeValElem=TagValue -UpdateTags -Wait -WaitContinuousOccurence 5 -WaitMinTimeout 1s -WaitTimeout 10m -ParentNotFoundErrCode=NotFoundException
//go:generate go run ../../generate/servicepackage/main.go
// ONLY generate directives and package declaration! Do not add anything else to this file.

Expand Down
86 changes: 86 additions & 0 deletions internal/service/kms/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,48 @@ func TestAccKMSKey_tags(t *testing.T) {
})
}

// https://github.com/hashicorp/terraform-provider-aws/issues/26174.
func TestAccKMSKey_ignoreTags(t *testing.T) {
ctx := acctest.Context(t)
var key kms.KeyMetadata
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_kms_key.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, kms.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckKeyDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccKeyConfig_ignoreTags(rName, "key1", "value1", "key2", "value2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckKeyExists(ctx, resourceName, &key),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
),
},
// Add an ignored tag.
{
Config: testAccKeyConfig_ignoreTags(rName, "key1", "value1", "key2", "value2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckKeyAddTag(ctx, &key, "ignkey1", "ignvalue1"),
),
},
{
Config: testAccKeyConfig_ignoreTags(rName, "key1", "value1updated", "key3", "value3"),
Check: resource.ComposeTestCheckFunc(
testAccCheckKeyExists(ctx, resourceName, &key),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"),
resource.TestCheckResourceAttr(resourceName, "tags.key3", "value3"),
),
},
},
})
}

func testAccCheckKeyHasPolicy(ctx context.Context, name string, expectedPolicyText string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
Expand Down Expand Up @@ -605,6 +647,24 @@ func testAccCheckKeyExists(ctx context.Context, name string, key *kms.KeyMetadat
}
}

func testAccCheckKeyAddTag(ctx context.Context, key *kms.KeyMetadata, tagKey, tagValue string) resource.TestCheckFunc {
return func(s *terraform.State) error {
conn := acctest.Provider.Meta().(*conns.AWSClient).KMSConn(ctx)

input := &kms.TagResourceInput{
KeyId: key.KeyId,
Tags: []*kms.Tag{{
TagKey: aws.String(tagKey),
TagValue: aws.String(tagValue),
}},
}

_, err := conn.TagResourceWithContext(ctx, input)

return err
}
}

func testAccKeyConfig_basic() string {
return `
resource "aws_kms_key" "test" {}
Expand Down Expand Up @@ -1091,3 +1151,29 @@ resource "aws_kms_key" "test" {
}
`, rName)
}

func testAccKeyConfig_ignoreTags(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string {
// lintignore:AT004
return fmt.Sprintf(`
provider "aws" {
default_tags {
tags = {
"defkey1" = "defvalue1"
}
}
ignore_tags {
keys = ["ignkey1"]
}
}

resource "aws_kms_key" "test" {
description = %[1]q
deletion_window_in_days = 7

tags = {
%[2]q = %[3]q
%[4]q = %[5]q
}
}
`, rName, tagKey1, tagValue1, tagKey2, tagValue2)
}
21 changes: 19 additions & 2 deletions internal/service/kms/tags_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/tags/key_value_tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (tags KeyValueTags) IgnoreServerlessApplicationRepository() KeyValueTags {
return result
}

// IgnoreAWS returns non-system tag keys.
// IgnoreSystem returns non-system tag keys.
// The ignored keys vary on the specified service.
func (tags KeyValueTags) IgnoreSystem(serviceName string) KeyValueTags {
switch serviceName {
Expand Down
Loading