From 39b240b2a330edd682e07f339fc5a8e7eb749e5f Mon Sep 17 00:00:00 2001 From: Anthony Wat Date: Sun, 3 Mar 2024 22:34:08 -0500 Subject: [PATCH 01/30] feat: Mark name arg as optional to match AWS API for aws_ram_resource_share data source --- .changelog/36062.txt | 3 ++ .../service/ram/resource_share_data_source.go | 12 +++++--- .../ram/resource_share_data_source_test.go | 30 +++++++++++++++++++ .../docs/d/ram_resource_share.html.markdown | 5 +--- 4 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 .changelog/36062.txt diff --git a/.changelog/36062.txt b/.changelog/36062.txt new file mode 100644 index 000000000000..904e6ddd24d3 --- /dev/null +++ b/.changelog/36062.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +data-source/aws_ram_resource_share: Mark `name` argument as optional as per AWS API +``` \ No newline at end of file diff --git a/internal/service/ram/resource_share_data_source.go b/internal/service/ram/resource_share_data_source.go index 7788559d76cc..b55d7090c616 100644 --- a/internal/service/ram/resource_share_data_source.go +++ b/internal/service/ram/resource_share_data_source.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/flex" tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) // @SDKDataSource("aws_ram_resource_share") @@ -48,7 +49,8 @@ func dataSourceResourceShare() *schema.Resource { }, "name": { Type: schema.TypeString, - Required: true, + Optional: true, + Computed: true, }, "owning_account_id": { Type: schema.TypeString, @@ -84,13 +86,15 @@ func dataSourceResourceShareRead(ctx context.Context, d *schema.ResourceData, me var diags diag.Diagnostics conn := meta.(*conns.AWSClient).RAMConn(ctx) - name := d.Get("name").(string) resourceOwner := d.Get("resource_owner").(string) inputG := &ram.GetResourceSharesInput{ - Name: aws.String(name), ResourceOwner: aws.String(resourceOwner), } + if v, ok := d.GetOk("name"); ok { + inputG.Name = aws.String(v.(string)) + } + if v, ok := d.GetOk("filter"); ok && v.(*schema.Set).Len() > 0 { inputG.TagFilters = expandTagFilters(v.(*schema.Set).List()) } @@ -102,7 +106,7 @@ func dataSourceResourceShareRead(ctx context.Context, d *schema.ResourceData, me share, err := findResourceShare(ctx, conn, inputG) if err != nil { - return sdkdiag.AppendErrorf(diags, "reading RAM Resource Share (%s): %s", name, err) + return sdkdiag.AppendFromErr(diags, tfresource.SingularDataSourceFindError("RAM Resource Share", err)) } arn := aws.StringValue(share.ResourceShareArn) diff --git a/internal/service/ram/resource_share_data_source_test.go b/internal/service/ram/resource_share_data_source_test.go index a59b2d5e3028..f08f2d1408d9 100644 --- a/internal/service/ram/resource_share_data_source_test.go +++ b/internal/service/ram/resource_share_data_source_test.go @@ -53,6 +53,15 @@ func TestAccRAMResourceShareDataSource_tags(t *testing.T) { Config: testAccResourceShareDataSourceConfig_tags(rName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrPair(datasourceName, "id", resourceName, "id"), + resource.TestCheckResourceAttrPair(datasourceName, "name", resourceName, "name"), + resource.TestCheckResourceAttrPair(datasourceName, "tags.%", resourceName, "tags.%"), + ), + }, + { + Config: testAccResourceShareDataSourceConfig_tagsWithoutName(rName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrPair(datasourceName, "id", resourceName, "id"), + resource.TestCheckResourceAttrPair(datasourceName, "name", resourceName, "name"), resource.TestCheckResourceAttrPair(datasourceName, "tags.%", resourceName, "tags.%"), ), }, @@ -145,6 +154,27 @@ data "aws_ram_resource_share" "test" { `, rName) } +func testAccResourceShareDataSourceConfig_tagsWithoutName(rName string) string { + return fmt.Sprintf(` +resource "aws_ram_resource_share" "test" { + name = %[1]q + + tags = { + Name = %[1]q + } +} + +data "aws_ram_resource_share" "test" { + resource_owner = "SELF" + + filter { + name = "Name" + values = [%[1]q] + } +} +`, rName) +} + func testAccResourceShareDataSourceConfig_resources(rName string) string { return acctest.ConfigCompose(acctest.ConfigVPCWithSubnets(rName, 1), fmt.Sprintf(` resource "aws_ram_resource_share" "test" { diff --git a/website/docs/d/ram_resource_share.html.markdown b/website/docs/d/ram_resource_share.html.markdown index f30d3e8bd5a8..0d4b697db595 100644 --- a/website/docs/d/ram_resource_share.html.markdown +++ b/website/docs/d/ram_resource_share.html.markdown @@ -23,9 +23,7 @@ data "aws_ram_resource_share" "example" { ```terraform data "aws_ram_resource_share" "tag_filter" { - name = "MyResourceName" resource_owner = "SELF" - filter { name = "NameOfTag" values = ["exampleNameTagValue"] @@ -37,9 +35,8 @@ data "aws_ram_resource_share" "tag_filter" { This data source supports the following arguments: -* `name` - (Required) Name of the resource share to retrieve. +* `name` - (Optional) Name of the resource share to retrieve. * `resource_owner` (Required) Owner of the resource share. Valid values are `SELF` or `OTHER-ACCOUNTS`. - * `resource_share_status` (Optional) Specifies that you want to retrieve details of only those resource shares that have this status. Valid values are `PENDING`, `ACTIVE`, `FAILED`, `DELETING`, and `DELETED`. * `filter` - (Optional) Filter used to scope the list e.g., by tags. See [related docs] (https://docs.aws.amazon.com/ram/latest/APIReference/API_TagFilter.html). * `name` - (Required) Name of the tag key to filter on. From 6653ae0a925e934768d95944e820398ebf0aa952 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:20:51 -0400 Subject: [PATCH 02/30] Tweak CHANGELOG entry. --- .changelog/36062.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/36062.txt b/.changelog/36062.txt index 904e6ddd24d3..d1ce2e6b443f 100644 --- a/.changelog/36062.txt +++ b/.changelog/36062.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -data-source/aws_ram_resource_share: Mark `name` argument as optional as per AWS API +data-source/aws_ram_resource_share: `name` is Optional ``` \ No newline at end of file From af043471914bab4a7054eed411043d74a087584a Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:31:50 -0400 Subject: [PATCH 03/30] Tweak 'testAccResourceShareDataSourceConfig_tagsWithoutName'. --- internal/service/ram/resource_share_data_source_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/ram/resource_share_data_source_test.go b/internal/service/ram/resource_share_data_source_test.go index f08f2d1408d9..8289053f38ac 100644 --- a/internal/service/ram/resource_share_data_source_test.go +++ b/internal/service/ram/resource_share_data_source_test.go @@ -169,7 +169,7 @@ data "aws_ram_resource_share" "test" { filter { name = "Name" - values = [%[1]q] + values = [aws_ram_resource_share.test.tags["Name"]] } } `, rName) From 96179e527e3eb30d8c2408fa8578957658df0999 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:34:32 -0400 Subject: [PATCH 04/30] Acceptance test output: % make testacc TESTARGS='-run=TestAccRAMResourceShareDataSource_' PKG=ram ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.21.8 test ./internal/service/ram/... -v -count 1 -parallel 2 -run=TestAccRAMResourceShareDataSource_ -timeout 360m === RUN TestAccRAMResourceShareDataSource_basic === PAUSE TestAccRAMResourceShareDataSource_basic === RUN TestAccRAMResourceShareDataSource_tags === PAUSE TestAccRAMResourceShareDataSource_tags === RUN TestAccRAMResourceShareDataSource_resources === PAUSE TestAccRAMResourceShareDataSource_resources === RUN TestAccRAMResourceShareDataSource_status === PAUSE TestAccRAMResourceShareDataSource_status === CONT TestAccRAMResourceShareDataSource_basic === CONT TestAccRAMResourceShareDataSource_resources --- PASS: TestAccRAMResourceShareDataSource_basic (22.08s) === CONT TestAccRAMResourceShareDataSource_status --- PASS: TestAccRAMResourceShareDataSource_resources (31.19s) === CONT TestAccRAMResourceShareDataSource_tags --- PASS: TestAccRAMResourceShareDataSource_status (20.36s) --- PASS: TestAccRAMResourceShareDataSource_tags (33.33s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/ram 71.779s From f052992dceac59afcd47921aba228589305a6006 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:41:22 -0400 Subject: [PATCH 05/30] ram: Reduce visibility. --- internal/service/ram/exports_test.go | 13 +++++++++++++ internal/service/ram/principal_association.go | 17 ++++++++--------- internal/service/ram/resource_association.go | 4 ++-- internal/service/ram/resource_share.go | 2 +- internal/service/ram/resource_share_accepter.go | 4 ++-- .../service/ram/resource_share_data_source.go | 2 +- internal/service/ram/service_package_gen.go | 15 ++++++++++----- .../service/ram/sharing_with_organization.go | 4 ++-- internal/service/ram/sweep.go | 2 +- 9 files changed, 40 insertions(+), 23 deletions(-) create mode 100644 internal/service/ram/exports_test.go diff --git a/internal/service/ram/exports_test.go b/internal/service/ram/exports_test.go new file mode 100644 index 000000000000..9e95fdca09f7 --- /dev/null +++ b/internal/service/ram/exports_test.go @@ -0,0 +1,13 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package ram + +// Exports for use in tests only. +var ( + ResourcePrincipalAssociation = resourcePrincipalAssociation + ResourceResourceAssociation = resourceResourceAssociation + ResourceResourceShare = resourceResourceShare + ResourceResourceShareAccepter = resourceResourceShareAccepter + ResourceSharingWithOrganization = resourceSharingWithOrganization +) diff --git a/internal/service/ram/principal_association.go b/internal/service/ram/principal_association.go index c19c8b687136..f9001b7478d3 100644 --- a/internal/service/ram/principal_association.go +++ b/internal/service/ram/principal_association.go @@ -22,8 +22,8 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -// @SDKResource("aws_ram_principal_association") -func ResourcePrincipalAssociation() *schema.Resource { +// @SDKResource("aws_ram_principal_association", name="Principal Association") +func resourcePrincipalAssociation() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourcePrincipalAssociationCreate, ReadWithoutTimeout: resourcePrincipalAssociationRead, @@ -34,13 +34,6 @@ func ResourcePrincipalAssociation() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "resource_share_arn": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: verify.ValidARN, - }, - "principal": { Type: schema.TypeString, Required: true, @@ -50,6 +43,12 @@ func ResourcePrincipalAssociation() *schema.Resource { verify.ValidARN, ), }, + "resource_share_arn": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, + }, }, } } diff --git a/internal/service/ram/resource_association.go b/internal/service/ram/resource_association.go index 7a69c40ae59b..f5bb300fc9b3 100644 --- a/internal/service/ram/resource_association.go +++ b/internal/service/ram/resource_association.go @@ -22,8 +22,8 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -// @SDKResource("aws_ram_resource_association") -func ResourceResourceAssociation() *schema.Resource { +// @SDKResource("aws_ram_resource_association", name="Resource Association") +func resourceResourceAssociation() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceResourceAssociationCreate, ReadWithoutTimeout: resourceResourceAssociationRead, diff --git a/internal/service/ram/resource_share.go b/internal/service/ram/resource_share.go index 53fab1f46572..f77345e38f1c 100644 --- a/internal/service/ram/resource_share.go +++ b/internal/service/ram/resource_share.go @@ -26,7 +26,7 @@ import ( // @SDKResource("aws_ram_resource_share", name="Resource Share") // @Tags(identifierAttribute="id") -func ResourceResourceShare() *schema.Resource { +func resourceResourceShare() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceResourceShareCreate, ReadWithoutTimeout: resourceResourceShareRead, diff --git a/internal/service/ram/resource_share_accepter.go b/internal/service/ram/resource_share_accepter.go index 0cb35d562701..5c0473a78139 100644 --- a/internal/service/ram/resource_share_accepter.go +++ b/internal/service/ram/resource_share_accepter.go @@ -22,8 +22,8 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -// @SDKResource("aws_ram_resource_share_accepter") -func ResourceResourceShareAccepter() *schema.Resource { +// @SDKResource("aws_ram_resource_share_accepter", name="Resource Share Accepter") +func resourceResourceShareAccepter() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceResourceShareAccepterCreate, ReadWithoutTimeout: resourceResourceShareAccepterRead, diff --git a/internal/service/ram/resource_share_data_source.go b/internal/service/ram/resource_share_data_source.go index b55d7090c616..4d7cca4c8a4d 100644 --- a/internal/service/ram/resource_share_data_source.go +++ b/internal/service/ram/resource_share_data_source.go @@ -19,7 +19,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -// @SDKDataSource("aws_ram_resource_share") +// @SDKDataSource("aws_ram_resource_share", name="Resource Shared") // @Tags func dataSourceResourceShare() *schema.Resource { return &schema.Resource{ diff --git a/internal/service/ram/service_package_gen.go b/internal/service/ram/service_package_gen.go index c770ab3c8239..4599d4fc8443 100644 --- a/internal/service/ram/service_package_gen.go +++ b/internal/service/ram/service_package_gen.go @@ -28,6 +28,7 @@ func (p *servicePackage) SDKDataSources(ctx context.Context) []*types.ServicePac { Factory: dataSourceResourceShare, TypeName: "aws_ram_resource_share", + Name: "Resource Shared", Tags: &types.ServicePackageResourceTags{}, }, } @@ -36,15 +37,17 @@ func (p *servicePackage) SDKDataSources(ctx context.Context) []*types.ServicePac func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePackageSDKResource { return []*types.ServicePackageSDKResource{ { - Factory: ResourcePrincipalAssociation, + Factory: resourcePrincipalAssociation, TypeName: "aws_ram_principal_association", + Name: "Principal Association", }, { - Factory: ResourceResourceAssociation, + Factory: resourceResourceAssociation, TypeName: "aws_ram_resource_association", + Name: "Resource Association", }, { - Factory: ResourceResourceShare, + Factory: resourceResourceShare, TypeName: "aws_ram_resource_share", Name: "Resource Share", Tags: &types.ServicePackageResourceTags{ @@ -52,12 +55,14 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka }, }, { - Factory: ResourceResourceShareAccepter, + Factory: resourceResourceShareAccepter, TypeName: "aws_ram_resource_share_accepter", + Name: "Resource Share Accepter", }, { - Factory: ResourceSharingWithOrganization, + Factory: resourceSharingWithOrganization, TypeName: "aws_ram_sharing_with_organization", + Name: "Sharing With Organization", }, } } diff --git a/internal/service/ram/sharing_with_organization.go b/internal/service/ram/sharing_with_organization.go index 6e8f01dfaa84..970c988c4b7d 100644 --- a/internal/service/ram/sharing_with_organization.go +++ b/internal/service/ram/sharing_with_organization.go @@ -19,8 +19,8 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -// @SDKResource("aws_ram_sharing_with_organization") -func ResourceSharingWithOrganization() *schema.Resource { +// @SDKResource("aws_ram_sharing_with_organization", name="Sharing With Organization") +func resourceSharingWithOrganization() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceSharingWithOrganizationCreate, ReadWithoutTimeout: resourceSharingWithOrganizationRead, diff --git a/internal/service/ram/sweep.go b/internal/service/ram/sweep.go index 1d844717ac84..7ed20ea7deec 100644 --- a/internal/service/ram/sweep.go +++ b/internal/service/ram/sweep.go @@ -43,7 +43,7 @@ func sweepResourceShares(region string) error { continue } - r := ResourceResourceShare() + r := resourceResourceShare() d := r.Data(nil) d.SetId(aws.StringValue(v.ResourceShareArn)) From acdbe8c9ad321ba7ea9b440a485c3db6bac050dc Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:45:28 -0400 Subject: [PATCH 06/30] r/aws_ram_resource_share: Tidy up. --- internal/service/ram/exports_test.go | 2 ++ internal/service/ram/resource_share.go | 21 +++++++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/internal/service/ram/exports_test.go b/internal/service/ram/exports_test.go index 9e95fdca09f7..99c32ca64d03 100644 --- a/internal/service/ram/exports_test.go +++ b/internal/service/ram/exports_test.go @@ -10,4 +10,6 @@ var ( ResourceResourceShare = resourceResourceShare ResourceResourceShareAccepter = resourceResourceShareAccepter ResourceSharingWithOrganization = resourceSharingWithOrganization + + FindResourceShareOwnerSelfByARN = findResourceShareOwnerSelfByARN ) diff --git a/internal/service/ram/resource_share.go b/internal/service/ram/resource_share.go index f77345e38f1c..057790845357 100644 --- a/internal/service/ram/resource_share.go +++ b/internal/service/ram/resource_share.go @@ -5,6 +5,7 @@ package ram import ( "context" + "errors" "log" "time" @@ -98,7 +99,7 @@ func resourceResourceShareCreate(ctx context.Context, d *schema.ResourceData, me d.SetId(aws.StringValue(output.ResourceShare.ResourceShareArn)) _, err = tfresource.RetryWhenNotFound(ctx, FindResourceShareTimeout, func() (interface{}, error) { - return FindResourceShareOwnerSelfByARN(ctx, conn, d.Id()) + return findResourceShareOwnerSelfByARN(ctx, conn, d.Id()) }) if err != nil { @@ -116,7 +117,7 @@ func resourceResourceShareRead(ctx context.Context, d *schema.ResourceData, meta var diags diag.Diagnostics conn := meta.(*conns.AWSClient).RAMConn(ctx) - resourceShare, err := FindResourceShareOwnerSelfByARN(ctx, conn, d.Id()) + resourceShare, err := findResourceShareOwnerSelfByARN(ctx, conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] RAM Resource Share (%s) not found, removing from state", d.Id()) @@ -210,7 +211,7 @@ func resourceResourceShareDelete(ctx context.Context, d *schema.ResourceData, me return diags } -func FindResourceShareOwnerSelfByARN(ctx context.Context, conn *ram.RAM, arn string) (*ram.ResourceShare, error) { +func findResourceShareOwnerSelfByARN(ctx context.Context, conn *ram.RAM, arn string) (*ram.ResourceShare, error) { input := &ram.GetResourceSharesInput{ ResourceOwner: aws.String(ram.ResourceOwnerSelf), ResourceShareArns: aws.StringSlice([]string{arn}), @@ -274,7 +275,7 @@ func findResourceShares(ctx context.Context, conn *ram.RAM, input *ram.GetResour func statusResourceShareOwnerSelf(ctx context.Context, conn *ram.RAM, arn string) retry.StateRefreshFunc { return func() (interface{}, string, error) { - output, err := FindResourceShareOwnerSelfByARN(ctx, conn, arn) + output, err := findResourceShareOwnerSelfByARN(ctx, conn, arn) if tfresource.NotFound(err) { return nil, "", nil @@ -298,8 +299,10 @@ func waitResourceShareOwnedBySelfActive(ctx context.Context, conn *ram.RAM, arn outputRaw, err := stateConf.WaitForStateContext(ctx) - if v, ok := outputRaw.(*ram.ResourceShare); ok { - return v, err + if output, ok := outputRaw.(*ram.ResourceShare); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + + return output, err } return nil, err @@ -315,8 +318,10 @@ func waitResourceShareOwnedBySelfDeleted(ctx context.Context, conn *ram.RAM, arn outputRaw, err := stateConf.WaitForStateContext(ctx) - if v, ok := outputRaw.(*ram.ResourceShare); ok { - return v, err + if output, ok := outputRaw.(*ram.ResourceShare); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + + return output, err } return nil, err From 240bde8c804d35d0f4834a85ccd9ef66d5f88060 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:51:26 -0400 Subject: [PATCH 07/30] Cosmetics. --- internal/service/ram/sharing_with_organization.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/ram/sharing_with_organization.go b/internal/service/ram/sharing_with_organization.go index 970c988c4b7d..cc93126476ad 100644 --- a/internal/service/ram/sharing_with_organization.go +++ b/internal/service/ram/sharing_with_organization.go @@ -46,11 +46,11 @@ func resourceSharingWithOrganizationCreate(ctx context.Context, d *schema.Resour output, err := conn.EnableSharingWithAwsOrganizationWithContext(ctx, &ram.EnableSharingWithAwsOrganizationInput{}) if err != nil { - return sdkdiag.AppendErrorf(diags, "enabling RAM Sharing with Organization: %s", err) + return sdkdiag.AppendErrorf(diags, "enabling RAM Sharing With Organization: %s", err) } if !aws.BoolValue(output.ReturnValue) { - return sdkdiag.AppendErrorf(diags, "RAM Sharing with Organization failed") + return sdkdiag.AppendErrorf(diags, "RAM Sharing With Organization failed") } d.SetId(meta.(*conns.AWSClient).AccountID) From f7b5296b830f85971aa552c0d10b1347463b48e0 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:54:35 -0400 Subject: [PATCH 08/30] Acceptance test output: % make testacc TESTARGS='-run=TestAccRAMSharingWithOrganization_serial' PKG=ram ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.21.8 test ./internal/service/ram/... -v -count 1 -parallel 2 -run=TestAccRAMSharingWithOrganization_serial -timeout 360m === RUN TestAccRAMSharingWithOrganization_serial === PAUSE TestAccRAMSharingWithOrganization_serial === CONT TestAccRAMSharingWithOrganization_serial === RUN TestAccRAMSharingWithOrganization_serial/basic === RUN TestAccRAMSharingWithOrganization_serial/disappears --- PASS: TestAccRAMSharingWithOrganization_serial (62.85s) --- PASS: TestAccRAMSharingWithOrganization_serial/basic (33.02s) --- PASS: TestAccRAMSharingWithOrganization_serial/disappears (29.83s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/ram 69.848s From 2e4c9eaa3ce153bf00bf225d7193629a39d70b38 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:55:54 -0400 Subject: [PATCH 09/30] 'FindResourceShareTimeout' -> 'resourceSharePropagationTimeout'. --- internal/service/ram/find.go | 4 ++-- internal/service/ram/resource_share.go | 2 +- internal/service/ram/resource_share_accepter.go | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/service/ram/find.go b/internal/service/ram/find.go index 5a5124c5d49a..f52d886fb4cf 100644 --- a/internal/service/ram/find.go +++ b/internal/service/ram/find.go @@ -15,8 +15,8 @@ import ( ) const ( - FindInvitationTimeout = 2 * time.Minute - FindResourceShareTimeout = 1 * time.Minute + FindInvitationTimeout = 2 * time.Minute + resourceSharePropagationTimeout = 1 * time.Minute ) // FindResourceShareInvitationByResourceShareARNAndStatus returns the resource share invitation corresponding to the specified resource share ARN. diff --git a/internal/service/ram/resource_share.go b/internal/service/ram/resource_share.go index 057790845357..c464076774e8 100644 --- a/internal/service/ram/resource_share.go +++ b/internal/service/ram/resource_share.go @@ -98,7 +98,7 @@ func resourceResourceShareCreate(ctx context.Context, d *schema.ResourceData, me d.SetId(aws.StringValue(output.ResourceShare.ResourceShareArn)) - _, err = tfresource.RetryWhenNotFound(ctx, FindResourceShareTimeout, func() (interface{}, error) { + _, err = tfresource.RetryWhenNotFound(ctx, resourceSharePropagationTimeout, func() (interface{}, error) { return findResourceShareOwnerSelfByARN(ctx, conn, d.Id()) }) diff --git a/internal/service/ram/resource_share_accepter.go b/internal/service/ram/resource_share_accepter.go index 5c0473a78139..1395b627b717 100644 --- a/internal/service/ram/resource_share_accepter.go +++ b/internal/service/ram/resource_share_accepter.go @@ -85,7 +85,6 @@ func resourceResourceShareAccepterCreate(ctx context.Context, d *schema.Resource conn := meta.(*conns.AWSClient).RAMConn(ctx) shareARN := d.Get("share_arn").(string) - invitation, err := FindResourceShareInvitationByResourceShareARNAndStatus(ctx, conn, shareARN, ram.ResourceShareInvitationStatusPending) if err != nil { @@ -121,7 +120,7 @@ func resourceResourceShareAccepterCreate(ctx context.Context, d *schema.Resource return sdkdiag.AppendErrorf(diags, "waiting for RAM resource share invitation accepted (%s) state: %s", d.Id(), err) } - _, err = tfresource.RetryWhenNotFound(ctx, FindResourceShareTimeout, func() (interface{}, error) { + _, err = tfresource.RetryWhenNotFound(ctx, resourceSharePropagationTimeout, func() (interface{}, error) { return findResourceShareOwnerOtherAccountsByARN(ctx, conn, d.Id()) }) From 96c93191b24dd6d75614e89ef06d77dbb0caf786 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:56:59 -0400 Subject: [PATCH 10/30] 'FindInvitationTimeout' -> 'resourceShareInvitationPropagationTimeout'. --- internal/service/ram/find.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/service/ram/find.go b/internal/service/ram/find.go index f52d886fb4cf..f3ae505989c9 100644 --- a/internal/service/ram/find.go +++ b/internal/service/ram/find.go @@ -15,8 +15,8 @@ import ( ) const ( - FindInvitationTimeout = 2 * time.Minute - resourceSharePropagationTimeout = 1 * time.Minute + resourceShareInvitationPropagationTimeout = 2 * time.Minute + resourceSharePropagationTimeout = 1 * time.Minute ) // FindResourceShareInvitationByResourceShareARNAndStatus returns the resource share invitation corresponding to the specified resource share ARN. @@ -25,7 +25,7 @@ func FindResourceShareInvitationByResourceShareARNAndStatus(ctx context.Context, var invitation *ram.ResourceShareInvitation // Retry for Ram resource share invitation eventual consistency - err := retry.RetryContext(ctx, FindInvitationTimeout, func() *retry.RetryError { + err := retry.RetryContext(ctx, resourceShareInvitationPropagationTimeout, func() *retry.RetryError { i, err := resourceShareInvitationByResourceShareARNAndStatus(ctx, conn, resourceShareArn, status) invitation = i @@ -61,7 +61,7 @@ func FindResourceShareInvitationByARN(ctx context.Context, conn *ram.RAM, arn st var invitation *ram.ResourceShareInvitation // Retry for Ram resource share invitation eventual consistency - err := retry.RetryContext(ctx, FindInvitationTimeout, func() *retry.RetryError { + err := retry.RetryContext(ctx, resourceShareInvitationPropagationTimeout, func() *retry.RetryError { i, err := resourceShareInvitationByARN(ctx, conn, arn) invitation = i From b13dd48b4e4109d75251b9848395374572df6e30 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 13:58:12 -0400 Subject: [PATCH 11/30] ram: Add 'consts.go'. --- internal/service/ram/consts.go | 13 +++++++++++++ internal/service/ram/find.go | 6 ------ 2 files changed, 13 insertions(+), 6 deletions(-) create mode 100644 internal/service/ram/consts.go diff --git a/internal/service/ram/consts.go b/internal/service/ram/consts.go new file mode 100644 index 000000000000..6b8ecafab086 --- /dev/null +++ b/internal/service/ram/consts.go @@ -0,0 +1,13 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package ram + +import ( + "time" +) + +const ( + resourceShareInvitationPropagationTimeout = 2 * time.Minute + resourceSharePropagationTimeout = 1 * time.Minute +) diff --git a/internal/service/ram/find.go b/internal/service/ram/find.go index f3ae505989c9..a37ce6caeacc 100644 --- a/internal/service/ram/find.go +++ b/internal/service/ram/find.go @@ -5,7 +5,6 @@ package ram import ( "context" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ram" @@ -14,11 +13,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -const ( - resourceShareInvitationPropagationTimeout = 2 * time.Minute - resourceSharePropagationTimeout = 1 * time.Minute -) - // FindResourceShareInvitationByResourceShareARNAndStatus returns the resource share invitation corresponding to the specified resource share ARN. // Returns nil if no configuration is found. func FindResourceShareInvitationByResourceShareARNAndStatus(ctx context.Context, conn *ram.RAM, resourceShareArn, status string) (*ram.ResourceShareInvitation, error) { From e8f0f2c6066ee2a589d7712e41b6dab4e50fb7fd Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 14:17:57 -0400 Subject: [PATCH 12/30] Add 'option.Unwrap'. --- internal/types/option/option.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/types/option/option.go b/internal/types/option/option.go index 78ecc4a371a7..dd6d67fc2040 100644 --- a/internal/types/option/option.go +++ b/internal/types/option/option.go @@ -3,6 +3,16 @@ package option +import ( + "errors" + + "github.com/hashicorp/terraform-provider-aws/internal/errs" +) + +var ( + errMissingValue = errors.New("missing value") +) + type Option[T any] []T const ( @@ -31,12 +41,18 @@ func (o Option[T]) IsSome() bool { return o != nil } -// MustUnwrap returns the contained value or panics. -func (o Option[T]) MustUnwrap() T { +// MustUnwrap returns the contained value or an error. +func (o Option[T]) Unwrap() (T, error) { if o.IsNone() { - panic("missing value") + var zero T + return zero, errMissingValue } - return o[value] + return o[value], nil +} + +// MustUnwrap returns the contained value or panics. +func (o Option[T]) MustUnwrap() T { + return errs.Must(o.Unwrap()) } // UnwrapOr returns the contained value or the specified default. From bc75b04b99f20f5bf5f7fce307a7f2c4a42b297f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 14:33:20 -0400 Subject: [PATCH 13/30] Add 'tfresource.AssertMaybeSinglePtrResult'. --- internal/tfresource/not_found_error.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/internal/tfresource/not_found_error.go b/internal/tfresource/not_found_error.go index 181142a11193..46d642bfae05 100644 --- a/internal/tfresource/not_found_error.go +++ b/internal/tfresource/not_found_error.go @@ -8,6 +8,7 @@ import ( "fmt" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" + "github.com/hashicorp/terraform-provider-aws/internal/types/option" ) type EmptyResultError struct { @@ -95,6 +96,8 @@ func SingularDataSourceFindError(resourceType string, err error) error { return fmt.Errorf("reading %s: %w", resourceType, err) } +// AssertSinglePtrResult returns the single non-nil pointer value in the specified slice. +// Returns a `NotFound` error otherwise. func AssertSinglePtrResult[T any](a []*T) (*T, error) { if l := len(a); l == 0 { return nil, NewEmptyResultError(nil) @@ -106,6 +109,21 @@ func AssertSinglePtrResult[T any](a []*T) (*T, error) { return a[0], nil } +// AssertMaybeSinglePtrResult returns the single non-nil pointer value in the specified slice, or `None` if the slice is empty. +// Returns a `NotFound` error otherwise. +func AssertMaybeSinglePtrResult[T any](a []*T) (option.Option[*T], error) { + if l := len(a); l == 0 { + return option.None[*T](), nil + } else if l > 1 { + return nil, NewTooManyResultsError(l, nil) + } else if a[0] == nil { + return nil, NewEmptyResultError(nil) + } + return option.Some(a[0]), nil +} + +// AssertSingleValueResult returns a pointer to the single value in the specified slice of values. +// Returns a `NotFound` error otherwise. func AssertSingleValueResult[T any](a []T) (*T, error) { if l := len(a); l == 0 { return nil, NewEmptyResultError(nil) @@ -115,6 +133,8 @@ func AssertSingleValueResult[T any](a []T) (*T, error) { return &a[0], nil } +// AssertFirstValueResult returns a pointer to the first value in the specified slice of values. +// Returns a `NotFound` error otherwise. func AssertFirstValueResult[T any](a []T) (*T, error) { if l := len(a); l == 0 { return nil, NewEmptyResultError(nil) From b14050cf00b5cb50af6135603f2abf6186a3be8c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 14:45:21 -0400 Subject: [PATCH 14/30] Tweak 'tfresource.RetryGWhen'. --- internal/tfresource/retry.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/tfresource/retry.go b/internal/tfresource/retry.go index d04f0eb418dd..453b09afd50f 100644 --- a/internal/tfresource/retry.go +++ b/internal/tfresource/retry.go @@ -60,8 +60,8 @@ func RetryWhen(ctx context.Context, timeout time.Duration, f func() (interface{} // RetryGWhen is the generic version of RetryWhen which obviates the need for a type // assertion after the call. It retries the function `f` when the error it returns // satisfies `retryable`. `f` is retried until `timeout` expires. -func RetryGWhen[T any](ctx context.Context, timeout time.Duration, f func() (*T, error), retryable Retryable) (*T, error) { - var output *T +func RetryGWhen[T any](ctx context.Context, timeout time.Duration, f func() (T, error), retryable Retryable) (T, error) { + var output T err := Retry(ctx, timeout, func() *retry.RetryError { var err error @@ -86,7 +86,8 @@ func RetryGWhen[T any](ctx context.Context, timeout time.Duration, f func() (*T, } if err != nil { - return nil, err + var zero T + return zero, err } return output, nil @@ -139,7 +140,7 @@ func RetryWhenMessageContains(ctx context.Context, timeout time.Duration, f func } // RetryGWhenMessageContains retries the specified function when it returns an error containing any of the specified messages. -func RetryGWhenMessageContains[T any](ctx context.Context, timeout time.Duration, f func() (*T, error), codes []string, messages []string) (*T, error) { +func RetryGWhenMessageContains[T any](ctx context.Context, timeout time.Duration, f func() (T, error), codes []string, messages []string) (T, error) { return RetryGWhen(ctx, timeout, f, func(err error) (bool, error) { for i, message := range messages { if tfawserr.ErrMessageContains(err, codes[i], message) || tfawserr_sdkv2.ErrMessageContains(err, codes[i], message) { From 33bd11acfdcd750fb2f19aa27bb755c69a086450 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 15:47:54 -0400 Subject: [PATCH 15/30] r/aws_ram_resource_share_accepter: Tidy up. --- internal/service/ram/exports_test.go | 3 +- internal/service/ram/find.go | 115 ---------- .../service/ram/resource_share_accepter.go | 213 +++++++++++++++--- .../ram/resource_share_accepter_test.go | 45 ++-- internal/service/ram/resource_share_test.go | 4 +- internal/service/ram/status.go | 17 -- internal/service/ram/wait.go | 36 --- 7 files changed, 196 insertions(+), 237 deletions(-) diff --git a/internal/service/ram/exports_test.go b/internal/service/ram/exports_test.go index 99c32ca64d03..105ef23f21de 100644 --- a/internal/service/ram/exports_test.go +++ b/internal/service/ram/exports_test.go @@ -11,5 +11,6 @@ var ( ResourceResourceShareAccepter = resourceResourceShareAccepter ResourceSharingWithOrganization = resourceSharingWithOrganization - FindResourceShareOwnerSelfByARN = findResourceShareOwnerSelfByARN + FindResourceShareOwnerOtherAccountsByARN = findResourceShareOwnerOtherAccountsByARN + FindResourceShareOwnerSelfByARN = findResourceShareOwnerSelfByARN ) diff --git a/internal/service/ram/find.go b/internal/service/ram/find.go index a37ce6caeacc..6fe98dde1a7b 100644 --- a/internal/service/ram/find.go +++ b/internal/service/ram/find.go @@ -13,121 +13,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -// FindResourceShareInvitationByResourceShareARNAndStatus returns the resource share invitation corresponding to the specified resource share ARN. -// Returns nil if no configuration is found. -func FindResourceShareInvitationByResourceShareARNAndStatus(ctx context.Context, conn *ram.RAM, resourceShareArn, status string) (*ram.ResourceShareInvitation, error) { - var invitation *ram.ResourceShareInvitation - - // Retry for Ram resource share invitation eventual consistency - err := retry.RetryContext(ctx, resourceShareInvitationPropagationTimeout, func() *retry.RetryError { - i, err := resourceShareInvitationByResourceShareARNAndStatus(ctx, conn, resourceShareArn, status) - invitation = i - - if err != nil { - return retry.NonRetryableError(err) - } - - if invitation == nil { - return retry.RetryableError(&retry.NotFoundError{}) - } - - return nil - }) - - if tfresource.TimedOut(err) { - invitation, err = resourceShareInvitationByResourceShareARNAndStatus(ctx, conn, resourceShareArn, status) - } - - if invitation == nil { - return nil, nil - } - - if err != nil { - return nil, err - } - - return invitation, nil -} - -// FindResourceShareInvitationByARN returns the resource share invitation corresponding to the specified ARN. -// Returns nil if no configuration is found. -func FindResourceShareInvitationByARN(ctx context.Context, conn *ram.RAM, arn string) (*ram.ResourceShareInvitation, error) { - var invitation *ram.ResourceShareInvitation - - // Retry for Ram resource share invitation eventual consistency - err := retry.RetryContext(ctx, resourceShareInvitationPropagationTimeout, func() *retry.RetryError { - i, err := resourceShareInvitationByARN(ctx, conn, arn) - invitation = i - - if err != nil { - return retry.NonRetryableError(err) - } - - if invitation == nil { - retry.RetryableError(&retry.NotFoundError{}) - } - - return nil - }) - - if tfresource.TimedOut(err) { - invitation, err = resourceShareInvitationByARN(ctx, conn, arn) - } - - if invitation == nil { - return nil, nil - } - - if err != nil { - return nil, err - } - - return invitation, nil -} - -func resourceShareInvitationByResourceShareARNAndStatus(ctx context.Context, conn *ram.RAM, resourceShareArn, status string) (*ram.ResourceShareInvitation, error) { - var invitation *ram.ResourceShareInvitation - - input := &ram.GetResourceShareInvitationsInput{ - ResourceShareArns: []*string{aws.String(resourceShareArn)}, - } - - err := conn.GetResourceShareInvitationsPagesWithContext(ctx, input, func(page *ram.GetResourceShareInvitationsOutput, lastPage bool) bool { - for _, rsi := range page.ResourceShareInvitations { - if aws.StringValue(rsi.Status) == status { - invitation = rsi - return false - } - } - - return !lastPage - }) - - if err != nil { - return nil, err - } - - return invitation, nil -} - -func resourceShareInvitationByARN(ctx context.Context, conn *ram.RAM, arn string) (*ram.ResourceShareInvitation, error) { - input := &ram.GetResourceShareInvitationsInput{ - ResourceShareInvitationArns: []*string{aws.String(arn)}, - } - - output, err := conn.GetResourceShareInvitationsWithContext(ctx, input) - - if err != nil { - return nil, err - } - - if len(output.ResourceShareInvitations) == 0 { - return nil, nil - } - - return output.ResourceShareInvitations[0], nil -} - func FindResourceSharePrincipalAssociationByShareARNPrincipal(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { input := &ram.GetResourceShareAssociationsInput{ AssociationType: aws.String(ram.ResourceShareAssociationTypePrincipal), diff --git a/internal/service/ram/resource_share_accepter.go b/internal/service/ram/resource_share_accepter.go index 1395b627b717..bdcaa5283c85 100644 --- a/internal/service/ram/resource_share_accepter.go +++ b/internal/service/ram/resource_share_accepter.go @@ -5,6 +5,7 @@ package ram import ( "context" + "errors" "log" "strings" "time" @@ -14,11 +15,13 @@ import ( "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/id" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/internal/types/option" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) @@ -85,39 +88,41 @@ func resourceResourceShareAccepterCreate(ctx context.Context, d *schema.Resource conn := meta.(*conns.AWSClient).RAMConn(ctx) shareARN := d.Get("share_arn").(string) - invitation, err := FindResourceShareInvitationByResourceShareARNAndStatus(ctx, conn, shareARN, ram.ResourceShareInvitationStatusPending) + maybeInvitation, err := findMaybeResourceShareInvitationByResourceShareARNAndStatus(ctx, conn, shareARN, ram.ResourceShareInvitationStatusPending) if err != nil { - return sdkdiag.AppendErrorf(diags, "creating RAM Resource Share Accepter: %s", err) + return sdkdiag.AppendErrorf(diags, "reading pending RAM Resource Share (%s) invitation: %s", shareARN, err) } - if invitation == nil || aws.StringValue(invitation.ResourceShareInvitationArn) == "" { - return sdkdiag.AppendErrorf(diags, "No RAM Resource Share (%s) invitation found\n\n"+ + var invitationExists bool + var invitationARN string + if maybeInvitation.IsSome() { + invitationExists = true + invitationARN = aws.StringValue(maybeInvitation.MustUnwrap().ResourceShareInvitationArn) + } + + if !invitationExists || invitationARN == "" { + return sdkdiag.AppendErrorf(diags, "No pending RAM Resource Share (%s) invitation found\n\n"+ "NOTE: If both AWS accounts are in the same AWS Organization and RAM Sharing with AWS Organizations is enabled, this resource is not necessary", shareARN) } input := &ram.AcceptResourceShareInvitationInput{ ClientToken: aws.String(id.UniqueId()), - ResourceShareInvitationArn: invitation.ResourceShareInvitationArn, + ResourceShareInvitationArn: aws.String(invitationARN), } - log.Printf("[DEBUG] Accept RAM resource share invitation request: %s", input) output, err := conn.AcceptResourceShareInvitationWithContext(ctx, input) if err != nil { - return sdkdiag.AppendErrorf(diags, "accepting RAM resource share invitation: %s", err) + return sdkdiag.AppendErrorf(diags, "accepting RAM Resource Share (%s) invitation (%s): %s", shareARN, invitationARN, err) } d.SetId(shareARN) - _, err = WaitResourceShareInvitationAccepted(ctx, conn, - aws.StringValue(output.ResourceShareInvitation.ResourceShareInvitationArn), - d.Timeout(schema.TimeoutCreate), - ) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for RAM resource share invitation accepted (%s) state: %s", d.Id(), err) + invitationARN = aws.StringValue(output.ResourceShareInvitation.ResourceShareInvitationArn) + if _, err := waitResourceShareInvitationAccepted(ctx, conn, invitationARN, d.Timeout(schema.TimeoutCreate)); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for RAM Resource Share (%s) invitation (%s) accept: %s", shareARN, invitationARN, err) } _, err = tfresource.RetryWhenNotFound(ctx, resourceSharePropagationTimeout, func() (interface{}, error) { @@ -125,7 +130,7 @@ func resourceResourceShareAccepterCreate(ctx context.Context, d *schema.Resource }) if err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for RAM resource share (%s) state: %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "waiting for RAM Resource Share (%s) create: %s", shareARN, err) } return append(diags, resourceResourceShareAccepterRead(ctx, d, meta)...) @@ -136,13 +141,14 @@ func resourceResourceShareAccepterRead(ctx context.Context, d *schema.ResourceDa accountID := meta.(*conns.AWSClient).AccountID conn := meta.(*conns.AWSClient).RAMConn(ctx) - invitation, err := FindResourceShareInvitationByResourceShareARNAndStatus(ctx, conn, d.Id(), ram.ResourceShareInvitationStatusAccepted) + maybeInvitation, err := findMaybeResourceShareInvitationByResourceShareARNAndStatus(ctx, conn, d.Id(), ram.ResourceShareInvitationStatusAccepted) if err != nil && !tfawserr.ErrCodeEquals(err, ram.ErrCodeResourceShareInvitationArnNotFoundException) { - return sdkdiag.AppendErrorf(diags, "retrieving invitation for resource share %s: %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "reading accepted RAM Resource Share (%s) invitation: %s", d.Id(), err) } - if invitation != nil { + if maybeInvitation.IsSome() { + invitation := maybeInvitation.MustUnwrap() d.Set("invitation_arn", invitation.ResourceShareInvitationArn) d.Set("receiver_account_id", invitation.ReceiverAccountId) } else { @@ -152,7 +158,7 @@ func resourceResourceShareAccepterRead(ctx context.Context, d *schema.ResourceDa resourceShare, err := findResourceShareOwnerOtherAccountsByARN(ctx, conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] No RAM resource share with ARN (%s) found, removing from state", d.Id()) + log.Printf("[WARN] No RAM Resource Share %s found, removing from state", d.Id()) d.SetId("") return diags } @@ -163,16 +169,16 @@ func resourceResourceShareAccepterRead(ctx context.Context, d *schema.ResourceDa d.Set("sender_account_id", resourceShare.OwningAccountId) d.Set("share_arn", resourceShare.ResourceShareArn) - d.Set("share_id", resourceResourceShareGetIDFromARN(d.Id())) + d.Set("share_id", resourceResourceShareIDFromARN(d.Id())) d.Set("share_name", resourceShare.Name) d.Set("status", resourceShare.Status) - inputL := &ram.ListResourcesInput{ + input := &ram.ListResourcesInput{ MaxResults: aws.Int64(500), ResourceOwner: aws.String(ram.ResourceOwnerOtherAccounts), ResourceShareArns: aws.StringSlice([]string{d.Id()}), } - resources, err := findResources(ctx, conn, inputL) + resources, err := findResources(ctx, conn, input) if err != nil { return sdkdiag.AppendErrorf(diags, "reading RAM Resource Share (%s) resources: %s", d.Id(), err) @@ -191,41 +197,37 @@ func resourceResourceShareAccepterDelete(ctx context.Context, d *schema.Resource conn := meta.(*conns.AWSClient).RAMConn(ctx) receiverAccountID := d.Get("receiver_account_id").(string) - if receiverAccountID == "" { return sdkdiag.AppendErrorf(diags, "The receiver account ID is required to leave a resource share") } input := &ram.DisassociateResourceShareInput{ ClientToken: aws.String(id.UniqueId()), - ResourceShareArn: aws.String(d.Id()), Principals: []*string{aws.String(receiverAccountID)}, + ResourceShareArn: aws.String(d.Id()), } - log.Printf("[DEBUG] Leave RAM resource share request: %s", input) _, err := conn.DisassociateResourceShareWithContext(ctx, input) - if tfawserr.ErrCodeEquals(err, ram.ErrCodeOperationNotPermittedException) { + switch { + case tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException): + return diags + + case tfawserr.ErrCodeEquals(err, ram.ErrCodeOperationNotPermittedException): log.Printf("[WARN] Resource share could not be disassociated, but continuing: %s", err) - } - if err != nil && !tfawserr.ErrCodeEquals(err, ram.ErrCodeOperationNotPermittedException) { - if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { - return diags - } + case err != nil: return sdkdiag.AppendErrorf(diags, "leaving RAM resource share: %s", err) } - _, err = WaitResourceShareOwnedBySelfDisassociated(ctx, conn, d.Id(), d.Timeout(schema.TimeoutDelete)) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for RAM resource share (%s) state: %s", d.Id(), err) + if _, err := waitResourceShareOwnedBySelfDisassociated(ctx, conn, d.Id(), d.Timeout(schema.TimeoutDelete)); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for RAM Resource Share (%s) disassociate: %s", d.Id(), err) } return diags } -func resourceResourceShareGetIDFromARN(arn string) string { +func resourceResourceShareIDFromARN(arn string) string { return strings.Replace(arn[strings.LastIndex(arn, ":")+1:], "resource-share/", "rs-", -1) } @@ -234,6 +236,7 @@ func findResourceShareOwnerOtherAccountsByARN(ctx context.Context, conn *ram.RAM ResourceOwner: aws.String(ram.ResourceOwnerOtherAccounts), ResourceShareArns: aws.StringSlice([]string{arn}), } + output, err := findResourceShare(ctx, conn, input) if err != nil { @@ -268,3 +271,141 @@ func findResources(ctx context.Context, conn *ram.RAM, input *ram.ListResourcesI return output, nil } + +func findMaybeResourceShareInvitationByResourceShareARNAndStatus(ctx context.Context, conn *ram.RAM, resourceShareARN, status string) (option.Option[*ram.ResourceShareInvitation], error) { + input := &ram.GetResourceShareInvitationsInput{ + ResourceShareArns: aws.StringSlice([]string{resourceShareARN}), + } + + return findMaybeResourceShareInvitationRetry(ctx, conn, input, func(v *ram.ResourceShareInvitation) bool { + return aws.StringValue(v.Status) == status + }) +} + +func findMaybeResourceShareInvitationByARN(ctx context.Context, conn *ram.RAM, arn string) (option.Option[*ram.ResourceShareInvitation], error) { + input := &ram.GetResourceShareInvitationsInput{ + ResourceShareInvitationArns: aws.StringSlice([]string{arn}), + } + + return findMaybeResourceShareInvitationRetry(ctx, conn, input, tfslices.PredicateTrue[*ram.ResourceShareInvitation]()) +} + +func findMaybeResourceShareInvitationRetry(ctx context.Context, conn *ram.RAM, input *ram.GetResourceShareInvitationsInput, filter tfslices.Predicate[*ram.ResourceShareInvitation]) (option.Option[*ram.ResourceShareInvitation], error) { + // Retry for RAM resource share invitation eventual consistency. + var output option.Option[*ram.ResourceShareInvitation] + err := tfresource.Retry(ctx, resourceShareInvitationPropagationTimeout, func() *retry.RetryError { + var err error + output, err = findMaybeResourceShareInvitation(ctx, conn, input, filter) + + if err != nil { + return retry.NonRetryableError(err) + } + + if output.IsNone() { + return retry.RetryableError(tfresource.NewEmptyResultError(nil)) + } + + return nil + }) + + if tfresource.TimedOut(err) { + output, err = findMaybeResourceShareInvitation(ctx, conn, input, filter) + } + + return output, err +} + +func findMaybeResourceShareInvitation(ctx context.Context, conn *ram.RAM, input *ram.GetResourceShareInvitationsInput, filter tfslices.Predicate[*ram.ResourceShareInvitation]) (option.Option[*ram.ResourceShareInvitation], error) { + output, err := findResourceShareInvitations(ctx, conn, input, filter) + + if err != nil { + return nil, err + } + + return tfresource.AssertMaybeSinglePtrResult(output) +} + +func findResourceShareInvitations(ctx context.Context, conn *ram.RAM, input *ram.GetResourceShareInvitationsInput, filter tfslices.Predicate[*ram.ResourceShareInvitation]) ([]*ram.ResourceShareInvitation, error) { + var output []*ram.ResourceShareInvitation + + err := conn.GetResourceShareInvitationsPagesWithContext(ctx, input, func(page *ram.GetResourceShareInvitationsOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.ResourceShareInvitations { + if v != nil && filter(v) { + output = append(output, v) + } + } + + return !lastPage + }) + + if tfawserr.ErrCodeEquals(err, ram.ErrCodeResourceShareInvitationArnNotFoundException, ram.ErrCodeUnknownResourceException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + return output, nil +} + +func statusResourceShareInvitation(ctx context.Context, conn *ram.RAM, arn string) retry.StateRefreshFunc { + return func() (interface{}, string, error) { + maybeInvitation, err := findMaybeResourceShareInvitationByARN(ctx, conn, arn) + + if tfresource.NotFound(err) || maybeInvitation.IsNone() { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + output := maybeInvitation.MustUnwrap() + + return output, aws.StringValue(output.Status), nil + } +} + +func waitResourceShareInvitationAccepted(ctx context.Context, conn *ram.RAM, arn string, timeout time.Duration) (*ram.ResourceShareInvitation, error) { + stateConf := &retry.StateChangeConf{ + Pending: []string{ram.ResourceShareInvitationStatusPending}, + Target: []string{ram.ResourceShareInvitationStatusAccepted}, + Refresh: statusResourceShareInvitation(ctx, conn, arn), + Timeout: timeout, + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if v, ok := outputRaw.(*ram.ResourceShareInvitation); ok { + return v, err + } + + return nil, err +} + +func waitResourceShareOwnedBySelfDisassociated(ctx context.Context, conn *ram.RAM, arn string, timeout time.Duration) (*ram.ResourceShare, error) { + stateConf := &retry.StateChangeConf{ + Pending: []string{ram.ResourceShareAssociationStatusAssociated}, + Target: []string{}, + Refresh: statusResourceShareOwnerSelf(ctx, conn, arn), + Timeout: timeout, + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*ram.ResourceShare); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + + return output, err + } + + return nil, err +} diff --git a/internal/service/ram/resource_share_accepter_test.go b/internal/service/ram/resource_share_accepter_test.go index 1a60e3f203c0..3c4a82803945 100644 --- a/internal/service/ram/resource_share_accepter_test.go +++ b/internal/service/ram/resource_share_accepter_test.go @@ -9,15 +9,14 @@ import ( "testing" "github.com/YakDriver/regexache" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ram" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfram "github.com/hashicorp/terraform-provider-aws/internal/service/ram" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -145,54 +144,40 @@ func testAccCheckResourceShareAccepterDestroy(ctx context.Context) resource.Test continue } - input := &ram.GetResourceSharesInput{ - ResourceShareArns: []*string{aws.String(rs.Primary.Attributes["share_arn"])}, - ResourceOwner: aws.String(ram.ResourceOwnerOtherAccounts), + _, err := tfram.FindResourceShareOwnerOtherAccountsByARN(ctx, conn, rs.Primary.ID) + + if tfresource.NotFound(err) { + continue } - output, err := conn.GetResourceSharesWithContext(ctx, input) if err != nil { - if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { - return nil - } - return fmt.Errorf("Error deleting RAM resource share: %s", err) + return err } - if len(output.ResourceShares) > 0 && aws.StringValue(output.ResourceShares[0].Status) != ram.ResourceShareStatusDeleted { - return fmt.Errorf("RAM resource share invitation found, should be destroyed") - } + return fmt.Errorf("RAM Resource Share %s still exists", rs.Primary.ID) } return nil } } -func testAccCheckResourceShareAccepterExists(ctx context.Context, name string) resource.TestCheckFunc { +func testAccCheckResourceShareAccepterExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[name] - - if !ok || rs.Type != "aws_ram_resource_share_accepter" { - return fmt.Errorf("RAM resource share invitation not found: %s", name) + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) } conn := acctest.Provider.Meta().(*conns.AWSClient).RAMConn(ctx) - input := &ram.GetResourceSharesInput{ - ResourceShareArns: []*string{aws.String(rs.Primary.Attributes["share_arn"])}, - ResourceOwner: aws.String(ram.ResourceOwnerOtherAccounts), - } + _, err := tfram.FindResourceShareOwnerOtherAccountsByARN(ctx, conn, rs.Primary.ID) - output, err := conn.GetResourceSharesWithContext(ctx, input) - if err != nil || len(output.ResourceShares) == 0 { - return fmt.Errorf("Error finding RAM resource share: %s", err) - } - - return nil + return err } } func testAccResourceShareAccepterConfig_basic(rName string) string { - return acctest.ConfigAlternateAccountProvider() + fmt.Sprintf(` + return acctest.ConfigCompose(acctest.ConfigAlternateAccountProvider(), fmt.Sprintf(` resource "aws_ram_resource_share_accepter" "test" { share_arn = aws_ram_principal_association.test.resource_share_arn } @@ -216,7 +201,7 @@ resource "aws_ram_principal_association" "test" { } data "aws_caller_identity" "receiver" {} -`, rName) +`, rName)) } func testAccResourceShareAccepterConfig_association(rName string) string { diff --git a/internal/service/ram/resource_share_test.go b/internal/service/ram/resource_share_test.go index 6cba505cdc62..e3ee86ee147f 100644 --- a/internal/service/ram/resource_share_test.go +++ b/internal/service/ram/resource_share_test.go @@ -227,13 +227,13 @@ func TestAccRAMResourceShare_disappears(t *testing.T) { func testAccCheckResourceShareExists(ctx context.Context, n string, v *ram.ResourceShare) resource.TestCheckFunc { return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).RAMConn(ctx) - rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("Not found: %s", n) } + conn := acctest.Provider.Meta().(*conns.AWSClient).RAMConn(ctx) + output, err := tfram.FindResourceShareOwnerSelfByARN(ctx, conn, rs.Primary.ID) if err != nil { diff --git a/internal/service/ram/status.go b/internal/service/ram/status.go index 4735b8acb73c..2c90d15ee35e 100644 --- a/internal/service/ram/status.go +++ b/internal/service/ram/status.go @@ -20,23 +20,6 @@ const ( PrincipalAssociationStatusNotFound = "NotFound" ) -// StatusResourceShareInvitation fetches the ResourceShareInvitation and its Status -func StatusResourceShareInvitation(ctx context.Context, conn *ram.RAM, arn string) retry.StateRefreshFunc { - return func() (interface{}, string, error) { - invitation, err := FindResourceShareInvitationByARN(ctx, conn, arn) - - if err != nil { - return nil, ResourceShareInvitationStatusUnknown, err - } - - if invitation == nil { - return nil, ResourceShareInvitationStatusNotFound, nil - } - - return invitation, aws.StringValue(invitation.Status), nil - } -} - func StatusResourceSharePrincipalAssociation(ctx context.Context, conn *ram.RAM, resourceShareArn, principal string) retry.StateRefreshFunc { return func() (interface{}, string, error) { association, err := FindResourceSharePrincipalAssociationByShareARNPrincipal(ctx, conn, resourceShareArn, principal) diff --git a/internal/service/ram/wait.go b/internal/service/ram/wait.go index df1f3eb7ce99..95af2eb0d06f 100644 --- a/internal/service/ram/wait.go +++ b/internal/service/ram/wait.go @@ -16,42 +16,6 @@ const ( PrincipalDisassociationTimeout = 3 * time.Minute ) -// WaitResourceShareInvitationAccepted waits for a ResourceShareInvitation to return ACCEPTED -func WaitResourceShareInvitationAccepted(ctx context.Context, conn *ram.RAM, arn string, timeout time.Duration) (*ram.ResourceShareInvitation, error) { - stateConf := &retry.StateChangeConf{ - Pending: []string{ram.ResourceShareInvitationStatusPending}, - Target: []string{ram.ResourceShareInvitationStatusAccepted}, - Refresh: StatusResourceShareInvitation(ctx, conn, arn), - Timeout: timeout, - } - - outputRaw, err := stateConf.WaitForStateContext(ctx) - - if v, ok := outputRaw.(*ram.ResourceShareInvitation); ok { - return v, err - } - - return nil, err -} - -// WaitResourceShareOwnedBySelfDisassociated waits for a ResourceShare owned by own account to be disassociated -func WaitResourceShareOwnedBySelfDisassociated(ctx context.Context, conn *ram.RAM, arn string, timeout time.Duration) (*ram.ResourceShare, error) { - stateConf := &retry.StateChangeConf{ - Pending: []string{ram.ResourceShareAssociationStatusAssociated}, - Target: []string{}, - Refresh: statusResourceShareOwnerSelf(ctx, conn, arn), - Timeout: timeout, - } - - outputRaw, err := stateConf.WaitForStateContext(ctx) - - if v, ok := outputRaw.(*ram.ResourceShare); ok { - return v, err - } - - return nil, err -} - func WaitResourceSharePrincipalAssociated(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { stateConf := &retry.StateChangeConf{ Pending: []string{ram.ResourceShareAssociationStatusAssociating, PrincipalAssociationStatusNotFound}, From e084a824dd1f3251576f200a6084ff73356e53b0 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 13 Mar 2024 16:45:08 -0400 Subject: [PATCH 16/30] r/aws_ram_resource_share_accepter: Fix acceptance tests. --- internal/service/ram/resource_share_accepter.go | 7 ++++++- internal/service/ram/resource_share_accepter_test.go | 6 ------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/service/ram/resource_share_accepter.go b/internal/service/ram/resource_share_accepter.go index bdcaa5283c85..87bd6fda1552 100644 --- a/internal/service/ram/resource_share_accepter.go +++ b/internal/service/ram/resource_share_accepter.go @@ -292,6 +292,7 @@ func findMaybeResourceShareInvitationByARN(ctx context.Context, conn *ram.RAM, a func findMaybeResourceShareInvitationRetry(ctx context.Context, conn *ram.RAM, input *ram.GetResourceShareInvitationsInput, filter tfslices.Predicate[*ram.ResourceShareInvitation]) (option.Option[*ram.ResourceShareInvitation], error) { // Retry for RAM resource share invitation eventual consistency. + errNotFound := errors.New("not found") var output option.Option[*ram.ResourceShareInvitation] err := tfresource.Retry(ctx, resourceShareInvitationPropagationTimeout, func() *retry.RetryError { var err error @@ -302,7 +303,7 @@ func findMaybeResourceShareInvitationRetry(ctx context.Context, conn *ram.RAM, i } if output.IsNone() { - return retry.RetryableError(tfresource.NewEmptyResultError(nil)) + return retry.RetryableError(errNotFound) } return nil @@ -312,6 +313,10 @@ func findMaybeResourceShareInvitationRetry(ctx context.Context, conn *ram.RAM, i output, err = findMaybeResourceShareInvitation(ctx, conn, input, filter) } + if errors.Is(err, errNotFound) { + output, err = option.None[*ram.ResourceShareInvitation](), nil + } + return output, err } diff --git a/internal/service/ram/resource_share_accepter_test.go b/internal/service/ram/resource_share_accepter_test.go index 3c4a82803945..dc6e52e829a3 100644 --- a/internal/service/ram/resource_share_accepter_test.go +++ b/internal/service/ram/resource_share_accepter_test.go @@ -125,12 +125,6 @@ func TestAccRAMResourceShareAccepter_resourceAssociation(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "resources.%", "0"), ), }, - { - Config: testAccResourceShareAccepterConfig_association(rName), - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, }, }) } From f3cc15f5584df17d7baff95f3559cc2e0933ff5e Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 08:42:11 -0400 Subject: [PATCH 17/30] r/aws_ram_resource_association: Tidy up. --- .changelog/36062.txt | 4 + internal/service/ram/exports_test.go | 6 +- internal/service/ram/find.go | 65 ------ internal/service/ram/principal_association.go | 26 ++- .../service/ram/principal_association_test.go | 12 +- internal/service/ram/resource_association.go | 201 +++++++++++------- .../service/ram/resource_association_test.go | 68 ++---- internal/service/ram/status.go | 2 +- 8 files changed, 173 insertions(+), 211 deletions(-) delete mode 100644 internal/service/ram/find.go diff --git a/.changelog/36062.txt b/.changelog/36062.txt index d1ce2e6b443f..119040e66b2d 100644 --- a/.changelog/36062.txt +++ b/.changelog/36062.txt @@ -1,3 +1,7 @@ ```release-note:enhancement data-source/aws_ram_resource_share: `name` is Optional +``` + +```release-note:enhancement +resource/aws_ram_resource_association: Add plan-time validation of `resource_arn` and `resource_share_arn` ``` \ No newline at end of file diff --git a/internal/service/ram/exports_test.go b/internal/service/ram/exports_test.go index 105ef23f21de..a18d20104e44 100644 --- a/internal/service/ram/exports_test.go +++ b/internal/service/ram/exports_test.go @@ -11,6 +11,8 @@ var ( ResourceResourceShareAccepter = resourceResourceShareAccepter ResourceSharingWithOrganization = resourceSharingWithOrganization - FindResourceShareOwnerOtherAccountsByARN = findResourceShareOwnerOtherAccountsByARN - FindResourceShareOwnerSelfByARN = findResourceShareOwnerSelfByARN + FindResourceAssociationByTwoPartKey = findResourceAssociationByTwoPartKey + FindResourceShareAssociationByShareARNAndPrincipal = findResourceShareAssociationByShareARNAndPrincipal + FindResourceShareOwnerOtherAccountsByARN = findResourceShareOwnerOtherAccountsByARN + FindResourceShareOwnerSelfByARN = findResourceShareOwnerSelfByARN ) diff --git a/internal/service/ram/find.go b/internal/service/ram/find.go deleted file mode 100644 index 6fe98dde1a7b..000000000000 --- a/internal/service/ram/find.go +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package ram - -import ( - "context" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ram" - "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/tfresource" -) - -func FindResourceSharePrincipalAssociationByShareARNPrincipal(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { - input := &ram.GetResourceShareAssociationsInput{ - AssociationType: aws.String(ram.ResourceShareAssociationTypePrincipal), - Principal: aws.String(principal), - ResourceShareArns: aws.StringSlice([]string{resourceShareARN}), - } - - return findResourceShareAssociation(ctx, conn, input) -} - -func findResourceShareAssociation(ctx context.Context, conn *ram.RAM, input *ram.GetResourceShareAssociationsInput) (*ram.ResourceShareAssociation, error) { - output, err := findResourceShareAssociations(ctx, conn, input) - - if err != nil { - return nil, err - } - - return tfresource.AssertSinglePtrResult(output) -} - -func findResourceShareAssociations(ctx context.Context, conn *ram.RAM, input *ram.GetResourceShareAssociationsInput) ([]*ram.ResourceShareAssociation, error) { - var output []*ram.ResourceShareAssociation - - err := conn.GetResourceShareAssociationsPagesWithContext(ctx, input, func(page *ram.GetResourceShareAssociationsOutput, lastPage bool) bool { - if page == nil { - return !lastPage - } - - for _, v := range page.ResourceShareAssociations { - if v != nil { - output = append(output, v) - } - } - - return !lastPage - }) - - if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { - return nil, &retry.NotFoundError{ - LastError: err, - LastRequest: input, - } - } - - if err != nil { - return nil, err - } - - return output, nil -} diff --git a/internal/service/ram/principal_association.go b/internal/service/ram/principal_association.go index f9001b7478d3..f8c894df2afe 100644 --- a/internal/service/ram/principal_association.go +++ b/internal/service/ram/principal_association.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/id" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -99,7 +100,7 @@ func resourcePrincipalAssociationRead(ctx context.Context, d *schema.ResourceDat if ok, _ := regexp.MatchString(`^\d{12}$`, principal); ok { // AWS Account ID Principals need to be accepted to become ASSOCIATED - association, err = FindResourceSharePrincipalAssociationByShareARNPrincipal(ctx, conn, resourceShareArn, principal) + association, err = findResourceShareAssociationByShareARNAndPrincipal(ctx, conn, resourceShareArn, principal) } else { association, err = WaitResourceSharePrincipalAssociated(ctx, conn, resourceShareArn, principal) } @@ -172,3 +173,26 @@ func PrincipalAssociationParseID(id string) (string, string, error) { return parts[0], parts[1], nil } + +func findResourceShareAssociationByShareARNAndPrincipal(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { + input := &ram.GetResourceShareAssociationsInput{ + AssociationType: aws.String(ram.ResourceShareAssociationTypePrincipal), + Principal: aws.String(principal), + ResourceShareArns: aws.StringSlice([]string{resourceShareARN}), + } + + output, err := findResourceShareAssociation(ctx, conn, input) + + if err != nil { + return nil, err + } + + if status := aws.StringValue(output.Status); status == ram.ResourceShareAssociationStatusDisassociated { + return nil, &retry.NotFoundError{ + Message: status, + LastRequest: input, + } + } + + return output, err +} diff --git a/internal/service/ram/principal_association_test.go b/internal/service/ram/principal_association_test.go index addac8bba679..8e92875db84f 100644 --- a/internal/service/ram/principal_association_test.go +++ b/internal/service/ram/principal_association_test.go @@ -94,7 +94,7 @@ func testAccCheckPrincipalAssociationExists(ctx context.Context, resourceName st if ok, _ := regexp.MatchString(`^\d{12}$`, principal); ok { // AWS Account ID Principals need to be accepted to become ASSOCIATED - association, err = tfram.FindResourceSharePrincipalAssociationByShareARNPrincipal(ctx, conn, resourceShareARN, principal) + association, err = tfram.FindResourceShareAssociationByShareARNAndPrincipal(ctx, conn, resourceShareARN, principal) } else { association, err = tfram.WaitResourceSharePrincipalAssociated(ctx, conn, resourceShareARN, principal) } @@ -126,20 +126,14 @@ func testAccCheckPrincipalAssociationDestroy(ctx context.Context) resource.TestC continue } - resourceShareARN, principal, err := tfram.DecodeResourceAssociationID(rs.Primary.ID) - - if err != nil { - return err - } - - association, err := tfram.WaitResourceSharePrincipalDisassociated(ctx, conn, resourceShareARN, principal) + association, err := tfram.WaitResourceSharePrincipalDisassociated(ctx, conn, rs.Primary.Attributes["resource_share_arn"], rs.Primary.Attributes["principal"]) if err != nil { return err } if association != nil && aws.StringValue(association.Status) != ram.ResourceShareAssociationStatusDisassociated { - return fmt.Errorf("RAM Resource Share (%s) Principal Association (%s) not disassociated: %s", resourceShareARN, principal, aws.StringValue(association.Status)) + return fmt.Errorf("RAM Principal Association: %s", rs.Primary.ID) } } diff --git a/internal/service/ram/resource_association.go b/internal/service/ram/resource_association.go index f5bb300fc9b3..916414013740 100644 --- a/internal/service/ram/resource_association.go +++ b/internal/service/ram/resource_association.go @@ -5,21 +5,23 @@ package ram import ( "context" - "fmt" + "errors" "log" - "strings" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ram" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/id" + sdkid "github.com/hashicorp/terraform-plugin-sdk/v2/helper/id" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/errs" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + "github.com/hashicorp/terraform-provider-aws/internal/flex" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/internal/verify" ) // @SDKResource("aws_ram_resource_association", name="Resource Association") @@ -35,42 +37,47 @@ func resourceResourceAssociation() *schema.Resource { Schema: map[string]*schema.Schema{ "resource_arn": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, }, - "resource_share_arn": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, }, }, } } +const ( + resourceAssociationResourceIDPartCount = 2 +) + func resourceResourceAssociationCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).RAMConn(ctx) - resourceARN := d.Get("resource_arn").(string) - resourceShareARN := d.Get("resource_share_arn").(string) + resourceShareARN, resourceARN := d.Get("resource_share_arn").(string), d.Get("resource_arn").(string) + id := errs.Must(flex.FlattenResourceId([]string{resourceShareARN, resourceARN}, resourceAssociationResourceIDPartCount, false)) input := &ram.AssociateResourceShareInput{ - ClientToken: aws.String(id.UniqueId()), + ClientToken: aws.String(sdkid.UniqueId()), ResourceArns: aws.StringSlice([]string{resourceARN}), ResourceShareArn: aws.String(resourceShareARN), } - log.Printf("[DEBUG] Associating RAM Resource Share: %s", input) _, err := conn.AssociateResourceShareWithContext(ctx, input) + if err != nil { - return sdkdiag.AppendErrorf(diags, "associating RAM Resource Share: %s", err) + return sdkdiag.AppendErrorf(diags, "creating RAM Resource Association (%s): %s", id, err) } - d.SetId(fmt.Sprintf("%s,%s", resourceShareARN, resourceARN)) + d.SetId(id) - if err := waitForResourceShareResourceAssociation(ctx, conn, resourceShareARN, resourceARN); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for RAM Resource Share (%s) Resource Association (%s): %s", resourceShareARN, resourceARN, err) + if _, err := waitResourceAssociationCreated(ctx, conn, resourceShareARN, resourceARN); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for RAM Resource Association (%s) create: %s", d.Id(), err) } return append(diags, resourceResourceAssociationRead(ctx, d, meta)...) @@ -80,29 +87,26 @@ func resourceResourceAssociationRead(ctx context.Context, d *schema.ResourceData var diags diag.Diagnostics conn := meta.(*conns.AWSClient).RAMConn(ctx) - resourceShareARN, resourceARN, err := DecodeResourceAssociationID(d.Id()) + parts, err := flex.ExpandResourceId(d.Id(), resourceAssociationResourceIDPartCount, false) if err != nil { - return sdkdiag.AppendErrorf(diags, "reading RAM Resource Share Resource Association (%s): %s", d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } + resourceShareARN, resourceARN := parts[0], parts[1] + + resourceShareAssociation, err := findResourceAssociationByTwoPartKey(ctx, conn, resourceShareARN, resourceARN) - resourceShareAssociation, err := GetResourceShareAssociation(ctx, conn, resourceShareARN, resourceARN) if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] RAM Resource Share (%s) Resource Association (%s) not found, removing from state", resourceShareARN, resourceARN) + log.Printf("[WARN] RAM Resource Association %s not found, removing from state", d.Id()) d.SetId("") return diags } - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading RAM Resource Share Resource Association (%s): %s", d.Id(), err) - } - if !d.IsNewResource() && aws.StringValue(resourceShareAssociation.Status) != ram.ResourceShareAssociationStatusAssociated { - log.Printf("[WARN] RAM Resource Share (%s) Resource Association (%s) not associated, removing from state", resourceShareARN, resourceARN) - d.SetId("") - return diags + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading RAM Resource Association (%s): %s", d.Id(), err) } - d.Set("resource_arn", resourceARN) - d.Set("resource_share_arn", resourceShareARN) + d.Set("resource_arn", resourceShareAssociation.AssociatedEntity) + d.Set("resource_share_arn", resourceShareAssociation.ResourceShareArn) return diags } @@ -111,53 +115,82 @@ func resourceResourceAssociationDelete(ctx context.Context, d *schema.ResourceDa var diags diag.Diagnostics conn := meta.(*conns.AWSClient).RAMConn(ctx) - resourceShareARN, resourceARN, err := DecodeResourceAssociationID(d.Id()) + parts, err := flex.ExpandResourceId(d.Id(), resourceAssociationResourceIDPartCount, false) if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting RAM Resource Share Resource Association (%s): %s", d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } + resourceShareARN, resourceARN := parts[0], parts[1] - input := &ram.DisassociateResourceShareInput{ + log.Printf("[DEBUG] Deleting RAM Resource Associating: %s", d.Id()) + _, err = conn.DisassociateResourceShareWithContext(ctx, &ram.DisassociateResourceShareInput{ ResourceArns: aws.StringSlice([]string{resourceARN}), ResourceShareArn: aws.String(resourceShareARN), - } - - log.Printf("[DEBUG] Disassociating RAM Resource Share: %s", input) - _, err = conn.DisassociateResourceShareWithContext(ctx, input) + }) if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting RAM Resource Share Resource Association (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "deleting RAM Resource Association (%s): %s", d.Id(), err) } - if err := WaitForResourceShareResourceDisassociation(ctx, conn, resourceShareARN, resourceARN); err != nil { - return sdkdiag.AppendErrorf(diags, "deleting RAM Resource Share Resource Association (%s): waiting for completion: %s", d.Id(), err) + if _, err := waitResourceAssociationDeleted(ctx, conn, resourceShareARN, resourceARN); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for RAM Resource Association (%s) delete: %s", d.Id(), err) } return diags } -func DecodeResourceAssociationID(id string) (string, string, error) { - idFormatErr := fmt.Errorf("unexpected format of ID (%s), expected SHARE,RESOURCE", id) +func findResourceAssociationByTwoPartKey(ctx context.Context, conn *ram.RAM, resourceShareARN, resourceARN string) (*ram.ResourceShareAssociation, error) { + input := &ram.GetResourceShareAssociationsInput{ + AssociationType: aws.String(ram.ResourceShareAssociationTypeResource), + ResourceArn: aws.String(resourceARN), + ResourceShareArns: aws.StringSlice([]string{resourceShareARN}), + } + + output, err := findResourceShareAssociation(ctx, conn, input) + + if err != nil { + return nil, err + } - parts := strings.SplitN(id, ",", 2) - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return "", "", idFormatErr + if status := aws.StringValue(output.Status); status == ram.ResourceShareAssociationStatusDisassociated { + return nil, &retry.NotFoundError{ + Message: status, + LastRequest: input, + } } - return parts[0], parts[1], nil + return output, err } -func GetResourceShareAssociation(ctx context.Context, conn *ram.RAM, resourceShareARN, resourceARN string) (*ram.ResourceShareAssociation, error) { - input := &ram.GetResourceShareAssociationsInput{ - AssociationType: aws.String(ram.ResourceShareAssociationTypeResource), - ResourceArn: aws.String(resourceARN), - ResourceShareArns: aws.StringSlice([]string{resourceShareARN}), +func findResourceShareAssociation(ctx context.Context, conn *ram.RAM, input *ram.GetResourceShareAssociationsInput) (*ram.ResourceShareAssociation, error) { + output, err := findResourceShareAssociations(ctx, conn, input) + + if err != nil { + return nil, err } - output, err := conn.GetResourceShareAssociationsWithContext(ctx, input) + return tfresource.AssertSinglePtrResult(output) +} + +func findResourceShareAssociations(ctx context.Context, conn *ram.RAM, input *ram.GetResourceShareAssociationsInput) ([]*ram.ResourceShareAssociation, error) { + var output []*ram.ResourceShareAssociation + + err := conn.GetResourceShareAssociationsPagesWithContext(ctx, input, func(page *ram.GetResourceShareAssociationsOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.ResourceShareAssociations { + if v != nil { + output = append(output, v) + } + } + + return !lastPage + }) if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { return nil, &retry.NotFoundError{ @@ -170,57 +203,65 @@ func GetResourceShareAssociation(ctx context.Context, conn *ram.RAM, resourceSha return nil, err } - switch count := len(output.ResourceShareAssociations); count { - case 0: - return nil, tfresource.NewEmptyResultError(input) - case 1: - return output.ResourceShareAssociations[0], nil - default: - return nil, tfresource.NewTooManyResultsError(count, input) - } + return output, nil } -func resourceAssociationStateRefreshFunc(ctx context.Context, conn *ram.RAM, resourceShareARN, resourceARN string) retry.StateRefreshFunc { +func statusResourceAssociation(ctx context.Context, conn *ram.RAM, resourceShareARN, resourceARN string) retry.StateRefreshFunc { return func() (interface{}, string, error) { - resourceShareAssociation, err := GetResourceShareAssociation(ctx, conn, resourceShareARN, resourceARN) + output, err := findResourceAssociationByTwoPartKey(ctx, conn, resourceShareARN, resourceARN) + if tfresource.NotFound(err) { - return nil, ram.ResourceShareAssociationStatusDisassociated, nil + return nil, "", nil } + if err != nil { return nil, "", err } - if aws.StringValue(resourceShareAssociation.Status) == ram.ResourceShareAssociationStatusFailed { - extendedErr := fmt.Errorf("association status message: %s", aws.StringValue(resourceShareAssociation.StatusMessage)) - return resourceShareAssociation, aws.StringValue(resourceShareAssociation.Status), extendedErr - } - - return resourceShareAssociation, aws.StringValue(resourceShareAssociation.Status), nil + return output, aws.StringValue(output.Status), nil } } -func waitForResourceShareResourceAssociation(ctx context.Context, conn *ram.RAM, resourceShareARN, resourceARN string) error { +func waitResourceAssociationCreated(ctx context.Context, conn *ram.RAM, resourceShareARN, resourceARN string) (*ram.ResourceShareAssociation, error) { + const ( + timeout = 5 * time.Minute + ) stateConf := &retry.StateChangeConf{ Pending: []string{ram.ResourceShareAssociationStatusAssociating}, Target: []string{ram.ResourceShareAssociationStatusAssociated}, - Refresh: resourceAssociationStateRefreshFunc(ctx, conn, resourceShareARN, resourceARN), - Timeout: 5 * time.Minute, + Refresh: statusResourceAssociation(ctx, conn, resourceShareARN, resourceARN), + Timeout: timeout, } - _, err := stateConf.WaitForStateContext(ctx) + outputRaw, err := stateConf.WaitForStateContext(ctx) - return err + if output, ok := outputRaw.(*ram.ResourceShareAssociation); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + + return output, err + } + + return nil, err } -func WaitForResourceShareResourceDisassociation(ctx context.Context, conn *ram.RAM, resourceShareARN, resourceARN string) error { +func waitResourceAssociationDeleted(ctx context.Context, conn *ram.RAM, resourceShareARN, resourceARN string) (*ram.ResourceShareAssociation, error) { + const ( + timeout = 5 * time.Minute + ) stateConf := &retry.StateChangeConf{ Pending: []string{ram.ResourceShareAssociationStatusAssociated, ram.ResourceShareAssociationStatusDisassociating}, Target: []string{ram.ResourceShareAssociationStatusDisassociated}, - Refresh: resourceAssociationStateRefreshFunc(ctx, conn, resourceShareARN, resourceARN), - Timeout: 5 * time.Minute, + Refresh: statusResourceAssociation(ctx, conn, resourceShareARN, resourceARN), + Timeout: timeout, } - _, err := stateConf.WaitForStateContext(ctx) + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*ram.ResourceShareAssociation); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + + return output, err + } - return err + return nil, err } diff --git a/internal/service/ram/resource_association_test.go b/internal/service/ram/resource_association_test.go index ee28cd79c39e..bdbfbdec8dc4 100644 --- a/internal/service/ram/resource_association_test.go +++ b/internal/service/ram/resource_association_test.go @@ -8,7 +8,6 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ram" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -22,7 +21,7 @@ import ( func TestAccRAMResourceAssociation_basic(t *testing.T) { ctx := acctest.Context(t) - var resourceShareAssociation1 ram.ResourceShareAssociation + var resourceShareAssociation ram.ResourceShareAssociation resourceName := "aws_ram_resource_association.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -35,7 +34,7 @@ func TestAccRAMResourceAssociation_basic(t *testing.T) { { Config: testAccResourceAssociationConfig_basic(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckResourceAssociationExists(ctx, resourceName, &resourceShareAssociation1), + testAccCheckResourceAssociationExists(ctx, resourceName, &resourceShareAssociation), ), }, { @@ -49,7 +48,7 @@ func TestAccRAMResourceAssociation_basic(t *testing.T) { func TestAccRAMResourceAssociation_disappears(t *testing.T) { ctx := acctest.Context(t) - var resourceShareAssociation1 ram.ResourceShareAssociation + var resourceShareAssociation ram.ResourceShareAssociation resourceName := "aws_ram_resource_association.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -62,8 +61,8 @@ func TestAccRAMResourceAssociation_disappears(t *testing.T) { { Config: testAccResourceAssociationConfig_basic(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckResourceAssociationExists(ctx, resourceName, &resourceShareAssociation1), - testAccCheckResourceAssociationDisappears(ctx, &resourceShareAssociation1), + testAccCheckResourceAssociationExists(ctx, resourceName, &resourceShareAssociation), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfram.ResourceResourceAssociation(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -71,53 +70,22 @@ func TestAccRAMResourceAssociation_disappears(t *testing.T) { }) } -func testAccCheckResourceAssociationDisappears(ctx context.Context, resourceShareAssociation *ram.ResourceShareAssociation) resource.TestCheckFunc { +func testAccCheckResourceAssociationExists(ctx context.Context, n string, v *ram.ResourceShareAssociation) resource.TestCheckFunc { return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).RAMConn(ctx) - - input := &ram.DisassociateResourceShareInput{ - ResourceArns: []*string{resourceShareAssociation.AssociatedEntity}, - ResourceShareArn: resourceShareAssociation.ResourceShareArn, - } - - _, err := conn.DisassociateResourceShareWithContext(ctx, input) - if err != nil { - return err + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) } - return tfram.WaitForResourceShareResourceDisassociation(ctx, conn, aws.StringValue(resourceShareAssociation.ResourceShareArn), aws.StringValue(resourceShareAssociation.AssociatedEntity)) - } -} - -func testAccCheckResourceAssociationExists(ctx context.Context, resourceName string, association *ram.ResourceShareAssociation) resource.TestCheckFunc { - return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).RAMConn(ctx) - rs, ok := s.RootModule().Resources[resourceName] - - if !ok { - return fmt.Errorf("Not found: %s", resourceName) - } - - resourceShareARN, resourceARN, err := tfram.DecodeResourceAssociationID(rs.Primary.ID) + output, err := tfram.FindResourceAssociationByTwoPartKey(ctx, conn, rs.Primary.Attributes["resource_share_arn"], rs.Primary.Attributes["resource_arn"]) if err != nil { return err } - resourceShareAssociation, err := tfram.GetResourceShareAssociation(ctx, conn, resourceShareARN, resourceARN) - if tfresource.NotFound(err) { - return fmt.Errorf("RAM Resource Share (%s) Resource Association (%s) not found", resourceShareARN, resourceARN) - } - if err != nil { - return fmt.Errorf("error reading RAM Resource Share (%s) Resource Association (%s): %s", resourceShareARN, resourceARN, err) - } - - if aws.StringValue(resourceShareAssociation.Status) != ram.ResourceShareAssociationStatusAssociated { - return fmt.Errorf("RAM Resource Share (%s) Resource Association (%s) not associated: %s", resourceShareARN, resourceARN, aws.StringValue(resourceShareAssociation.Status)) - } - - *association = *resourceShareAssociation + *v = *output return nil } @@ -132,23 +100,17 @@ func testAccCheckResourceAssociationDestroy(ctx context.Context) resource.TestCh continue } - resourceShareARN, resourceARN, err := tfram.DecodeResourceAssociationID(rs.Primary.ID) + _, err := tfram.FindResourceAssociationByTwoPartKey(ctx, conn, rs.Primary.Attributes["resource_share_arn"], rs.Primary.Attributes["resource_arn"]) - if err != nil { - return err - } - - resourceShareAssociation, err := tfram.GetResourceShareAssociation(ctx, conn, resourceShareARN, resourceARN) if tfresource.NotFound(err) { - return nil + continue } + if err != nil { return err } - if resourceShareAssociation != nil && aws.StringValue(resourceShareAssociation.Status) != ram.ResourceShareAssociationStatusDisassociated { - return fmt.Errorf("RAM Resource Share (%s) Resource Association (%s) not disassociated: %s", resourceShareARN, resourceARN, aws.StringValue(resourceShareAssociation.Status)) - } + return fmt.Errorf("RAM Resource Association %s still exists", rs.Primary.ID) } return nil diff --git a/internal/service/ram/status.go b/internal/service/ram/status.go index 2c90d15ee35e..c1c213d3cd09 100644 --- a/internal/service/ram/status.go +++ b/internal/service/ram/status.go @@ -22,7 +22,7 @@ const ( func StatusResourceSharePrincipalAssociation(ctx context.Context, conn *ram.RAM, resourceShareArn, principal string) retry.StateRefreshFunc { return func() (interface{}, string, error) { - association, err := FindResourceSharePrincipalAssociationByShareARNPrincipal(ctx, conn, resourceShareArn, principal) + association, err := findResourceShareAssociationByShareARNAndPrincipal(ctx, conn, resourceShareArn, principal) if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { return nil, PrincipalAssociationStatusNotFound, err From 642077dfdf3b2f470f137bb636232b67426739a9 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 08:52:00 -0400 Subject: [PATCH 18/30] r/aws_ram_resource_association: Fix acceptance tests. --- internal/service/ram/resource_association.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/ram/resource_association.go b/internal/service/ram/resource_association.go index 916414013740..e72265394c13 100644 --- a/internal/service/ram/resource_association.go +++ b/internal/service/ram/resource_association.go @@ -250,7 +250,7 @@ func waitResourceAssociationDeleted(ctx context.Context, conn *ram.RAM, resource ) stateConf := &retry.StateChangeConf{ Pending: []string{ram.ResourceShareAssociationStatusAssociated, ram.ResourceShareAssociationStatusDisassociating}, - Target: []string{ram.ResourceShareAssociationStatusDisassociated}, + Target: []string{}, Refresh: statusResourceAssociation(ctx, conn, resourceShareARN, resourceARN), Timeout: timeout, } From f68d43c9859e74078828eb68f719a56a2c4b5da5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 09:10:48 -0400 Subject: [PATCH 19/30] Add 'IsAWSAccountID'. --- internal/types/aws_account_id.go | 13 +++++++++++++ internal/types/aws_account_id_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 internal/types/aws_account_id.go create mode 100644 internal/types/aws_account_id_test.go diff --git a/internal/types/aws_account_id.go b/internal/types/aws_account_id.go new file mode 100644 index 000000000000..4b9301c7b1f5 --- /dev/null +++ b/internal/types/aws_account_id.go @@ -0,0 +1,13 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package types + +import ( + "github.com/YakDriver/regexache" +) + +// IsAWSAccountID returns whether or not the specified string is a valid AWS account ID. +func IsAWSAccountID(s string) bool { + return regexache.MustCompile(`^\d{12}$`).MatchString(s) +} diff --git a/internal/types/aws_account_id_test.go b/internal/types/aws_account_id_test.go new file mode 100644 index 000000000000..491d8e614a4b --- /dev/null +++ b/internal/types/aws_account_id_test.go @@ -0,0 +1,26 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package types + +import "testing" + +func TestIsAWSAccountID(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + id string + valid bool + }{ + {"123456789012", true}, + {"1234567890123", false}, + {"12345678901", false}, + {"", false}, + {"1234567890I2", false}, + } { + ok := IsAWSAccountID(tc.id) + if got, want := ok, tc.valid; got != want { + t.Errorf("IsAWSAccountID(%q) = %v, want %v", tc.id, got, want) + } + } +} From df6bb85e9f3f4fd7ffcd5b172f854c8eb444b00e Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 09:14:41 -0400 Subject: [PATCH 20/30] Use 'IsAWSAccountID'. --- internal/framework/validators/aws_account_id.go | 4 ++-- internal/types/aws_account_id.go | 1 + internal/verify/validate.go | 12 +++++------- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/framework/validators/aws_account_id.go b/internal/framework/validators/aws_account_id.go index d748d3ec2547..9aba4a671254 100644 --- a/internal/framework/validators/aws_account_id.go +++ b/internal/framework/validators/aws_account_id.go @@ -6,9 +6,9 @@ package validators import ( "context" - "github.com/YakDriver/regexache" "github.com/hashicorp/terraform-plugin-framework-validators/helpers/validatordiag" "github.com/hashicorp/terraform-plugin-framework/schema/validator" + itypes "github.com/hashicorp/terraform-provider-aws/internal/types" ) // awsAccountIDValidator validates that a string Attribute's value is a valid AWS account ID. @@ -31,7 +31,7 @@ func (validator awsAccountIDValidator) ValidateString(ctx context.Context, reque } // https://docs.aws.amazon.com/accounts/latest/reference/manage-acct-identifiers.html. - if !regexache.MustCompile(`^\d{12}$`).MatchString(request.ConfigValue.ValueString()) { + if !itypes.IsAWSAccountID(request.ConfigValue.ValueString()) { response.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( request.Path, validator.Description(ctx), diff --git a/internal/types/aws_account_id.go b/internal/types/aws_account_id.go index 4b9301c7b1f5..6569333e93a5 100644 --- a/internal/types/aws_account_id.go +++ b/internal/types/aws_account_id.go @@ -9,5 +9,6 @@ import ( // IsAWSAccountID returns whether or not the specified string is a valid AWS account ID. func IsAWSAccountID(s string) bool { + // https://docs.aws.amazon.com/accounts/latest/reference/manage-acct-identifiers.html. return regexache.MustCompile(`^\d{12}$`).MatchString(s) } diff --git a/internal/verify/validate.go b/internal/verify/validate.go index 498bc4f86a9d..509b72dfee75 100644 --- a/internal/verify/validate.go +++ b/internal/verify/validate.go @@ -20,7 +20,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/errs" - "github.com/hashicorp/terraform-provider-aws/internal/types" + itypes "github.com/hashicorp/terraform-provider-aws/internal/types" "github.com/hashicorp/terraform-provider-aws/internal/types/timestamp" ) @@ -130,9 +130,7 @@ func ValidARNCheck(f ...ARNCheckFunc) schema.SchemaValidateFunc { func ValidAccountID(v interface{}, k string) (ws []string, errors []error) { value := v.(string) - // http://docs.aws.amazon.com/lambda/latest/dg/API_AddPermission.html - pattern := `^\d{12}$` - if !regexache.MustCompile(pattern).MatchString(value) { + if !itypes.IsAWSAccountID(value) { errors = append(errors, fmt.Errorf( "%q doesn't look like AWS Account ID (exactly 12 digits): %q", k, value)) @@ -156,7 +154,7 @@ func ValidBase64String(v interface{}, k string) (ws []string, errors []error) { // ValidCIDRNetworkAddress ensures that the string value is a valid CIDR that // represents a network address - it adds an error otherwise func ValidCIDRNetworkAddress(v interface{}, k string) (ws []string, errors []error) { - if err := types.ValidateCIDRBlock(v.(string)); err != nil { + if err := itypes.ValidateCIDRBlock(v.(string)); err != nil { errors = append(errors, err) return } @@ -223,7 +221,7 @@ func ValidateIPv4CIDRBlock(cidr string) error { return fmt.Errorf("%q is not a valid IPv4 CIDR block", cidr) } - if !types.CIDRBlocksEqual(cidr, ipnet.String()) { + if !itypes.CIDRBlocksEqual(cidr, ipnet.String()) { return fmt.Errorf("%q is not a valid IPv4 CIDR block; did you mean %q?", cidr, ipnet) } @@ -245,7 +243,7 @@ func ValidateIPv6CIDRBlock(cidr string) error { return fmt.Errorf("%q is not a valid IPv6 CIDR block", cidr) } - if !types.CIDRBlocksEqual(cidr, ipnet.String()) { + if !itypes.CIDRBlocksEqual(cidr, ipnet.String()) { return fmt.Errorf("%q is not a valid IPv6 CIDR block; did you mean %q?", cidr, ipnet) } From d99ba73827ef9f72ad19b117b35178a5843e70f4 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 10:44:29 -0400 Subject: [PATCH 21/30] r/aws_ram_principal_association: Tidy up. --- internal/service/ram/exports_test.go | 8 +- internal/service/ram/principal_association.go | 169 +++++++++++------- .../service/ram/principal_association_test.go | 52 ++---- internal/service/ram/resource_association.go | 10 +- internal/service/ram/status.go | 46 ----- internal/service/ram/wait.go | 51 ------ 6 files changed, 126 insertions(+), 210 deletions(-) delete mode 100644 internal/service/ram/status.go delete mode 100644 internal/service/ram/wait.go diff --git a/internal/service/ram/exports_test.go b/internal/service/ram/exports_test.go index a18d20104e44..7e0552e35fb3 100644 --- a/internal/service/ram/exports_test.go +++ b/internal/service/ram/exports_test.go @@ -11,8 +11,8 @@ var ( ResourceResourceShareAccepter = resourceResourceShareAccepter ResourceSharingWithOrganization = resourceSharingWithOrganization - FindResourceAssociationByTwoPartKey = findResourceAssociationByTwoPartKey - FindResourceShareAssociationByShareARNAndPrincipal = findResourceShareAssociationByShareARNAndPrincipal - FindResourceShareOwnerOtherAccountsByARN = findResourceShareOwnerOtherAccountsByARN - FindResourceShareOwnerSelfByARN = findResourceShareOwnerSelfByARN + FindPrincipalAssociationByTwoPartKey = findPrincipalAssociationByTwoPartKey + FindResourceAssociationByTwoPartKey = findResourceAssociationByTwoPartKey + FindResourceShareOwnerOtherAccountsByARN = findResourceShareOwnerOtherAccountsByARN + FindResourceShareOwnerSelfByARN = findResourceShareOwnerSelfByARN ) diff --git a/internal/service/ram/principal_association.go b/internal/service/ram/principal_association.go index f8c894df2afe..bda88a7f6beb 100644 --- a/internal/service/ram/principal_association.go +++ b/internal/service/ram/principal_association.go @@ -5,21 +5,24 @@ package ram import ( "context" - "fmt" + "errors" "log" - "regexp" - "strings" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ram" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/id" + sdkid "github.com/hashicorp/terraform-plugin-sdk/v2/helper/id" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/errs" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + "github.com/hashicorp/terraform-provider-aws/internal/flex" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + itypes "github.com/hashicorp/terraform-provider-aws/internal/types" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) @@ -54,34 +57,37 @@ func resourcePrincipalAssociation() *schema.Resource { } } +const ( + principalAssociationResourceIDPartCount = 2 +) + func resourcePrincipalAssociationCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).RAMConn(ctx) - resourceShareArn := d.Get("resource_share_arn").(string) - principal := d.Get("principal").(string) - - request := &ram.AssociateResourceShareInput{ - ClientToken: aws.String(id.UniqueId()), - ResourceShareArn: aws.String(resourceShareArn), + resourceShareARN, principal := d.Get("resource_share_arn").(string), d.Get("principal").(string) + id := errs.Must(flex.FlattenResourceId([]string{resourceShareARN, principal}, principalAssociationResourceIDPartCount, false)) + input := &ram.AssociateResourceShareInput{ + ClientToken: aws.String(sdkid.UniqueId()), Principals: []*string{aws.String(principal)}, + ResourceShareArn: aws.String(resourceShareARN), } - log.Println("[DEBUG] Create RAM principal association request:", request) - _, err := conn.AssociateResourceShareWithContext(ctx, request) + _, err := conn.AssociateResourceShareWithContext(ctx, input) + if err != nil { - return sdkdiag.AppendErrorf(diags, "associating principal with RAM resource share: %s", err) + return sdkdiag.AppendErrorf(diags, "creating RAM Principal Association (%s): %s", id, err) } - d.SetId(fmt.Sprintf("%s,%s", resourceShareArn, principal)) + d.SetId(id) - // AWS Account ID Principals need to be accepted to become ASSOCIATED - if ok, _ := regexp.MatchString(`^\d{12}$`, principal); ok { + // AWS Account ID principals need to be accepted to become ASSOCIATED. + if itypes.IsAWSAccountID(principal) { return append(diags, resourcePrincipalAssociationRead(ctx, d, meta)...) } - if _, err := WaitResourceSharePrincipalAssociated(ctx, conn, resourceShareArn, principal); err != nil { - return sdkdiag.AppendErrorf(diags, "waiting for RAM principal association (%s) to become ready: %s", d.Id(), err) + if _, err := waitPrincipalAssociationCreated(ctx, conn, resourceShareARN, principal); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for RAM Principal Association (%s) create: %s", d.Id(), err) } return append(diags, resourcePrincipalAssociationRead(ctx, d, meta)...) @@ -91,42 +97,26 @@ func resourcePrincipalAssociationRead(ctx context.Context, d *schema.ResourceDat var diags diag.Diagnostics conn := meta.(*conns.AWSClient).RAMConn(ctx) - resourceShareArn, principal, err := PrincipalAssociationParseID(d.Id()) + parts, err := flex.ExpandResourceId(d.Id(), principalAssociationResourceIDPartCount, false) if err != nil { - return sdkdiag.AppendErrorf(diags, "reading RAM Principal Association, parsing ID (%s): %s", d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } + resourceShareARN, principal := parts[0], parts[1] - var association *ram.ResourceShareAssociation - - if ok, _ := regexp.MatchString(`^\d{12}$`, principal); ok { - // AWS Account ID Principals need to be accepted to become ASSOCIATED - association, err = findResourceShareAssociationByShareARNAndPrincipal(ctx, conn, resourceShareArn, principal) - } else { - association, err = WaitResourceSharePrincipalAssociated(ctx, conn, resourceShareArn, principal) - } + principalAssociation, err := findPrincipalAssociationByTwoPartKey(ctx, conn, resourceShareARN, principal) - if !d.IsNewResource() && (tfawserr.ErrCodeEquals(err, ram.ErrCodeResourceArnNotFoundException) || tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException)) { - log.Printf("[WARN] No RAM resource share principal association with ARN (%s) found, removing from state", d.Id()) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] RAM Resource Association %s not found, removing from state", d.Id()) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "reading RAM Resource Share (%s) Principal Association (%s): %s", resourceShareArn, principal, err) + return sdkdiag.AppendErrorf(diags, "reading RAM Resource Association (%s): %s", d.Id(), err) } - if !d.IsNewResource() && (association == nil || aws.StringValue(association.Status) == ram.ResourceShareAssociationStatusDisassociated) { - log.Printf("[WARN] RAM resource share principal association with ARN (%s) found, but empty or disassociated - removing from state", d.Id()) - d.SetId("") - return diags - } - - if aws.StringValue(association.Status) != ram.ResourceShareAssociationStatusAssociated && aws.StringValue(association.Status) != ram.ResourceShareAssociationStatusAssociating { - return sdkdiag.AppendErrorf(diags, "reading RAM Resource Share (%s) Principal Association (%s), status not associating or associated: %s", resourceShareArn, principal, aws.StringValue(association.Status)) - } - - d.Set("resource_share_arn", resourceShareArn) - d.Set("principal", principal) + d.Set("principal", principalAssociation.AssociatedEntity) + d.Set("resource_share_arn", principalAssociation.ResourceShareArn) return diags } @@ -135,46 +125,34 @@ func resourcePrincipalAssociationDelete(ctx context.Context, d *schema.ResourceD var diags diag.Diagnostics conn := meta.(*conns.AWSClient).RAMConn(ctx) - resourceShareArn, principal, err := PrincipalAssociationParseID(d.Id()) + parts, err := flex.ExpandResourceId(d.Id(), principalAssociationResourceIDPartCount, false) if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting RAM Resource Share Principal Association (%s): %s", d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } + resourceShareARN, principal := parts[0], parts[1] - request := &ram.DisassociateResourceShareInput{ - ResourceShareArn: aws.String(resourceShareArn), + log.Printf("[DEBUG] Deleting RAM Principal Association: %s", d.Id()) + _, err = conn.DisassociateResourceShareWithContext(ctx, &ram.DisassociateResourceShareInput{ Principals: []*string{aws.String(principal)}, - } - - log.Println("[DEBUG] Delete RAM principal association request:", request) - _, err = conn.DisassociateResourceShareWithContext(ctx, request) + ResourceShareArn: aws.String(resourceShareARN), + }) if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting RAM Resource Share Principal Association (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "deleting RAM Principal Association (%s): %s", d.Id(), err) } - if _, err := WaitResourceSharePrincipalDisassociated(ctx, conn, resourceShareArn, principal); err != nil { - return sdkdiag.AppendErrorf(diags, "deleting RAM Resource Share Principal Association (%s): waiting for completion: %s", d.Id(), err) + if _, err := waitPrincipalAssociationDeleted(ctx, conn, resourceShareARN, principal); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for RAM Principal Association (%s) delete: %s", d.Id(), err) } return diags } -func PrincipalAssociationParseID(id string) (string, string, error) { - idFormatErr := fmt.Errorf("unexpected format of ID (%s), expected SHARE,PRINCIPAL", id) - - parts := strings.SplitN(id, ",", 2) - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return "", "", idFormatErr - } - - return parts[0], parts[1], nil -} - -func findResourceShareAssociationByShareARNAndPrincipal(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { +func findPrincipalAssociationByTwoPartKey(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { input := &ram.GetResourceShareAssociationsInput{ AssociationType: aws.String(ram.ResourceShareAssociationTypePrincipal), Principal: aws.String(principal), @@ -196,3 +174,62 @@ func findResourceShareAssociationByShareARNAndPrincipal(ctx context.Context, con return output, err } + +func statusPrincipalAssociation(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) retry.StateRefreshFunc { + return func() (interface{}, string, error) { + output, err := findPrincipalAssociationByTwoPartKey(ctx, conn, resourceShareARN, principal) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return output, aws.StringValue(output.Status), nil + } +} + +func waitPrincipalAssociationCreated(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { + const ( + timeout = 3 * time.Minute + ) + stateConf := &retry.StateChangeConf{ + Pending: []string{ram.ResourceShareAssociationStatusAssociating}, + Target: []string{ram.ResourceShareAssociationStatusAssociated}, + Refresh: statusPrincipalAssociation(ctx, conn, resourceShareARN, principal), + Timeout: timeout, + NotFoundChecks: 20, + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*ram.ResourceShareAssociation); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.StatusMessage))) + + return output, err + } + + return nil, err +} + +func waitPrincipalAssociationDeleted(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { + const ( + timeout = 3 * time.Minute + ) + stateConf := &retry.StateChangeConf{ + Pending: []string{ram.ResourceShareAssociationStatusAssociated, ram.ResourceShareAssociationStatusDisassociating}, + Target: []string{}, + Refresh: statusPrincipalAssociation(ctx, conn, resourceShareARN, principal), + Timeout: timeout, + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if v, ok := outputRaw.(*ram.ResourceShareAssociation); ok { + return v, err + } + + return nil, err +} diff --git a/internal/service/ram/principal_association_test.go b/internal/service/ram/principal_association_test.go index 8e92875db84f..a1a8be40aaa4 100644 --- a/internal/service/ram/principal_association_test.go +++ b/internal/service/ram/principal_association_test.go @@ -6,10 +6,8 @@ package ram_test import ( "context" "fmt" - "regexp" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ram" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -17,6 +15,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfram "github.com/hashicorp/terraform-provider-aws/internal/service/ram" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -71,47 +70,22 @@ func TestAccRAMPrincipalAssociation_disappears(t *testing.T) { }) } -func testAccCheckPrincipalAssociationExists(ctx context.Context, resourceName string, resourceShare *ram.ResourceShareAssociation) resource.TestCheckFunc { +func testAccCheckPrincipalAssociationExists(ctx context.Context, n string, v *ram.ResourceShareAssociation) resource.TestCheckFunc { return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).RAMConn(ctx) - - rs, ok := s.RootModule().Resources[resourceName] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not found: %s", resourceName) + return fmt.Errorf("Not found: %s", n) } - if rs.Primary.ID == "" { - return fmt.Errorf("No ID is set") - } - - resourceShareARN, principal, err := tfram.PrincipalAssociationParseID(rs.Primary.ID) - - if err != nil { - return fmt.Errorf("error parsing ID (%s): %w", rs.Primary.ID, err) - } - - var association *ram.ResourceShareAssociation + conn := acctest.Provider.Meta().(*conns.AWSClient).RAMConn(ctx) - if ok, _ := regexp.MatchString(`^\d{12}$`, principal); ok { - // AWS Account ID Principals need to be accepted to become ASSOCIATED - association, err = tfram.FindResourceShareAssociationByShareARNAndPrincipal(ctx, conn, resourceShareARN, principal) - } else { - association, err = tfram.WaitResourceSharePrincipalAssociated(ctx, conn, resourceShareARN, principal) - } + output, err := tfram.FindPrincipalAssociationByTwoPartKey(ctx, conn, rs.Primary.Attributes["resource_share_arn"], rs.Primary.Attributes["principal"]) if err != nil { - return fmt.Errorf("error reading RAM Resource Share (%s) Principal Association (%s): %s", resourceShareARN, principal, err) - } - - if association == nil { - return fmt.Errorf("RAM Resource Share (%s) Principal Association (%s) not found", resourceShareARN, principal) - } - - if aws.StringValue(association.Status) != ram.ResourceShareAssociationStatusAssociated && aws.StringValue(association.Status) != ram.ResourceShareAssociationStatusAssociating { - return fmt.Errorf("RAM Resource Share (%s) Principal Association (%s) status not associating or associated: %s", resourceShareARN, principal, aws.StringValue(association.Status)) + return err } - *resourceShare = *association + *v = *output return nil } @@ -126,15 +100,17 @@ func testAccCheckPrincipalAssociationDestroy(ctx context.Context) resource.TestC continue } - association, err := tfram.WaitResourceSharePrincipalDisassociated(ctx, conn, rs.Primary.Attributes["resource_share_arn"], rs.Primary.Attributes["principal"]) + _, err := tfram.FindPrincipalAssociationByTwoPartKey(ctx, conn, rs.Primary.Attributes["resource_share_arn"], rs.Primary.Attributes["principal"]) + + if tfresource.NotFound(err) { + continue + } if err != nil { return err } - if association != nil && aws.StringValue(association.Status) != ram.ResourceShareAssociationStatusDisassociated { - return fmt.Errorf("RAM Principal Association: %s", rs.Primary.ID) - } + return fmt.Errorf("RAM Resource Association %s still exists", rs.Primary.ID) } return nil diff --git a/internal/service/ram/resource_association.go b/internal/service/ram/resource_association.go index e72265394c13..0254918d3446 100644 --- a/internal/service/ram/resource_association.go +++ b/internal/service/ram/resource_association.go @@ -93,7 +93,7 @@ func resourceResourceAssociationRead(ctx context.Context, d *schema.ResourceData } resourceShareARN, resourceARN := parts[0], parts[1] - resourceShareAssociation, err := findResourceAssociationByTwoPartKey(ctx, conn, resourceShareARN, resourceARN) + resourceAssociation, err := findResourceAssociationByTwoPartKey(ctx, conn, resourceShareARN, resourceARN) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] RAM Resource Association %s not found, removing from state", d.Id()) @@ -105,8 +105,8 @@ func resourceResourceAssociationRead(ctx context.Context, d *schema.ResourceData return sdkdiag.AppendErrorf(diags, "reading RAM Resource Association (%s): %s", d.Id(), err) } - d.Set("resource_arn", resourceShareAssociation.AssociatedEntity) - d.Set("resource_share_arn", resourceShareAssociation.ResourceShareArn) + d.Set("resource_arn", resourceAssociation.AssociatedEntity) + d.Set("resource_share_arn", resourceAssociation.ResourceShareArn) return diags } @@ -121,7 +121,7 @@ func resourceResourceAssociationDelete(ctx context.Context, d *schema.ResourceDa } resourceShareARN, resourceARN := parts[0], parts[1] - log.Printf("[DEBUG] Deleting RAM Resource Associating: %s", d.Id()) + log.Printf("[DEBUG] Deleting RAM Resource Association: %s", d.Id()) _, err = conn.DisassociateResourceShareWithContext(ctx, &ram.DisassociateResourceShareInput{ ResourceArns: aws.StringSlice([]string{resourceARN}), ResourceShareArn: aws.String(resourceShareARN), @@ -192,7 +192,7 @@ func findResourceShareAssociations(ctx context.Context, conn *ram.RAM, input *ra return !lastPage }) - if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { + if tfawserr.ErrCodeEquals(err, ram.ErrCodeResourceArnNotFoundException, ram.ErrCodeUnknownResourceException) { return nil, &retry.NotFoundError{ LastError: err, LastRequest: input, diff --git a/internal/service/ram/status.go b/internal/service/ram/status.go deleted file mode 100644 index c1c213d3cd09..000000000000 --- a/internal/service/ram/status.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package ram - -import ( - "context" - "fmt" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ram" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" -) - -const ( - ResourceShareInvitationStatusNotFound = "NotFound" - ResourceShareInvitationStatusUnknown = "Unknown" - - PrincipalAssociationStatusNotFound = "NotFound" -) - -func StatusResourceSharePrincipalAssociation(ctx context.Context, conn *ram.RAM, resourceShareArn, principal string) retry.StateRefreshFunc { - return func() (interface{}, string, error) { - association, err := findResourceShareAssociationByShareARNAndPrincipal(ctx, conn, resourceShareArn, principal) - - if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { - return nil, PrincipalAssociationStatusNotFound, err - } - - if err != nil { - return nil, ram.ResourceShareAssociationStatusFailed, err - } - - if association == nil { - return nil, ram.ResourceShareAssociationStatusDisassociated, nil - } - - if aws.StringValue(association.Status) == ram.ResourceShareAssociationStatusFailed { - extendedErr := fmt.Errorf("association status message: %s", aws.StringValue(association.StatusMessage)) - return association, aws.StringValue(association.Status), extendedErr - } - - return association, aws.StringValue(association.Status), nil - } -} diff --git a/internal/service/ram/wait.go b/internal/service/ram/wait.go deleted file mode 100644 index 95af2eb0d06f..000000000000 --- a/internal/service/ram/wait.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package ram - -import ( - "context" - "time" - - "github.com/aws/aws-sdk-go/service/ram" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" -) - -const ( - PrincipalAssociationTimeout = 3 * time.Minute - PrincipalDisassociationTimeout = 3 * time.Minute -) - -func WaitResourceSharePrincipalAssociated(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { - stateConf := &retry.StateChangeConf{ - Pending: []string{ram.ResourceShareAssociationStatusAssociating, PrincipalAssociationStatusNotFound}, - Target: []string{ram.ResourceShareAssociationStatusAssociated}, - Refresh: StatusResourceSharePrincipalAssociation(ctx, conn, resourceShareARN, principal), - Timeout: PrincipalAssociationTimeout, - } - - outputRaw, err := stateConf.WaitForStateContext(ctx) - - if v, ok := outputRaw.(*ram.ResourceShareAssociation); ok { - return v, err - } - - return nil, err -} - -func WaitResourceSharePrincipalDisassociated(ctx context.Context, conn *ram.RAM, resourceShareARN, principal string) (*ram.ResourceShareAssociation, error) { - stateConf := &retry.StateChangeConf{ - Pending: []string{ram.ResourceShareAssociationStatusAssociated, ram.ResourceShareAssociationStatusDisassociating}, - Target: []string{ram.ResourceShareAssociationStatusDisassociated, PrincipalAssociationStatusNotFound}, - Refresh: StatusResourceSharePrincipalAssociation(ctx, conn, resourceShareARN, principal), - Timeout: PrincipalDisassociationTimeout, - } - - outputRaw, err := stateConf.WaitForStateContext(ctx) - - if v, ok := outputRaw.(*ram.ResourceShareAssociation); ok { - return v, err - } - - return nil, err -} From 40859480c6917a3a3c410e984a2c307dfe7e19fb Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 10:54:59 -0400 Subject: [PATCH 22/30] Add CHANGELOG entry for #36070. --- .changelog/36062.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.changelog/36062.txt b/.changelog/36062.txt index 119040e66b2d..7f851cd5f049 100644 --- a/.changelog/36062.txt +++ b/.changelog/36062.txt @@ -4,4 +4,8 @@ data-source/aws_ram_resource_share: `name` is Optional ```release-note:enhancement resource/aws_ram_resource_association: Add plan-time validation of `resource_arn` and `resource_share_arn` +``` + +```release-note:bug +resource/aws_ram_principal_association: Remove from state on resource Read if `principal` is disassociated outside of Terraform ``` \ No newline at end of file From 1002297f21b56e110a8c120b20dad90ff060a9e7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 11:50:06 -0400 Subject: [PATCH 23/30] r/aws_ram_principal_association: Additional acceptance tests. --- internal/service/ram/exports_test.go | 1 + .../service/ram/principal_association_test.go | 91 ++++++++++++++++++- .../service/ram/sharing_with_organization.go | 49 ++++++---- 3 files changed, 117 insertions(+), 24 deletions(-) diff --git a/internal/service/ram/exports_test.go b/internal/service/ram/exports_test.go index 7e0552e35fb3..564d666be9c0 100644 --- a/internal/service/ram/exports_test.go +++ b/internal/service/ram/exports_test.go @@ -15,4 +15,5 @@ var ( FindResourceAssociationByTwoPartKey = findResourceAssociationByTwoPartKey FindResourceShareOwnerOtherAccountsByARN = findResourceShareOwnerOtherAccountsByARN FindResourceShareOwnerSelfByARN = findResourceShareOwnerSelfByARN + FindSharingWithOrganization = findSharingWithOrganization ) diff --git a/internal/service/ram/principal_association_test.go b/internal/service/ram/principal_association_test.go index a1a8be40aaa4..0bbf2df81be4 100644 --- a/internal/service/ram/principal_association_test.go +++ b/internal/service/ram/principal_association_test.go @@ -26,7 +26,10 @@ func TestAccRAMPrincipalAssociation_basic(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + testAccPreCheckSharingWithOrganizationEnabled(ctx, t) + }, ErrorCheck: acctest.ErrorCheck(t, names.RAMServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckPrincipalAssociationDestroy(ctx), @@ -46,6 +49,36 @@ func TestAccRAMPrincipalAssociation_basic(t *testing.T) { }) } +func TestAccRAMPrincipalAssociation_AccountID(t *testing.T) { + ctx := acctest.Context(t) + var association ram.ResourceShareAssociation + resourceName := "aws_ram_principal_association.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckAlternateAccount(t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.RAMServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5FactoriesAlternate(ctx, t), + CheckDestroy: testAccCheckPrincipalAssociationDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccPrincipalAssociationConfig_accountID(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckPrincipalAssociationExists(ctx, resourceName, &association), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccRAMPrincipalAssociation_disappears(t *testing.T) { ctx := acctest.Context(t) var association ram.ResourceShareAssociation @@ -53,7 +86,10 @@ func TestAccRAMPrincipalAssociation_disappears(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + testAccPreCheckSharingWithOrganizationEnabled(ctx, t) + }, ErrorCheck: acctest.ErrorCheck(t, names.RAMServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckPrincipalAssociationDestroy(ctx), @@ -70,6 +106,18 @@ func TestAccRAMPrincipalAssociation_disappears(t *testing.T) { }) } +func testAccPreCheckSharingWithOrganizationEnabled(ctx context.Context, t *testing.T) { + err := tfram.FindSharingWithOrganization(ctx, acctest.Provider.Meta().(*conns.AWSClient)) + + if acctest.PreCheckSkipError(err) { + t.Skipf("skipping acceptance testing: %s", err) + } + + if tfresource.NotFound(err) { + t.Skipf("Sharing with AWS Organization not found, skipping acceptance test: %s", err) + } +} + func testAccCheckPrincipalAssociationExists(ctx context.Context, n string, v *ram.ResourceShareAssociation) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -120,13 +168,48 @@ func testAccCheckPrincipalAssociationDestroy(ctx context.Context) resource.TestC func testAccPrincipalAssociationConfig_basic(rName string) string { return fmt.Sprintf(` resource "aws_ram_resource_share" "test" { - allow_external_principals = true + allow_external_principals = false name = %[1]q } +data "aws_partition" "current" {} + +resource "aws_iam_role" "test" { + name = %[1]q + + assume_role_policy = jsonencode({ + Version = "2012-10-17" + Statement = [{ + Action = "sts:AssumeRole", + Principal = { + Service = "ec2.${data.aws_partition.current.dns_suffix}", + } + Effect = "Allow" + }] + }) +} + resource "aws_ram_principal_association" "test" { - principal = "111111111111" + principal = aws_iam_role.test.arn resource_share_arn = aws_ram_resource_share.test.id } `, rName) } + +func testAccPrincipalAssociationConfig_accountID(rName string) string { + return acctest.ConfigCompose(acctest.ConfigAlternateAccountProvider(), fmt.Sprintf(` +resource "aws_ram_resource_share" "test" { + allow_external_principals = true + name = %[1]q +} + +data "aws_caller_identity" "receiver" { + provider = "awsalternate" +} + +resource "aws_ram_principal_association" "test" { + principal = data.aws_caller_identity.receiver.account_id + resource_share_arn = aws_ram_resource_share.test.id +} +`, rName)) +} diff --git a/internal/service/ram/sharing_with_organization.go b/internal/service/ram/sharing_with_organization.go index cc93126476ad..30d54c180721 100644 --- a/internal/service/ram/sharing_with_organization.go +++ b/internal/service/ram/sharing_with_organization.go @@ -5,12 +5,14 @@ package ram import ( "context" + "fmt" "log" "slices" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ram" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" @@ -61,33 +63,16 @@ func resourceSharingWithOrganizationCreate(ctx context.Context, d *schema.Resour func resourceSharingWithOrganizationRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics - // See https://docs.aws.amazon.com/ram/latest/userguide/getting-started-sharing.html#getting-started-sharing-orgs. - // Check for IAM role and Organizations trusted access. - - _, err := tfiam.FindRoleByName(ctx, meta.(*conns.AWSClient).IAMConn(ctx), sharingWithOrganizationRoleName) + err := findSharingWithOrganization(ctx, meta.(*conns.AWSClient)) if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] IAM Role (%s) not found, removing from state", sharingWithOrganizationRoleName) + log.Printf("[WARN] RAM Sharing With Organization %s not found, removing from state", d.Id()) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM Role (%s): %s", sharingWithOrganizationRoleName, err) - } - - servicePrincipalNames, err := tforganizations.FindEnabledServicePrincipalNames(ctx, meta.(*conns.AWSClient).OrganizationsConn(ctx)) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading Organization service principals: %s", err) - } - - enabled := slices.Contains(servicePrincipalNames, servicePrincipalName) - - if !d.IsNewResource() && !enabled { - log.Printf("[WARN] Organization service principal (%s) not enabled, removing from state", servicePrincipalName) - d.SetId("") - return diags + return sdkdiag.AppendErrorf(diags, "reading RAM Sharing With Organization (%s): %s", d.Id(), err) } return diags @@ -108,3 +93,27 @@ func resourceSharingWithOrganizationDelete(ctx context.Context, d *schema.Resour return diags } + +func findSharingWithOrganization(ctx context.Context, awsClient *conns.AWSClient) error { + // See https://docs.aws.amazon.com/ram/latest/userguide/getting-started-sharing.html#getting-started-sharing-orgs. + // Check for IAM role and Organizations trusted access. + _, err := tfiam.FindRoleByName(ctx, awsClient.IAMConn(ctx), sharingWithOrganizationRoleName) + + if err != nil { + return fmt.Errorf("reading IAM Role (%s): %w", sharingWithOrganizationRoleName, err) + } + + servicePrincipalNames, err := tforganizations.FindEnabledServicePrincipalNames(ctx, awsClient.OrganizationsConn(ctx)) + + if err != nil { + return fmt.Errorf("reading Organization service principals: %w", err) + } + + if !slices.Contains(servicePrincipalNames, servicePrincipalName) { + return &retry.NotFoundError{ + Message: fmt.Sprintf("Organization service principal (%s) not enabled", servicePrincipalName), + } + } + + return nil +} From 857044b54d884b295729fbc8a6777a08871986ae Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 11:57:00 -0400 Subject: [PATCH 24/30] Add 'TestAccRAMPrincipalAssociation_duplicate'. --- .../service/ram/principal_association_test.go | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/internal/service/ram/principal_association_test.go b/internal/service/ram/principal_association_test.go index 0bbf2df81be4..24bd199fc0a2 100644 --- a/internal/service/ram/principal_association_test.go +++ b/internal/service/ram/principal_association_test.go @@ -106,6 +106,36 @@ func TestAccRAMPrincipalAssociation_disappears(t *testing.T) { }) } +func TestAccRAMPrincipalAssociation_duplicate(t *testing.T) { + ctx := acctest.Context(t) + var association ram.ResourceShareAssociation + resourceName := "aws_ram_principal_association.test1" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + // testAccPreCheckSharingWithOrganizationEnabled(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.RAMServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckPrincipalAssociationDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccPrincipalAssociationConfig_duplicate(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckPrincipalAssociationExists(ctx, resourceName, &association), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func testAccPreCheckSharingWithOrganizationEnabled(ctx context.Context, t *testing.T) { err := tfram.FindSharingWithOrganization(ctx, acctest.Provider.Meta().(*conns.AWSClient)) @@ -213,3 +243,39 @@ resource "aws_ram_principal_association" "test" { } `, rName)) } + +func testAccPrincipalAssociationConfig_duplicate(rName string) string { + return fmt.Sprintf(` +resource "aws_ram_resource_share" "test" { + allow_external_principals = false + name = %[1]q +} + +data "aws_partition" "current" {} + +resource "aws_iam_role" "test" { + name = %[1]q + + assume_role_policy = jsonencode({ + Version = "2012-10-17" + Statement = [{ + Action = "sts:AssumeRole", + Principal = { + Service = "ec2.${data.aws_partition.current.dns_suffix}", + } + Effect = "Allow" + }] + }) +} + +resource "aws_ram_principal_association" "test1" { + principal = aws_iam_role.test.arn + resource_share_arn = aws_ram_resource_share.test.id +} + +resource "aws_ram_principal_association" "test2" { + principal = aws_iam_role.test.arn + resource_share_arn = aws_ram_principal_association.test1.resource_share_arn +} +`, rName) +} From f743ef89176b3af86119dd393afcd9deca6d0d9f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 11:57:08 -0400 Subject: [PATCH 25/30] Acceptance test output: % make testacc TESTARGS='-run=TestAccRAMPrincipalAssociation_duplicate' PKG=ram ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.21.8 test ./internal/service/ram/... -v -count 1 -parallel 2 -run=TestAccRAMPrincipalAssociation_duplicate -timeout 360m === RUN TestAccRAMPrincipalAssociation_duplicate === PAUSE TestAccRAMPrincipalAssociation_duplicate === CONT TestAccRAMPrincipalAssociation_duplicate --- PASS: TestAccRAMPrincipalAssociation_duplicate (28.71s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/ram 35.841s From 5c33065bafba132b7b18305b2d996e6937ea2d8b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 12:10:48 -0400 Subject: [PATCH 26/30] r/aws_ram_principal_association: Prevent creation of duplicate Terraform resources. --- .changelog/36062.txt | 4 ++++ internal/service/ram/principal_association.go | 14 +++++++++++++- .../service/ram/principal_association_test.go | 16 ++++------------ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/.changelog/36062.txt b/.changelog/36062.txt index 7f851cd5f049..c7a1e2545380 100644 --- a/.changelog/36062.txt +++ b/.changelog/36062.txt @@ -8,4 +8,8 @@ resource/aws_ram_resource_association: Add plan-time validation of `resource_arn ```release-note:bug resource/aws_ram_principal_association: Remove from state on resource Read if `principal` is disassociated outside of Terraform +``` + +```release-note:bug +resource/aws_ram_principal_association: Prevent creation of duplicate Terraform resources ``` \ No newline at end of file diff --git a/internal/service/ram/principal_association.go b/internal/service/ram/principal_association.go index bda88a7f6beb..dd3fb3cdbf3a 100644 --- a/internal/service/ram/principal_association.go +++ b/internal/service/ram/principal_association.go @@ -6,6 +6,7 @@ package ram import ( "context" "errors" + "fmt" "log" "time" @@ -67,13 +68,24 @@ func resourcePrincipalAssociationCreate(ctx context.Context, d *schema.ResourceD resourceShareARN, principal := d.Get("resource_share_arn").(string), d.Get("principal").(string) id := errs.Must(flex.FlattenResourceId([]string{resourceShareARN, principal}, principalAssociationResourceIDPartCount, false)) + _, err := findPrincipalAssociationByTwoPartKey(ctx, conn, resourceShareARN, principal) + + switch { + case err == nil: + return sdkdiag.AppendFromErr(diags, fmt.Errorf("RAM Principal Association (%s) already exists", id)) + case tfresource.NotFound(err): + break + default: + return sdkdiag.AppendErrorf(diags, "reading RAM Principal Association: %s", err) + } + input := &ram.AssociateResourceShareInput{ ClientToken: aws.String(sdkid.UniqueId()), Principals: []*string{aws.String(principal)}, ResourceShareArn: aws.String(resourceShareARN), } - _, err := conn.AssociateResourceShareWithContext(ctx, input) + _, err = conn.AssociateResourceShareWithContext(ctx, input) if err != nil { return sdkdiag.AppendErrorf(diags, "creating RAM Principal Association (%s): %s", id, err) diff --git a/internal/service/ram/principal_association_test.go b/internal/service/ram/principal_association_test.go index 24bd199fc0a2..d17b95fd0871 100644 --- a/internal/service/ram/principal_association_test.go +++ b/internal/service/ram/principal_association_test.go @@ -8,6 +8,7 @@ import ( "fmt" "testing" + "github.com/YakDriver/regexache" "github.com/aws/aws-sdk-go/service/ram" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -108,29 +109,20 @@ func TestAccRAMPrincipalAssociation_disappears(t *testing.T) { func TestAccRAMPrincipalAssociation_duplicate(t *testing.T) { ctx := acctest.Context(t) - var association ram.ResourceShareAssociation - resourceName := "aws_ram_principal_association.test1" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) - // testAccPreCheckSharingWithOrganizationEnabled(ctx, t) + testAccPreCheckSharingWithOrganizationEnabled(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, names.RAMServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckPrincipalAssociationDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccPrincipalAssociationConfig_duplicate(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckPrincipalAssociationExists(ctx, resourceName, &association), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + Config: testAccPrincipalAssociationConfig_duplicate(rName), + ExpectError: regexache.MustCompile(`RAM Principal Association .* already exists`), }, }, }) From 47b1a1bd9c3fde02d886f95d0ffae7b0f475aaf2 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 12:14:59 -0400 Subject: [PATCH 27/30] Add 'TestAccRAMResourceAssociation_duplicate'. --- .../service/ram/resource_association_test.go | 59 +++++++++++++------ 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/internal/service/ram/resource_association_test.go b/internal/service/ram/resource_association_test.go index bdbfbdec8dc4..3165a106476f 100644 --- a/internal/service/ram/resource_association_test.go +++ b/internal/service/ram/resource_association_test.go @@ -70,6 +70,28 @@ func TestAccRAMResourceAssociation_disappears(t *testing.T) { }) } +func TestAccRAMResourceAssociation_duplicate(t *testing.T) { + ctx := acctest.Context(t) + var resourceShareAssociation ram.ResourceShareAssociation + resourceName := "aws_ram_resource_association.test1" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.RAMServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckResourceAssociationDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccResourceAssociationConfig_duplicate(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckResourceAssociationExists(ctx, resourceName, &resourceShareAssociation), + ), + }, + }, + }) +} + func testAccCheckResourceAssociationExists(ctx context.Context, n string, v *ram.ResourceShareAssociation) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -118,31 +140,32 @@ func testAccCheckResourceAssociationDestroy(ctx context.Context) resource.TestCh } func testAccResourceAssociationConfig_basic(rName string) string { - return fmt.Sprintf(` -resource "aws_vpc" "test" { - cidr_block = "10.0.0.0/16" - - tags = { - Name = "tf-acc-test-ram-resource-association" - } + return acctest.ConfigCompose(acctest.ConfigVPCWithSubnets(rName, 1), fmt.Sprintf(` +resource "aws_ram_resource_share" "test" { + name = %[1]q } -resource "aws_subnet" "test" { - cidr_block = "10.0.0.0/24" - vpc_id = aws_vpc.test.id - - tags = { - Name = "tf-acc-test-ram-resource-association" - } +resource "aws_ram_resource_association" "test" { + resource_arn = aws_subnet.test[0].arn + resource_share_arn = aws_ram_resource_share.test.id +} +`, rName)) } +func testAccResourceAssociationConfig_duplicate(rName string) string { + return acctest.ConfigCompose(acctest.ConfigVPCWithSubnets(rName, 1), fmt.Sprintf(` resource "aws_ram_resource_share" "test" { - name = %q + name = %[1]q } -resource "aws_ram_resource_association" "test" { - resource_arn = aws_subnet.test.arn +resource "aws_ram_resource_association" "test1" { + resource_arn = aws_subnet.test[0].arn + resource_share_arn = aws_ram_resource_share.test.id +} + +resource "aws_ram_resource_association" "test2" { + resource_arn = aws_subnet.test[0].arn resource_share_arn = aws_ram_resource_share.test.id } -`, rName) +`, rName)) } From cc36db5833aada6c541375f5b78d007da9ed24d0 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 12:15:06 -0400 Subject: [PATCH 28/30] Acceptance test output: % make testacc TESTARGS='-run=TestAccRAMResourceAssociation_duplicate' PKG=ram ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.21.8 test ./internal/service/ram/... -v -count 1 -parallel 2 -run=TestAccRAMResourceAssociation_duplicate -timeout 360m === RUN TestAccRAMResourceAssociation_duplicate === PAUSE TestAccRAMResourceAssociation_duplicate === CONT TestAccRAMResourceAssociation_duplicate --- PASS: TestAccRAMResourceAssociation_duplicate (27.12s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/ram 34.588s From c1b883d2a64829241dbe888ab63d18b5f619ed85 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 12:21:34 -0400 Subject: [PATCH 29/30] r/aws_ram_resource_association: Prevent creation of duplicate Terraform resources. --- .changelog/36062.txt | 4 ++++ internal/service/ram/resource_association.go | 14 +++++++++++++- internal/service/ram/resource_association_test.go | 11 ++++------- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.changelog/36062.txt b/.changelog/36062.txt index c7a1e2545380..072c0d101386 100644 --- a/.changelog/36062.txt +++ b/.changelog/36062.txt @@ -12,4 +12,8 @@ resource/aws_ram_principal_association: Remove from state on resource Read if `p ```release-note:bug resource/aws_ram_principal_association: Prevent creation of duplicate Terraform resources +``` + +```release-note:bug +resource/aws_ram_resource_association: Prevent creation of duplicate Terraform resources ``` \ No newline at end of file diff --git a/internal/service/ram/resource_association.go b/internal/service/ram/resource_association.go index 0254918d3446..7fc820efe505 100644 --- a/internal/service/ram/resource_association.go +++ b/internal/service/ram/resource_association.go @@ -6,6 +6,7 @@ package ram import ( "context" "errors" + "fmt" "log" "time" @@ -62,13 +63,24 @@ func resourceResourceAssociationCreate(ctx context.Context, d *schema.ResourceDa resourceShareARN, resourceARN := d.Get("resource_share_arn").(string), d.Get("resource_arn").(string) id := errs.Must(flex.FlattenResourceId([]string{resourceShareARN, resourceARN}, resourceAssociationResourceIDPartCount, false)) + _, err := findResourceAssociationByTwoPartKey(ctx, conn, resourceShareARN, resourceARN) + + switch { + case err == nil: + return sdkdiag.AppendFromErr(diags, fmt.Errorf("RAM Resource Association (%s) already exists", id)) + case tfresource.NotFound(err): + break + default: + return sdkdiag.AppendErrorf(diags, "reading RAM Resource Association: %s", err) + } + input := &ram.AssociateResourceShareInput{ ClientToken: aws.String(sdkid.UniqueId()), ResourceArns: aws.StringSlice([]string{resourceARN}), ResourceShareArn: aws.String(resourceShareARN), } - _, err := conn.AssociateResourceShareWithContext(ctx, input) + _, err = conn.AssociateResourceShareWithContext(ctx, input) if err != nil { return sdkdiag.AppendErrorf(diags, "creating RAM Resource Association (%s): %s", id, err) diff --git a/internal/service/ram/resource_association_test.go b/internal/service/ram/resource_association_test.go index 3165a106476f..74d7ff19bc00 100644 --- a/internal/service/ram/resource_association_test.go +++ b/internal/service/ram/resource_association_test.go @@ -8,6 +8,7 @@ import ( "fmt" "testing" + "github.com/YakDriver/regexache" "github.com/aws/aws-sdk-go/service/ram" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -72,8 +73,6 @@ func TestAccRAMResourceAssociation_disappears(t *testing.T) { func TestAccRAMResourceAssociation_duplicate(t *testing.T) { ctx := acctest.Context(t) - var resourceShareAssociation ram.ResourceShareAssociation - resourceName := "aws_ram_resource_association.test1" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ @@ -83,10 +82,8 @@ func TestAccRAMResourceAssociation_duplicate(t *testing.T) { CheckDestroy: testAccCheckResourceAssociationDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccResourceAssociationConfig_duplicate(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckResourceAssociationExists(ctx, resourceName, &resourceShareAssociation), - ), + Config: testAccResourceAssociationConfig_duplicate(rName), + ExpectError: regexache.MustCompile(`RAM Resource Association .* already exists`), }, }, }) @@ -165,7 +162,7 @@ resource "aws_ram_resource_association" "test1" { resource "aws_ram_resource_association" "test2" { resource_arn = aws_subnet.test[0].arn - resource_share_arn = aws_ram_resource_share.test.id + resource_share_arn = aws_ram_resource_association.test1.resource_share_arn } `, rName)) } From 56129a9cfbabde7dc696930182a1a3bb172d3380 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 14 Mar 2024 12:41:05 -0400 Subject: [PATCH 30/30] Fix semgrep 'ci.aws-in-func-name'. --- internal/types/aws_account_id.go | 2 +- internal/types/aws_account_id_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/types/aws_account_id.go b/internal/types/aws_account_id.go index 6569333e93a5..e7ab9ce8dca8 100644 --- a/internal/types/aws_account_id.go +++ b/internal/types/aws_account_id.go @@ -8,7 +8,7 @@ import ( ) // IsAWSAccountID returns whether or not the specified string is a valid AWS account ID. -func IsAWSAccountID(s string) bool { +func IsAWSAccountID(s string) bool { // nosemgrep:ci.aws-in-func-name // https://docs.aws.amazon.com/accounts/latest/reference/manage-acct-identifiers.html. return regexache.MustCompile(`^\d{12}$`).MatchString(s) } diff --git a/internal/types/aws_account_id_test.go b/internal/types/aws_account_id_test.go index 491d8e614a4b..cae322fa4358 100644 --- a/internal/types/aws_account_id_test.go +++ b/internal/types/aws_account_id_test.go @@ -5,7 +5,7 @@ package types import "testing" -func TestIsAWSAccountID(t *testing.T) { +func TestIsAWSAccountID(t *testing.T) { // nosemgrep:ci.aws-in-func-name t.Parallel() for _, tc := range []struct {