Skip to content

Commit

Permalink
Merge pull request #33167 from hashicorp/b-aws_kms_key-tag-update-tim…
Browse files Browse the repository at this point in the history
…eout

r/aws_kms_key: Fix tag propagation timeout errors when `ignore_tags` is configured
  • Loading branch information
ewbankkit committed Aug 24, 2023
2 parents 0d5c5f6 + d639d0c commit b8b8d2c
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 14 deletions.
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

0 comments on commit b8b8d2c

Please sign in to comment.