From 766b8dcc99bdeaf052110bc65e98a75adfc6c17e Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 4 Jul 2023 16:45:24 -0700 Subject: [PATCH 1/2] Generates `waitTagsPropagated` if needed and calls it from `updateTags` --- internal/generate/tags/main.go | 143 +++++++++++++----- .../tags/templates/v1/header_body.tmpl | 3 + .../tags/templates/v1/update_tags_body.tmpl | 8 + internal/generate/tags/templates/v1/v1.go | 3 + .../v1/wait_tags_propagated_body.tmpl | 34 +++++ .../tags/templates/v2/header_body.tmpl | 3 + .../tags/templates/v2/update_tags_body.tmpl | 8 + internal/generate/tags/templates/v2/v2.go | 3 + .../v2/wait_tags_propagated_body.tmpl | 34 +++++ internal/service/kms/external_key.go | 8 +- internal/service/kms/external_key_test.go | 36 +++-- internal/service/kms/generate.go | 2 +- internal/service/kms/key.go | 8 +- internal/service/kms/key_test.go | 21 +++ internal/service/kms/replica_external_key.go | 8 +- .../service/kms/replica_external_key_test.go | 48 +++++- internal/service/kms/replica_key.go | 8 +- internal/service/kms/replica_key_test.go | 47 +++++- internal/service/kms/tags_gen.go | 42 +++++ internal/service/kms/wait.go | 24 --- 20 files changed, 381 insertions(+), 110 deletions(-) create mode 100644 internal/generate/tags/templates/v1/wait_tags_propagated_body.tmpl create mode 100644 internal/generate/tags/templates/v2/wait_tags_propagated_body.tmpl diff --git a/internal/generate/tags/main.go b/internal/generate/tags/main.go index 9b1fb99dbf3..1595a4b3d14 100644 --- a/internal/generate/tags/main.go +++ b/internal/generate/tags/main.go @@ -12,6 +12,7 @@ import ( "os" "regexp" "strings" + "time" "github.com/hashicorp/terraform-provider-aws/internal/generate/common" v1 "github.com/hashicorp/terraform-provider-aws/internal/generate/tags/templates/v1" @@ -25,8 +26,9 @@ const ( ) const ( - defaultListTagsFunc = "listTags" - defaultUpdateTagsFunc = "updateTags" + defaultListTagsFunc = "listTags" + defaultUpdateTagsFunc = "updateTags" + defaultWaitTagsPropagatedFunc = "waitTagsPropagated" ) var ( @@ -38,38 +40,45 @@ var ( untagInNeedTagType = flag.Bool("UntagInNeedTagType", false, "whether Untag input needs tag type") updateTags = flag.Bool("UpdateTags", false, "whether to generate UpdateTags") updateTagsNoIgnoreSystem = flag.Bool("UpdateTagsNoIgnoreSystem", false, "whether to not ignore system tags in UpdateTags") - - createTagsFunc = flag.String("CreateTagsFunc", "createTags", "createTagsFunc") - getTagFunc = flag.String("GetTagFunc", "GetTag", "getTagFunc") - getTagsInFunc = flag.String("GetTagsInFunc", "getTagsIn", "getTagsInFunc") - keyValueTagsFunc = flag.String("KeyValueTagsFunc", "KeyValueTags", "keyValueTagsFunc") - listTagsFunc = flag.String("ListTagsFunc", defaultListTagsFunc, "listTagsFunc") - listTagsInFiltIDName = flag.String("ListTagsInFiltIDName", "", "listTagsInFiltIDName") - listTagsInIDElem = flag.String("ListTagsInIDElem", "ResourceArn", "listTagsInIDElem") - listTagsInIDNeedSlice = flag.String("ListTagsInIDNeedSlice", "", "listTagsInIDNeedSlice") - listTagsOp = flag.String("ListTagsOp", "ListTagsForResource", "listTagsOp") - listTagsOutTagsElem = flag.String("ListTagsOutTagsElem", "Tags", "listTagsOutTagsElem") - setTagsOutFunc = flag.String("SetTagsOutFunc", "setTagsOut", "setTagsOutFunc") - tagInCustomVal = flag.String("TagInCustomVal", "", "tagInCustomVal") - tagInIDElem = flag.String("TagInIDElem", "ResourceArn", "tagInIDElem") - tagInIDNeedSlice = flag.String("TagInIDNeedSlice", "", "tagInIDNeedSlice") - tagInTagsElem = flag.String("TagInTagsElem", "Tags", "tagInTagsElem") - tagKeyType = flag.String("TagKeyType", "", "tagKeyType") - tagOp = flag.String("TagOp", "TagResource", "tagOp") - tagOpBatchSize = flag.String("TagOpBatchSize", "", "tagOpBatchSize") - tagResTypeElem = flag.String("TagResTypeElem", "", "tagResTypeElem") - tagType = flag.String("TagType", "Tag", "tagType") - tagType2 = flag.String("TagType2", "", "tagType") - tagTypeAddBoolElem = flag.String("TagTypeAddBoolElem", "", "TagTypeAddBoolElem") - tagTypeIDElem = flag.String("TagTypeIDElem", "", "tagTypeIDElem") - tagTypeKeyElem = flag.String("TagTypeKeyElem", "Key", "tagTypeKeyElem") - tagTypeValElem = flag.String("TagTypeValElem", "Value", "tagTypeValElem") - tagsFunc = flag.String("TagsFunc", "Tags", "tagsFunc") - untagInCustomVal = flag.String("UntagInCustomVal", "", "untagInCustomVal") - untagInNeedTagKeyType = flag.String("UntagInNeedTagKeyType", "", "untagInNeedTagKeyType") - untagInTagsElem = flag.String("UntagInTagsElem", "TagKeys", "untagInTagsElem") - untagOp = flag.String("UntagOp", "UntagResource", "untagOp") - updateTagsFunc = flag.String("UpdateTagsFunc", defaultUpdateTagsFunc, "updateTagsFunc") + waitForPropagation = flag.Bool("Wait", false, "whether to generate WaitTagsPropagated") + + createTagsFunc = flag.String("CreateTagsFunc", "createTags", "createTagsFunc") + getTagFunc = flag.String("GetTagFunc", "GetTag", "getTagFunc") + getTagsInFunc = flag.String("GetTagsInFunc", "getTagsIn", "getTagsInFunc") + keyValueTagsFunc = flag.String("KeyValueTagsFunc", "KeyValueTags", "keyValueTagsFunc") + listTagsFunc = flag.String("ListTagsFunc", defaultListTagsFunc, "listTagsFunc") + listTagsInFiltIDName = flag.String("ListTagsInFiltIDName", "", "listTagsInFiltIDName") + listTagsInIDElem = flag.String("ListTagsInIDElem", "ResourceArn", "listTagsInIDElem") + listTagsInIDNeedSlice = flag.String("ListTagsInIDNeedSlice", "", "listTagsInIDNeedSlice") + listTagsOp = flag.String("ListTagsOp", "ListTagsForResource", "listTagsOp") + listTagsOutTagsElem = flag.String("ListTagsOutTagsElem", "Tags", "listTagsOutTagsElem") + setTagsOutFunc = flag.String("SetTagsOutFunc", "setTagsOut", "setTagsOutFunc") + tagInCustomVal = flag.String("TagInCustomVal", "", "tagInCustomVal") + tagInIDElem = flag.String("TagInIDElem", "ResourceArn", "tagInIDElem") + tagInIDNeedSlice = flag.String("TagInIDNeedSlice", "", "tagInIDNeedSlice") + tagInTagsElem = flag.String("TagInTagsElem", "Tags", "tagInTagsElem") + tagKeyType = flag.String("TagKeyType", "", "tagKeyType") + tagOp = flag.String("TagOp", "TagResource", "tagOp") + tagOpBatchSize = flag.String("TagOpBatchSize", "", "tagOpBatchSize") + tagResTypeElem = flag.String("TagResTypeElem", "", "tagResTypeElem") + tagType = flag.String("TagType", "Tag", "tagType") + tagType2 = flag.String("TagType2", "", "tagType") + tagTypeAddBoolElem = flag.String("TagTypeAddBoolElem", "", "TagTypeAddBoolElem") + tagTypeIDElem = flag.String("TagTypeIDElem", "", "tagTypeIDElem") + tagTypeKeyElem = flag.String("TagTypeKeyElem", "Key", "tagTypeKeyElem") + tagTypeValElem = flag.String("TagTypeValElem", "Value", "tagTypeValElem") + tagsFunc = flag.String("TagsFunc", "Tags", "tagsFunc") + untagInCustomVal = flag.String("UntagInCustomVal", "", "untagInCustomVal") + untagInNeedTagKeyType = flag.String("UntagInNeedTagKeyType", "", "untagInNeedTagKeyType") + untagInTagsElem = flag.String("UntagInTagsElem", "TagKeys", "untagInTagsElem") + untagOp = flag.String("UntagOp", "UntagResource", "untagOp") + updateTagsFunc = flag.String("UpdateTagsFunc", defaultUpdateTagsFunc, "updateTagsFunc") + waitTagsPropagatedFunc = flag.String("WaitFunc", defaultWaitTagsPropagatedFunc, "waitFunc") + waitContinuousOccurence = flag.Int("WaitContinuousOccurence", 0, "ContinuousTargetOccurence for Wait function") + waitDelay = flag.Duration("WaitDelay", 0, "Delay for Wait function") + waitMinTimeout = flag.Duration("WaitMinTimeout", 0, `"MinTimeout" (minimum poll interval) for Wait function`) + waitPollInterval = flag.Duration("WaitPollInterval", 0, "PollInterval for Wait function") + waitTimeout = flag.Duration("WaitTimeout", 0, "Timeout for Wait function") parentNotFoundErrCode = flag.String("ParentNotFoundErrCode", "", "Parent 'NotFound' Error Code") parentNotFoundErrMsg = flag.String("ParentNotFoundErrMsg", "", "Parent 'NotFound' Error Message") @@ -90,12 +99,13 @@ func usage() { } type TemplateBody struct { - getTag string - header string - listTags string - serviceTagsMap string - serviceTagsSlice string - updateTags string + getTag string + header string + listTags string + serviceTagsMap string + serviceTagsSlice string + updateTags string + waitTagsPropagated string } func newTemplateBody(version int, kvtValues bool) *TemplateBody { @@ -108,6 +118,7 @@ func newTemplateBody(version int, kvtValues bool) *TemplateBody { "\n" + v1.ServiceTagsMapBody, "\n" + v1.ServiceTagsSliceBody, "\n" + v1.UpdateTagsBody, + "\n" + v1.WaitTagsPropagatedBody, } case sdkV2: if kvtValues { @@ -118,6 +129,7 @@ func newTemplateBody(version int, kvtValues bool) *TemplateBody { "\n" + v2.ServiceTagsValueMapBody, "\n" + v2.ServiceTagsSliceBody, "\n" + v2.UpdateTagsBody, + "\n" + v2.WaitTagsPropagatedBody, } } return &TemplateBody{ @@ -127,6 +139,7 @@ func newTemplateBody(version int, kvtValues bool) *TemplateBody { "\n" + v2.ServiceTagsMapBody, "\n" + v2.ServiceTagsSliceBody, "\n" + v2.UpdateTagsBody, + "\n" + v2.WaitTagsPropagatedBody, } default: return nil @@ -178,6 +191,13 @@ type TemplateData struct { UntagOp string UpdateTagsFunc string UpdateTagsIgnoreSystem bool + WaitForPropagation bool + WaitTagsPropagatedFunc string + WaitContinuousOccurence int + WaitDelay string + WaitMinTimeout string + WaitPollInterval string + WaitTimeout string // The following are specific to writing import paths in the `headerBody`; // to include the package, set the corresponding field's value to true @@ -189,6 +209,7 @@ type TemplateData struct { SkipServiceImp bool SkipTypesImp bool TfResourcePkg bool + TimePkg bool IsDefaultListTags bool IsDefaultUpdateTags bool @@ -276,7 +297,8 @@ func main() { NamesPkg: *updateTags && !*skipNamesImp, SkipServiceImp: *skipServiceImp, SkipTypesImp: *skipTypesImp, - TfResourcePkg: *getTag, + TfResourcePkg: (*getTag || *waitForPropagation), + TimePkg: *waitForPropagation, CreateTagsFunc: createTagsFunc, GetTagFunc: *getTagFunc, @@ -315,6 +337,13 @@ func main() { UntagOp: *untagOp, UpdateTagsFunc: *updateTagsFunc, UpdateTagsIgnoreSystem: !*updateTagsNoIgnoreSystem, + WaitForPropagation: *waitForPropagation, + WaitTagsPropagatedFunc: *waitTagsPropagatedFunc, + WaitContinuousOccurence: *waitContinuousOccurence, + WaitDelay: formatDuration(*waitDelay), + WaitMinTimeout: formatDuration(*waitMinTimeout), + WaitPollInterval: formatDuration(*waitPollInterval), + WaitTimeout: formatDuration(*waitTimeout), IsDefaultListTags: *listTagsFunc == defaultListTagsFunc, IsDefaultUpdateTags: *updateTagsFunc == defaultUpdateTagsFunc, @@ -366,6 +395,12 @@ func main() { } } + if *waitForPropagation { + if err := d.WriteTemplate("waittagspropagated", templateBody.waitTagsPropagated, templateData); err != nil { + g.Fatalf("generating file (%s): %s", filename, err) + } + } + if err := d.Write(); err != nil { g.Fatalf("generating file (%s): %s", filename, err) } @@ -376,3 +411,29 @@ func toSnakeCase(str string) string { result = regexp.MustCompile("([a-z0-9])([A-Z])").ReplaceAllString(result, "${1}_${2}") return strings.ToLower(result) } + +func formatDuration(d time.Duration) string { + if d == 0 { + return "" + } + + var buf []string + if h := d.Hours(); h >= 1 { + buf = append(buf, fmt.Sprintf("%d * time.Hour", int64(h))) + d = d - time.Duration(int64(h)*int64(time.Hour)) + } + if m := d.Minutes(); m >= 1 { + buf = append(buf, fmt.Sprintf("%d * time.Minute", int64(m))) + d = d - time.Duration(int64(m)*int64(time.Minute)) + } + if s := d.Seconds(); s >= 1 { + buf = append(buf, fmt.Sprintf("%d * time.Second", int64(s))) + d = d - time.Duration(int64(s)*int64(time.Second)) + } + if ms := d.Milliseconds(); ms >= 1 { + buf = append(buf, fmt.Sprintf("%d * time.Millisecond", int64(ms))) + } + // Ignoring anything below milliseconds + + return strings.Join(buf, " + ") +} diff --git a/internal/generate/tags/templates/v1/header_body.tmpl b/internal/generate/tags/templates/v1/header_body.tmpl index 2e4249e1dd7..1a51de1a5f6 100644 --- a/internal/generate/tags/templates/v1/header_body.tmpl +++ b/internal/generate/tags/templates/v1/header_body.tmpl @@ -6,6 +6,9 @@ import ( {{- if .FmtPkg }} "fmt" {{- end }} + {{- if .TimePkg }} + "time" + {{- end }} "github.com/aws/aws-sdk-go/aws" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" diff --git a/internal/generate/tags/templates/v1/update_tags_body.tmpl b/internal/generate/tags/templates/v1/update_tags_body.tmpl index f9f651db5f5..7ff616acdc8 100644 --- a/internal/generate/tags/templates/v1/update_tags_body.tmpl +++ b/internal/generate/tags/templates/v1/update_tags_body.tmpl @@ -140,6 +140,14 @@ func {{ .UpdateTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifi {{- end }} + {{ if .WaitForPropagation }} + if len(removedTags) > 0 || len(updatedTags) > 0 { + if err := {{ .WaitTagsPropagatedFunc }}(ctx, conn, identifier, newTags); err != nil { + return fmt.Errorf("waiting for resource (%s) tag propagation: %w", identifier, err) + } + } + {{- end }} + return nil } diff --git a/internal/generate/tags/templates/v1/v1.go b/internal/generate/tags/templates/v1/v1.go index f1035df9d50..7e933e491ed 100644 --- a/internal/generate/tags/templates/v1/v1.go +++ b/internal/generate/tags/templates/v1/v1.go @@ -24,3 +24,6 @@ var ServiceTagsSliceBody string //go:embed update_tags_body.tmpl var UpdateTagsBody string + +//go:embed wait_tags_propagated_body.tmpl +var WaitTagsPropagatedBody string diff --git a/internal/generate/tags/templates/v1/wait_tags_propagated_body.tmpl b/internal/generate/tags/templates/v1/wait_tags_propagated_body.tmpl new file mode 100644 index 00000000000..52e8d005ec5 --- /dev/null +++ b/internal/generate/tags/templates/v1/wait_tags_propagated_body.tmpl @@ -0,0 +1,34 @@ +// {{ .WaitTagsPropagatedFunc }} waits for {{ .ServicePackage }} service tags to be propagated. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func {{ .WaitTagsPropagatedFunc }}(ctx context.Context, conn {{ .ClientType }}, id string, tags tftags.KeyValueTags) error { + checkFunc := func() (bool, error) { + output, err := listTags(ctx, conn, id) + + if tfresource.NotFound(err) { + return false, nil + } + + if err != nil { + return false, err + } + + return output.Equal(tags), nil + } + opts := tfresource.WaitOpts{ + {{- if ne .WaitContinuousOccurence 0 }} + ContinuousTargetOccurence: {{ .WaitContinuousOccurence }}, + {{- end }} + {{- if ne .WaitDelay "" }} + Delay: {{ .WaitDelay }}, + {{- end }} + {{- if ne .WaitMinTimeout "" }} + MinTimeout: {{ .WaitMinTimeout }}, + {{- end }} + {{- if ne .WaitPollInterval "" }} + PollInterval: {{ .WaitPollInterval }}, + {{- end }} + } + + return tfresource.WaitUntil(ctx, {{ .WaitTimeout }}, checkFunc, opts) +} diff --git a/internal/generate/tags/templates/v2/header_body.tmpl b/internal/generate/tags/templates/v2/header_body.tmpl index 248d7a0a444..3ea1639da38 100644 --- a/internal/generate/tags/templates/v2/header_body.tmpl +++ b/internal/generate/tags/templates/v2/header_body.tmpl @@ -6,6 +6,9 @@ import ( {{- if .FmtPkg }} "fmt" {{- end }} + {{- if .TimePkg }} + "time" + {{- end }} "github.com/aws/aws-sdk-go-v2/aws" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" diff --git a/internal/generate/tags/templates/v2/update_tags_body.tmpl b/internal/generate/tags/templates/v2/update_tags_body.tmpl index a5fb3cb1639..109af549783 100644 --- a/internal/generate/tags/templates/v2/update_tags_body.tmpl +++ b/internal/generate/tags/templates/v2/update_tags_body.tmpl @@ -140,6 +140,14 @@ func {{ .UpdateTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifi {{- end }} + {{ if .WaitForPropagation }} + if len(removedTags) > 0 || len(updatedTags) > 0 { + if err := {{ .WaitTagsPropagatedFunc }}(ctx, conn, identifier, newTags); err != nil { + return fmt.Errorf("waiting for resource (%s) tag propagation: %w", identifier, err) + } + } + {{- end }} + return nil } diff --git a/internal/generate/tags/templates/v2/v2.go b/internal/generate/tags/templates/v2/v2.go index 6f149b71a58..c148caaaf66 100644 --- a/internal/generate/tags/templates/v2/v2.go +++ b/internal/generate/tags/templates/v2/v2.go @@ -30,3 +30,6 @@ var ServiceTagsSliceBody string //go:embed update_tags_body.tmpl var UpdateTagsBody string + +//go:embed wait_tags_propagated_body.tmpl +var WaitTagsPropagatedBody string diff --git a/internal/generate/tags/templates/v2/wait_tags_propagated_body.tmpl b/internal/generate/tags/templates/v2/wait_tags_propagated_body.tmpl new file mode 100644 index 00000000000..52e8d005ec5 --- /dev/null +++ b/internal/generate/tags/templates/v2/wait_tags_propagated_body.tmpl @@ -0,0 +1,34 @@ +// {{ .WaitTagsPropagatedFunc }} waits for {{ .ServicePackage }} service tags to be propagated. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func {{ .WaitTagsPropagatedFunc }}(ctx context.Context, conn {{ .ClientType }}, id string, tags tftags.KeyValueTags) error { + checkFunc := func() (bool, error) { + output, err := listTags(ctx, conn, id) + + if tfresource.NotFound(err) { + return false, nil + } + + if err != nil { + return false, err + } + + return output.Equal(tags), nil + } + opts := tfresource.WaitOpts{ + {{- if ne .WaitContinuousOccurence 0 }} + ContinuousTargetOccurence: {{ .WaitContinuousOccurence }}, + {{- end }} + {{- if ne .WaitDelay "" }} + Delay: {{ .WaitDelay }}, + {{- end }} + {{- if ne .WaitMinTimeout "" }} + MinTimeout: {{ .WaitMinTimeout }}, + {{- end }} + {{- if ne .WaitPollInterval "" }} + PollInterval: {{ .WaitPollInterval }}, + {{- end }} + } + + return tfresource.WaitUntil(ctx, {{ .WaitTimeout }}, checkFunc, opts) +} diff --git a/internal/service/kms/external_key.go b/internal/service/kms/external_key.go index bd8db713a03..2f46ef1bc38 100644 --- a/internal/service/kms/external_key.go +++ b/internal/service/kms/external_key.go @@ -195,7 +195,7 @@ func resourceExternalKeyCreate(ctx context.Context, d *schema.ResourceData, meta } if tags := KeyValueTags(ctx, getTagsIn(ctx)); len(tags) > 0 { - if err := WaitTagsPropagated(ctx, conn, d.Id(), tags); err != nil { + if err := waitTagsPropagated(ctx, conn, d.Id(), tags); err != nil { return sdkdiag.AppendErrorf(diags, "waiting for KMS External Key (%s) tag propagation: %s", d.Id(), err) } } @@ -304,12 +304,6 @@ func resourceExternalKeyUpdate(ctx context.Context, d *schema.ResourceData, meta } } - if d.HasChange("tags_all") { - if err := WaitTagsPropagated(ctx, conn, d.Id(), tftags.New(ctx, d.Get("tags_all").(map[string]interface{}))); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for KMS External Key (%s) tag propagation: %s", d.Id(), err) - } - } - return append(diags, resourceExternalKeyRead(ctx, d, meta)...) } diff --git a/internal/service/kms/external_key_test.go b/internal/service/kms/external_key_test.go index 4efd8c547fb..a73b5c757fb 100644 --- a/internal/service/kms/external_key_test.go +++ b/internal/service/kms/external_key_test.go @@ -58,7 +58,6 @@ func TestAccKMSExternalKey_basic(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, }, @@ -114,7 +113,6 @@ func TestAccKMSExternalKey_multiRegion(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, }, @@ -147,7 +145,6 @@ func TestAccKMSExternalKey_deletionWindowInDays(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, { @@ -188,7 +185,6 @@ func TestAccKMSExternalKey_description(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, { @@ -229,7 +225,6 @@ func TestAccKMSExternalKey_enabled(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, { @@ -279,7 +274,6 @@ func TestAccKMSExternalKey_keyMaterialBase64(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, { @@ -323,7 +317,6 @@ func TestAccKMSExternalKey_policy(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, { @@ -366,7 +359,6 @@ func TestAccKMSExternalKey_policyBypass(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, }, @@ -400,7 +392,6 @@ func TestAccKMSExternalKey_tags(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, { @@ -422,6 +413,23 @@ func TestAccKMSExternalKey_tags(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, + { + Config: testAccExternalKeyConfig_tags0(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckExternalKeyExists(ctx, resourceName, &key3), + testAccCheckExternalKeyNotRecreated(&key2, &key3), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", + "deletion_window_in_days", + }, + }, }, }) } @@ -455,7 +463,6 @@ func TestAccKMSExternalKey_validTo(t *testing.T) { ImportStateVerifyIgnore: []string{ "bypass_policy_lockout_safety_check", "deletion_window_in_days", - "key_material_base64", }, }, { @@ -704,6 +711,15 @@ resource "aws_kms_external_key" "test" { `, rName, tagKey1, tagValue1, tagKey2, tagValue2) } +func testAccExternalKeyConfig_tags0(rName string) string { + return fmt.Sprintf(` +resource "aws_kms_external_key" "test" { + description = %[1]q + deletion_window_in_days = 7 +} +`, rName) +} + func testAccExternalKeyConfig_validTo(rName, validTo string) string { return fmt.Sprintf(` # ACCEPTANCE TESTING ONLY -- NEVER EXPOSE YOUR KEY MATERIAL diff --git a/internal/service/kms/generate.go b/internal/service/kms/generate.go index 0f789dffed8..da2cdd33aae 100644 --- a/internal/service/kms/generate.go +++ b/internal/service/kms/generate.go @@ -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 +//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/servicepackage/main.go // ONLY generate directives and package declaration! Do not add anything else to this file. diff --git a/internal/service/kms/key.go b/internal/service/kms/key.go index 42ad993c366..ed15821c52e 100644 --- a/internal/service/kms/key.go +++ b/internal/service/kms/key.go @@ -185,7 +185,7 @@ func resourceKeyCreate(ctx context.Context, d *schema.ResourceData, meta interfa } if tags := KeyValueTags(ctx, getTagsIn(ctx)); len(tags) > 0 { - if err := WaitTagsPropagated(ctx, conn, d.Id(), tags); err != nil { + if err := waitTagsPropagated(ctx, conn, d.Id(), tags); err != nil { return sdkdiag.AppendErrorf(diags, "waiting for KMS Key (%s) tag propagation: %s", d.Id(), err) } } @@ -271,12 +271,6 @@ func resourceKeyUpdate(ctx context.Context, d *schema.ResourceData, meta interfa } } - if d.HasChange("tags_all") { - if err := WaitTagsPropagated(ctx, conn, d.Id(), tftags.New(ctx, d.Get("tags_all").(map[string]interface{}))); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for KMS Key (%s) tag propagation: %s", d.Id(), err) - } - } - return append(diags, resourceKeyRead(ctx, d, meta)...) } diff --git a/internal/service/kms/key_test.go b/internal/service/kms/key_test.go index f7ca5535560..c3beb01d3ba 100644 --- a/internal/service/kms/key_test.go +++ b/internal/service/kms/key_test.go @@ -492,6 +492,19 @@ func TestAccKMSKey_tags(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, + { + Config: testAccKeyConfig_tags0(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeyExists(ctx, resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, + }, }, }) } @@ -1052,3 +1065,11 @@ resource "aws_kms_key" "test" { } `, rName, tagKey1, tagValue1, tagKey2, tagValue2) } + +func testAccKeyConfig_tags0(rName string) string { + return fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = %[1]q +} +`, rName) +} diff --git a/internal/service/kms/replica_external_key.go b/internal/service/kms/replica_external_key.go index 1bfa7ebb565..5255d185105 100644 --- a/internal/service/kms/replica_external_key.go +++ b/internal/service/kms/replica_external_key.go @@ -195,7 +195,7 @@ func resourceReplicaExternalKeyCreate(ctx context.Context, d *schema.ResourceDat } if tags := KeyValueTags(ctx, getTagsIn(ctx)); len(tags) > 0 { - if err := WaitTagsPropagated(ctx, conn, d.Id(), tags); err != nil { + if err := waitTagsPropagated(ctx, conn, d.Id(), tags); err != nil { return sdkdiag.AppendErrorf(diags, "waiting for KMS Replica External Key (%s) tag propagation: %s", d.Id(), err) } } @@ -305,12 +305,6 @@ func resourceReplicaExternalKeyUpdate(ctx context.Context, d *schema.ResourceDat } } - if d.HasChange("tags_all") { - if err := WaitTagsPropagated(ctx, conn, d.Id(), tftags.New(ctx, d.Get("tags_all").(map[string]interface{}))); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for KMS Replica External Key (%s) tag propagation: %s", d.Id(), err) - } - } - return append(diags, resourceReplicaExternalKeyRead(ctx, d, meta)...) } diff --git a/internal/service/kms/replica_external_key_test.go b/internal/service/kms/replica_external_key_test.go index 056439e0cbc..4eeb179bb5f 100644 --- a/internal/service/kms/replica_external_key_test.go +++ b/internal/service/kms/replica_external_key_test.go @@ -216,6 +216,23 @@ func TestAccKMSReplicaExternalKey_tags(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, + { + Config: testAccReplicaExternalKeyConfig_tags0(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeyExists(ctx, resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", + "deletion_window_in_days", + "key_material_base64", + }, + }, }, }) } @@ -317,7 +334,7 @@ resource "aws_kms_external_key" "test" { } resource "aws_kms_replica_external_key" "test" { - description = %[2]q + description = "%[1]s-Replica" enabled = true primary_key_arn = aws_kms_external_key.test.arn @@ -352,7 +369,7 @@ resource "aws_kms_external_key" "test" { } resource "aws_kms_replica_external_key" "test" { - description = %[2]q + description = "%[1]s-Replica" enabled = true primary_key_arn = aws_kms_external_key.test.arn @@ -367,3 +384,30 @@ resource "aws_kms_replica_external_key" "test" { } `, rName, tagKey1, tagValue1, tagKey2, tagValue2)) } + +func testAccReplicaExternalKeyConfig_tags0(rName string) string { + return acctest.ConfigCompose(acctest.ConfigAlternateRegionProvider(), fmt.Sprintf(` +# ACCEPTANCE TESTING ONLY -- NEVER EXPOSE YOUR KEY MATERIAL +resource "aws_kms_external_key" "test" { + provider = awsalternate + + description = %[1]q + multi_region = true + enabled = true + + key_material_base64 = "Wblj06fduthWggmsT0cLVoIMOkeLbc2kVfMud77i/JY=" + + deletion_window_in_days = 7 +} + +resource "aws_kms_replica_external_key" "test" { + description = "%[1]s-Replica" + enabled = true + primary_key_arn = aws_kms_external_key.test.arn + + key_material_base64 = "Wblj06fduthWggmsT0cLVoIMOkeLbc2kVfMud77i/JY=" + + deletion_window_in_days = 7 +} +`, rName)) +} diff --git a/internal/service/kms/replica_key.go b/internal/service/kms/replica_key.go index 49b3454ad9d..cf4b43b981f 100644 --- a/internal/service/kms/replica_key.go +++ b/internal/service/kms/replica_key.go @@ -167,7 +167,7 @@ func resourceReplicaKeyCreate(ctx context.Context, d *schema.ResourceData, meta } if tags := KeyValueTags(ctx, getTagsIn(ctx)); len(tags) > 0 { - if err := WaitTagsPropagated(ctx, conn, d.Id(), tags); err != nil { + if err := waitTagsPropagated(ctx, conn, d.Id(), tags); err != nil { return sdkdiag.AppendErrorf(diags, "waiting for KMS Replica Key (%s) tag propagation: %s", d.Id(), err) } } @@ -255,12 +255,6 @@ func resourceReplicaKeyUpdate(ctx context.Context, d *schema.ResourceData, meta } } - if d.HasChange("tags_all") { - if err := WaitTagsPropagated(ctx, conn, d.Id(), tftags.New(ctx, d.Get("tags_all").(map[string]interface{}))); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for KMS Replica Key (%s) tag propagation: %s", d.Id(), err) - } - } - return append(diags, resourceReplicaKeyRead(ctx, d, meta)...) } diff --git a/internal/service/kms/replica_key_test.go b/internal/service/kms/replica_key_test.go index 181eb053c81..0e645571f3b 100644 --- a/internal/service/kms/replica_key_test.go +++ b/internal/service/kms/replica_key_test.go @@ -203,10 +203,13 @@ func TestAccKMSReplicaKey_tags(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deletion_window_in_days", + "bypass_policy_lockout_safety_check", + }, }, { Config: testAccReplicaKeyConfig_tags2(rName, "key1", "value1updated", "key2", "value2"), @@ -225,6 +228,22 @@ func TestAccKMSReplicaKey_tags(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, + { + Config: testAccReplicaKeyConfig_tags0(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeyExists(ctx, resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deletion_window_in_days", + "bypass_policy_lockout_safety_check", + }, + }, }, }) } @@ -371,6 +390,26 @@ resource "aws_kms_replica_key" "test" { `, rName, tagKey1, tagValue1, tagKey2, tagValue2)) } +func testAccReplicaKeyConfig_tags0(rName string) string { + return acctest.ConfigCompose(acctest.ConfigAlternateRegionProvider(), fmt.Sprintf(` +resource "aws_kms_key" "test" { + provider = awsalternate + + description = %[1]q + multi_region = true + + deletion_window_in_days = 7 +} + +resource "aws_kms_replica_key" "test" { + description = %[1]q + primary_key_arn = aws_kms_key.test.arn + + deletion_window_in_days = 7 +} +`, rName)) +} + func testAccReplicaKeyConfig_two(rName string) string { return acctest.ConfigCompose(acctest.ConfigMultipleRegionProvider(3), fmt.Sprintf(` resource "aws_kms_key" "test" { diff --git a/internal/service/kms/tags_gen.go b/internal/service/kms/tags_gen.go index 53962c5d70a..33201aed10e 100644 --- a/internal/service/kms/tags_gen.go +++ b/internal/service/kms/tags_gen.go @@ -4,12 +4,16 @@ package kms import ( "context" "fmt" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/kms" "github.com/aws/aws-sdk-go/service/kms/kmsiface" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-provider-aws/internal/conns" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/types" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -24,6 +28,13 @@ func listTags(ctx context.Context, conn kmsiface.KMSAPI, identifier string) (tft output, err := conn.ListResourceTagsWithContext(ctx, input) + if tfawserr.ErrCodeEquals(err, "NotFoundException") { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + if err != nil { return tftags.New(ctx, nil), err } @@ -132,6 +143,12 @@ func updateTags(ctx context.Context, conn kmsiface.KMSAPI, identifier string, ol } } + if len(removedTags) > 0 || len(updatedTags) > 0 { + if err := waitTagsPropagated(ctx, conn, identifier, newTags); err != nil { + return fmt.Errorf("waiting for resource (%s) tag propagation: %w", identifier, err) + } + } + return nil } @@ -140,3 +157,28 @@ func updateTags(ctx context.Context, conn kmsiface.KMSAPI, identifier string, ol func (p *servicePackage) UpdateTags(ctx context.Context, meta any, identifier string, oldTags, newTags any) error { return updateTags(ctx, meta.(*conns.AWSClient).KMSConn(ctx), identifier, oldTags, newTags) } + +// waitTagsPropagated waits for kms service tags to be propagated. +// The identifier is typically the Amazon Resource Name (ARN), although +// it may also be a different identifier depending on the service. +func waitTagsPropagated(ctx context.Context, conn kmsiface.KMSAPI, id string, tags tftags.KeyValueTags) error { + checkFunc := func() (bool, error) { + output, err := listTags(ctx, conn, id) + + if tfresource.NotFound(err) { + return false, nil + } + + if err != nil { + return false, err + } + + return output.Equal(tags), nil + } + opts := tfresource.WaitOpts{ + ContinuousTargetOccurence: 5, + MinTimeout: 1 * time.Second, + } + + return tfresource.WaitUntil(ctx, 10*time.Minute, checkFunc, opts) +} diff --git a/internal/service/kms/wait.go b/internal/service/kms/wait.go index e3475da4cea..09307585925 100644 --- a/internal/service/kms/wait.go +++ b/internal/service/kms/wait.go @@ -9,10 +9,8 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/kms" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" awspolicy "github.com/hashicorp/awspolicyequivalence" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" - tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) @@ -205,28 +203,6 @@ func WaitKeyValidToPropagated(ctx context.Context, conn *kms.KMS, id string, val return tfresource.WaitUntil(ctx, KeyValidToPropagationTimeout, checkFunc, opts) } -func WaitTagsPropagated(ctx context.Context, conn *kms.KMS, id string, tags tftags.KeyValueTags) error { - checkFunc := func() (bool, error) { - output, err := listTags(ctx, conn, id) - - if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { - return false, nil - } - - if err != nil { - return false, err - } - - return output.Equal(tags), nil - } - opts := tfresource.WaitOpts{ - ContinuousTargetOccurence: 5, - MinTimeout: 1 * time.Second, - } - - return tfresource.WaitUntil(ctx, KeyTagsPropagationTimeout, checkFunc, opts) -} - func WaitReplicaExternalKeyCreated(ctx context.Context, conn *kms.KMS, id string) (*kms.KeyMetadata, error) { stateConf := &retry.StateChangeConf{ Pending: []string{kms.KeyStateCreating}, From 378a9bcf2fe56b043425f35e1da64f0f52db8f9f Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 4 Jul 2023 17:06:36 -0700 Subject: [PATCH 2/2] Adds CHANGELOG entry --- .changelog/32371.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .changelog/32371.txt diff --git a/.changelog/32371.txt b/.changelog/32371.txt new file mode 100644 index 00000000000..2de6f5655fb --- /dev/null +++ b/.changelog/32371.txt @@ -0,0 +1,15 @@ +```release-note:bug +resource/aws_kms_external_key: Correctly remove all tags +``` + +```release-note:bug +resource/aws_kms_key: Correctly remove all tags +``` + +```release-note:bug +resource/aws_kms_replica_external_key: Correctly remove all tags +``` + +```release-note:bug +resource/aws_kms_replica_key: Correctly remove all tags +```