From 559e521f49aac73512292df668f83b66e119ad91 Mon Sep 17 00:00:00 2001 From: shuheiktgw Date: Sun, 17 Jan 2021 20:11:17 +0900 Subject: [PATCH] Fix aws_ram_resource_share_accepter eventual consistency problems --- aws/internal/service/ram/finder/finder.go | 186 ++++++++++++++++++ aws/internal/service/ram/waiter/status.go | 55 ++++++ aws/internal/service/ram/waiter/waiter.go | 80 ++++++++ aws/resource_aws_ram_resource_share.go | 42 +--- ...esource_aws_ram_resource_share_accepter.go | 97 ++------- 5 files changed, 338 insertions(+), 122 deletions(-) create mode 100644 aws/internal/service/ram/finder/finder.go create mode 100644 aws/internal/service/ram/waiter/status.go create mode 100644 aws/internal/service/ram/waiter/waiter.go diff --git a/aws/internal/service/ram/finder/finder.go b/aws/internal/service/ram/finder/finder.go new file mode 100644 index 00000000000..c26bc01e1ef --- /dev/null +++ b/aws/internal/service/ram/finder/finder.go @@ -0,0 +1,186 @@ +package finder + +import ( + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ram" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" +) + +// ResourceShareOwnerOtherAccountsByArn returns the resource share owned by other accounts corresponding to the specified ARN. +// Returns nil if no configuration is found. +func ResourceShareOwnerOtherAccountsByArn(conn *ram.RAM, arn string) (*ram.ResourceShare, error) { + listResourceSharesInput := &ram.GetResourceSharesInput{ + ResourceOwner: aws.String(ram.ResourceOwnerOtherAccounts), + ResourceShareArns: aws.StringSlice([]string{arn}), + } + + return resourceShare(conn, listResourceSharesInput) +} + +// ResourceShareOwnerSelfByArn returns the resource share owned by own account corresponding to the specified ARN. +// Returns nil if no configuration is found. +func ResourceShareOwnerSelfByArn(conn *ram.RAM, arn string) (*ram.ResourceShare, error) { + listResourceSharesInput := &ram.GetResourceSharesInput{ + ResourceOwner: aws.String(ram.ResourceOwnerSelf), + ResourceShareArns: aws.StringSlice([]string{arn}), + } + + return resourceShare(conn, listResourceSharesInput) +} + +// ResourceShareInvitationByResourceShareArnAndStatus returns the resource share invitation corresponding to the specified resource share ARN. +// Returns nil if no configuration is found. +func ResourceShareInvitationByResourceShareArnAndStatus(conn *ram.RAM, resourceShareArn, status string) (*ram.ResourceShareInvitation, error) { + var invitation *ram.ResourceShareInvitation + + // Retry for Ram resource share invitation eventual consistency + err := resource.Retry(30*time.Second, func() *resource.RetryError { + i, err := resourceShareInvitationByResourceShareArnAndStatus(conn, resourceShareArn, status) + invitation = i + + if err != nil { + return resource.NonRetryableError(err) + } + + if invitation == nil { + return resource.RetryableError(&resource.NotFoundError{}) + } + + return nil + }) + + if tfresource.TimedOut(err) { + invitation, err = resourceShareInvitationByResourceShareArnAndStatus(conn, resourceShareArn, status) + } + + if invitation == nil { + return nil, nil + } + + if err != nil { + return nil, err + } + + return invitation, nil +} + +// ResourceShareInvitationByArn returns the resource share invitation corresponding to the specified ARN. +// Returns nil if no configuration is found. +func ResourceShareInvitationByArn(conn *ram.RAM, arn string) (*ram.ResourceShareInvitation, error) { + var invitation *ram.ResourceShareInvitation + + // Retry for Ram resource share invitation eventual consistency + err := resource.Retry(30*time.Second, func() *resource.RetryError { + i, err := resourceShareInvitationByArn(conn, arn) + invitation = i + + if err != nil { + return resource.NonRetryableError(err) + } + + if invitation == nil { + resource.RetryableError(&resource.NotFoundError{}) + } + + return nil + }) + + if tfresource.TimedOut(err) { + invitation, err = resourceShareInvitationByArn(conn, arn) + } + + if invitation == nil { + return nil, nil + } + + if err != nil { + return nil, err + } + + return invitation, nil +} + +func resourceShare(conn *ram.RAM, input *ram.GetResourceSharesInput) (*ram.ResourceShare, error) { + var shares *ram.GetResourceSharesOutput + + // Retry for Ram resource share eventual consistency + err := resource.Retry(30*time.Second, func() *resource.RetryError { + ss, err := conn.GetResourceShares(input) + shares = ss + + if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + if len(shares.ResourceShares) == 0 { + return resource.RetryableError(&resource.NotFoundError{}) + } + + return nil + }) + + if tfresource.TimedOut(err) { + shares, err = conn.GetResourceShares(input) + } + + if err != nil { + return nil, err + } + + if shares == nil || len(shares.ResourceShares) == 0 { + return nil, nil + } + + return shares.ResourceShares[0], nil +} + +func resourceShareInvitationByResourceShareArnAndStatus(conn *ram.RAM, resourceShareArn, status string) (*ram.ResourceShareInvitation, error) { + var invitation *ram.ResourceShareInvitation + + input := &ram.GetResourceShareInvitationsInput{ + ResourceShareArns: []*string{aws.String(resourceShareArn)}, + } + + err := conn.GetResourceShareInvitationsPages(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(conn *ram.RAM, arn string) (*ram.ResourceShareInvitation, error) { + input := &ram.GetResourceShareInvitationsInput{ + ResourceShareInvitationArns: []*string{aws.String(arn)}, + } + + output, err := conn.GetResourceShareInvitations(input) + + if err != nil { + return nil, err + } + + if len(output.ResourceShareInvitations) == 0 { + return nil, nil + } + + return output.ResourceShareInvitations[0], nil +} diff --git a/aws/internal/service/ram/waiter/status.go b/aws/internal/service/ram/waiter/status.go new file mode 100644 index 00000000000..529039e25ac --- /dev/null +++ b/aws/internal/service/ram/waiter/status.go @@ -0,0 +1,55 @@ +package waiter + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ram" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ram/finder" +) + +const ( + resourceShareInvitationStatusNotFound = "NotFound" + resourceShareInvitationStatusUnknown = "Unknown" + + resourceShareStatusNotFound = "NotFound" + resourceShareStatusUnknown = "Unknown" +) + +// ResourceShareInvitationStatus fetches the ResourceShareInvitation and its Status +func ResourceShareInvitationStatus(conn *ram.RAM, arn string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + invitation, err := finder.ResourceShareInvitationByArn(conn, arn) + + if err != nil { + return nil, resourceShareInvitationStatusUnknown, err + } + + if invitation == nil { + return nil, resourceShareInvitationStatusNotFound, nil + } + + return invitation, aws.StringValue(invitation.Status), nil + } +} + +// ResourceShareOwnerSelfStatus fetches the ResourceShare and its Status +func ResourceShareOwnerSelfStatus(conn *ram.RAM, arn string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + share, err := finder.ResourceShareOwnerSelfByArn(conn, arn) + + if err != nil { + if tfawserr.ErrCodeEquals(err, ram.ErrCodeUnknownResourceException) { + return nil, resourceShareStatusNotFound, nil + } + + return nil, resourceShareStatusUnknown, err + } + + if share == nil { + return nil, resourceShareStatusNotFound, nil + } + + return share, aws.StringValue(share.Status), nil + } +} diff --git a/aws/internal/service/ram/waiter/waiter.go b/aws/internal/service/ram/waiter/waiter.go new file mode 100644 index 00000000000..ff659fe84ee --- /dev/null +++ b/aws/internal/service/ram/waiter/waiter.go @@ -0,0 +1,80 @@ +package waiter + +import ( + "time" + + "github.com/aws/aws-sdk-go/service/ram" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +// ResourceShareInvitationAccepted waits for a ResourceShareInvitation to return ACCEPTED +func ResourceShareInvitationAccepted(conn *ram.RAM, arn string, timeout time.Duration) (*ram.ResourceShareInvitation, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ram.ResourceShareInvitationStatusPending}, + Target: []string{ram.ResourceShareInvitationStatusAccepted}, + Refresh: ResourceShareInvitationStatus(conn, arn), + Timeout: timeout, + } + + outputRaw, err := stateConf.WaitForState() + + if v, ok := outputRaw.(*ram.ResourceShareInvitation); ok { + return v, err + } + + return nil, err +} + +// ResourceShareOwnedBySelfDisassociated waits for a ResourceShare owned by own account to be disassociated +func ResourceShareOwnedBySelfDisassociated(conn *ram.RAM, arn string, timeout time.Duration) (*ram.ResourceShare, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ram.ResourceShareAssociationStatusAssociated}, + Target: []string{}, + Refresh: ResourceShareOwnerSelfStatus(conn, arn), + Timeout: timeout, + } + + outputRaw, err := stateConf.WaitForState() + + if v, ok := outputRaw.(*ram.ResourceShare); ok { + return v, err + } + + return nil, err +} + +// ResourceShareOwnedBySelfActive waits for a ResourceShare owned by own account to return ACTIVE +func ResourceShareOwnedBySelfActive(conn *ram.RAM, arn string, timeout time.Duration) (*ram.ResourceShare, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ram.ResourceShareStatusPending}, + Target: []string{ram.ResourceShareStatusActive}, + Refresh: ResourceShareOwnerSelfStatus(conn, arn), + Timeout: timeout, + } + + outputRaw, err := stateConf.WaitForState() + + if v, ok := outputRaw.(*ram.ResourceShare); ok { + return v, err + } + + return nil, err +} + +// ResourceShareOwnedBySelfDeleted waits for a ResourceShare owned by own account to return DELETED +func ResourceShareOwnedBySelfDeleted(conn *ram.RAM, arn string, timeout time.Duration) (*ram.ResourceShare, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ram.ResourceShareStatusDeleting}, + Target: []string{ram.ResourceShareStatusDeleted}, + Refresh: ResourceShareOwnerSelfStatus(conn, arn), + Timeout: timeout, + } + + outputRaw, err := stateConf.WaitForState() + + if v, ok := outputRaw.(*ram.ResourceShare); ok { + return v, err + } + + return nil, err +} diff --git a/aws/resource_aws_ram_resource_share.go b/aws/resource_aws_ram_resource_share.go index 521182d89e6..4a31dcb5951 100644 --- a/aws/resource_aws_ram_resource_share.go +++ b/aws/resource_aws_ram_resource_share.go @@ -7,9 +7,9 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ram" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ram/waiter" ) func resourceAwsRamResourceShare() *schema.Resource { @@ -75,14 +75,7 @@ func resourceAwsRamResourceShareCreate(d *schema.ResourceData, meta interface{}) d.SetId(aws.StringValue(createResp.ResourceShare.ResourceShareArn)) - stateConf := &resource.StateChangeConf{ - Pending: []string{ram.ResourceShareStatusPending}, - Target: []string{ram.ResourceShareStatusActive}, - Refresh: resourceAwsRamResourceShareStateRefreshFunc(conn, d.Id()), - Timeout: d.Timeout(schema.TimeoutCreate), - } - - _, err = stateConf.WaitForState() + _, err = waiter.ResourceShareOwnedBySelfActive(conn, d.Id(), d.Timeout(schema.TimeoutCreate)) if err != nil { return fmt.Errorf("Error waiting for RAM resource share (%s) to become ready: %s", d.Id(), err) } @@ -191,40 +184,11 @@ func resourceAwsRamResourceShareDelete(d *schema.ResourceData, meta interface{}) return fmt.Errorf("Error deleting RAM resource share %s: %s", d.Id(), err) } - stateConf := &resource.StateChangeConf{ - Pending: []string{ram.ResourceShareStatusDeleting}, - Target: []string{ram.ResourceShareStatusDeleted}, - Refresh: resourceAwsRamResourceShareStateRefreshFunc(conn, d.Id()), - Timeout: d.Timeout(schema.TimeoutDelete), - } + _, err = waiter.ResourceShareOwnedBySelfDeleted(conn, d.Id(), d.Timeout(schema.TimeoutDelete)) - _, err = stateConf.WaitForState() if err != nil { return fmt.Errorf("Error waiting for RAM resource share (%s) to become ready: %s", d.Id(), err) } return nil } - -func resourceAwsRamResourceShareStateRefreshFunc(conn *ram.RAM, resourceShareArn string) resource.StateRefreshFunc { - return func() (interface{}, string, error) { - request := &ram.GetResourceSharesInput{ - ResourceShareArns: []*string{aws.String(resourceShareArn)}, - ResourceOwner: aws.String(ram.ResourceOwnerSelf), - } - - output, err := conn.GetResourceShares(request) - - if err != nil { - return nil, ram.ResourceShareStatusFailed, err - } - - if len(output.ResourceShares) == 0 { - return nil, ram.ResourceShareStatusDeleted, nil - } - - resourceShare := output.ResourceShares[0] - - return resourceShare, aws.StringValue(resourceShare.Status), nil - } -} diff --git a/aws/resource_aws_ram_resource_share_accepter.go b/aws/resource_aws_ram_resource_share_accepter.go index 2dbf4d4abd7..4e0e60f819e 100644 --- a/aws/resource_aws_ram_resource_share_accepter.go +++ b/aws/resource_aws_ram_resource_share_accepter.go @@ -10,6 +10,8 @@ import ( "github.com/aws/aws-sdk-go/service/ram" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ram/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ram/waiter" ) func resourceAwsRamResourceShareAccepter() *schema.Resource { @@ -81,7 +83,7 @@ func resourceAwsRamResourceShareAccepterCreate(d *schema.ResourceData, meta inte shareARN := d.Get("share_arn").(string) - invitation, err := resourceAwsRamResourceShareGetInvitation(conn, shareARN, ram.ResourceShareInvitationStatusPending) + invitation, err := finder.ResourceShareInvitationByResourceShareArnAndStatus(conn, shareARN, ram.ResourceShareInvitationStatusPending) if err != nil { return err @@ -108,16 +110,11 @@ func resourceAwsRamResourceShareAccepterCreate(d *schema.ResourceData, meta inte d.SetId(shareARN) - stateConf := &resource.StateChangeConf{ - Pending: []string{ram.ResourceShareInvitationStatusPending}, - Target: []string{ram.ResourceShareInvitationStatusAccepted}, - Refresh: resourceAwsRamResourceShareAccepterStateRefreshFunc( - conn, - aws.StringValue(output.ResourceShareInvitation.ResourceShareInvitationArn)), - Timeout: d.Timeout(schema.TimeoutCreate), - } - - _, err = stateConf.WaitForState() + _, err = waiter.ResourceShareInvitationAccepted( + conn, + aws.StringValue(output.ResourceShareInvitation.ResourceShareInvitationArn), + d.Timeout(schema.TimeoutCreate), + ) if err != nil { return fmt.Errorf("Error waiting for RAM resource share (%s) state: %s", d.Id(), err) @@ -130,7 +127,7 @@ func resourceAwsRamResourceShareAccepterRead(d *schema.ResourceData, meta interf accountID := meta.(*AWSClient).accountid conn := meta.(*AWSClient).ramconn - invitation, err := resourceAwsRamResourceShareGetInvitation(conn, d.Id(), ram.ResourceShareInvitationStatusAccepted) + invitation, err := finder.ResourceShareInvitationByResourceShareArnAndStatus(conn, d.Id(), ram.ResourceShareInvitationStatusAccepted) if err != nil { return fmt.Errorf("Error retrieving invitation for resource share %s: %s", d.Id(), err) @@ -143,24 +140,18 @@ func resourceAwsRamResourceShareAccepterRead(d *schema.ResourceData, meta interf d.Set("receiver_account_id", accountID) } - listResourceSharesInput := &ram.GetResourceSharesInput{ - ResourceOwner: aws.String(ram.ResourceOwnerOtherAccounts), - ResourceShareArns: aws.StringSlice([]string{d.Id()}), - } + resourceShare, err := finder.ResourceShareOwnerOtherAccountsByArn(conn, d.Id()) - shares, err := conn.GetResourceShares(listResourceSharesInput) if err != nil { - return fmt.Errorf("error retrieving resource shares: %w", err) + return fmt.Errorf("error retrieving resource share: %w", err) } - if len(shares.ResourceShares) != 1 { + if resourceShare == nil { log.Printf("[WARN] No RAM resource share with ARN (%s) found, removing from state", d.Id()) d.SetId("") return nil } - resourceShare := shares.ResourceShares[0] - d.Set("status", resourceShare.Status) d.Set("sender_account_id", resourceShare.OwningAccountId) d.Set("share_arn", resourceShare.ResourceShareArn) @@ -215,74 +206,14 @@ func resourceAwsRamResourceShareAccepterDelete(d *schema.ResourceData, meta inte return fmt.Errorf("Error leaving RAM resource share: %s", err) } - stateConf := &resource.StateChangeConf{ - Pending: []string{ram.ResourceShareAssociationStatusAssociated}, - Target: []string{ram.ResourceShareAssociationStatusDisassociated}, - Refresh: resourceAwsRamResourceShareStateRefreshFunc(conn, d.Id()), - Timeout: d.Timeout(schema.TimeoutDelete), - } - - if _, err := stateConf.WaitForState(); err != nil { - if isAWSErr(err, ram.ErrCodeUnknownResourceException, "") { - // what we want - return nil - } + _, err = waiter.ResourceShareOwnedBySelfDisassociated(conn, d.Id(), d.Timeout(schema.TimeoutDelete)) + if err != nil { return fmt.Errorf("Error waiting for RAM resource share (%s) state: %s", d.Id(), err) } return nil } -func resourceAwsRamResourceShareGetInvitation(conn *ram.RAM, resourceShareARN, status string) (*ram.ResourceShareInvitation, error) { - input := &ram.GetResourceShareInvitationsInput{ - ResourceShareArns: []*string{aws.String(resourceShareARN)}, - } - - var invitation *ram.ResourceShareInvitation - err := conn.GetResourceShareInvitationsPages(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 invitation == nil { - return nil, nil - } - - if err != nil { - return nil, fmt.Errorf("Error reading RAM resource share invitation %s: %s", resourceShareARN, err) - } - - return invitation, nil -} - -func resourceAwsRamResourceShareAccepterStateRefreshFunc(conn *ram.RAM, invitationArn string) resource.StateRefreshFunc { - return func() (interface{}, string, error) { - request := &ram.GetResourceShareInvitationsInput{ - ResourceShareInvitationArns: []*string{aws.String(invitationArn)}, - } - - output, err := conn.GetResourceShareInvitations(request) - - if err != nil { - return nil, "Unable to get resource share invitations", err - } - - if len(output.ResourceShareInvitations) == 0 { - return nil, "Resource share invitation not found", nil - } - - invitation := output.ResourceShareInvitations[0] - - return invitation, aws.StringValue(invitation.Status), nil - } -} - func resourceAwsRamResourceShareGetIDFromARN(arn string) string { return strings.Replace(arn[strings.LastIndex(arn, ":")+1:], "resource-share/", "rs-", -1) }