From 723aae3a97c6c436441e138e495f4ba83202b39b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 23 Dec 2022 13:45:25 -0500 Subject: [PATCH] r/aws_s3control_multi_region_access_point & r/aws_s3control_multi_region_access_point_policy: Switch to 'WithoutTimeout' CRUD handlers (#15090). Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ControlMultiRegionAccessPoint' PKG=s3control ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3control/... -v -count 1 -parallel 3 -run=TestAccS3ControlMultiRegionAccessPoint -timeout 180m === RUN TestAccS3ControlMultiRegionAccessPointDataSource_basic === PAUSE TestAccS3ControlMultiRegionAccessPointDataSource_basic === RUN TestAccS3ControlMultiRegionAccessPointPolicy_basic === PAUSE TestAccS3ControlMultiRegionAccessPointPolicy_basic === RUN TestAccS3ControlMultiRegionAccessPointPolicy_disappears_MultiRegionAccessPoint === PAUSE TestAccS3ControlMultiRegionAccessPointPolicy_disappears_MultiRegionAccessPoint === RUN TestAccS3ControlMultiRegionAccessPointPolicy_details_policy === PAUSE TestAccS3ControlMultiRegionAccessPointPolicy_details_policy === RUN TestAccS3ControlMultiRegionAccessPointPolicy_details_name === PAUSE TestAccS3ControlMultiRegionAccessPointPolicy_details_name === RUN TestAccS3ControlMultiRegionAccessPoint_basic === PAUSE TestAccS3ControlMultiRegionAccessPoint_basic === RUN TestAccS3ControlMultiRegionAccessPoint_disappears === PAUSE TestAccS3ControlMultiRegionAccessPoint_disappears === RUN TestAccS3ControlMultiRegionAccessPoint_PublicAccessBlock === PAUSE TestAccS3ControlMultiRegionAccessPoint_PublicAccessBlock === RUN TestAccS3ControlMultiRegionAccessPoint_name === PAUSE TestAccS3ControlMultiRegionAccessPoint_name === RUN TestAccS3ControlMultiRegionAccessPoint_threeRegions === PAUSE TestAccS3ControlMultiRegionAccessPoint_threeRegions === CONT TestAccS3ControlMultiRegionAccessPointDataSource_basic === CONT TestAccS3ControlMultiRegionAccessPoint_basic === CONT TestAccS3ControlMultiRegionAccessPoint_threeRegions --- PASS: TestAccS3ControlMultiRegionAccessPoint_basic (336.67s) === CONT TestAccS3ControlMultiRegionAccessPoint_name --- PASS: TestAccS3ControlMultiRegionAccessPoint_threeRegions (348.44s) === CONT TestAccS3ControlMultiRegionAccessPoint_PublicAccessBlock --- PASS: TestAccS3ControlMultiRegionAccessPointDataSource_basic (359.54s) === CONT TestAccS3ControlMultiRegionAccessPoint_disappears --- PASS: TestAccS3ControlMultiRegionAccessPoint_disappears (256.99s) === CONT TestAccS3ControlMultiRegionAccessPointPolicy_details_policy --- PASS: TestAccS3ControlMultiRegionAccessPoint_PublicAccessBlock (333.59s) === CONT TestAccS3ControlMultiRegionAccessPointPolicy_details_name --- PASS: TestAccS3ControlMultiRegionAccessPointPolicy_details_policy (265.17s) === CONT TestAccS3ControlMultiRegionAccessPointPolicy_disappears_MultiRegionAccessPoint --- PASS: TestAccS3ControlMultiRegionAccessPoint_name (587.22s) === CONT TestAccS3ControlMultiRegionAccessPointPolicy_basic --- PASS: TestAccS3ControlMultiRegionAccessPointPolicy_disappears_MultiRegionAccessPoint (262.98s) --- PASS: TestAccS3ControlMultiRegionAccessPointPolicy_details_name (526.15s) --- PASS: TestAccS3ControlMultiRegionAccessPointPolicy_basic (286.04s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3control 1215.420s --- internal/provider/provider.go | 4 - internal/service/s3control/exports_test.go | 2 + internal/service/s3control/find.go | 78 --------- .../s3control/multi_region_access_point.go | 163 +++++++++++++++--- .../multi_region_access_point_data_source.go | 8 +- ...ti_region_access_point_data_source_test.go | 11 +- .../multi_region_access_point_policy.go | 89 ++++++---- .../multi_region_access_point_policy_test.go | 28 +-- .../multi_region_access_point_test.go | 40 ++--- internal/service/s3control/status.go | 24 --- internal/service/s3control/sweep.go | 2 +- internal/service/s3control/wait.go | 48 ------ 12 files changed, 232 insertions(+), 265 deletions(-) delete mode 100644 internal/service/s3control/status.go delete mode 100644 internal/service/s3control/wait.go diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 3c46613da124..5c88d4c0da3c 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -870,8 +870,6 @@ func New(ctx context.Context) (*schema.Provider, error) { "aws_s3_bucket_objects": s3.DataSourceBucketObjects(), // DEPRECATED: use aws_s3_objects instead "aws_s3_bucket_policy": s3.DataSourceBucketPolicy(), - "aws_s3control_multi_region_access_point": s3control.DataSourceMultiRegionAccessPoint(), - "aws_sagemaker_prebuilt_ecr_image": sagemaker.DataSourcePrebuiltECRImage(), "aws_secretsmanager_random_password": secretsmanager.DataSourceRandomPassword(), @@ -2005,8 +2003,6 @@ func New(ctx context.Context) (*schema.Provider, error) { "aws_s3_object_copy": s3.ResourceObjectCopy(), "aws_s3_bucket_object": s3.ResourceBucketObject(), // DEPRECATED: use aws_s3_object instead - "aws_s3control_multi_region_access_point": s3control.ResourceMultiRegionAccessPoint(), - "aws_s3control_multi_region_access_point_policy": s3control.ResourceMultiRegionAccessPointPolicy(), "aws_s3control_object_lambda_access_point": s3control.ResourceObjectLambdaAccessPoint(), "aws_s3control_object_lambda_access_point_policy": s3control.ResourceObjectLambdaAccessPointPolicy(), diff --git a/internal/service/s3control/exports_test.go b/internal/service/s3control/exports_test.go index 28a05f0e1ee8..6a8ee5fbe942 100644 --- a/internal/service/s3control/exports_test.go +++ b/internal/service/s3control/exports_test.go @@ -8,5 +8,7 @@ var ( ResourceBucket = resourceBucket ResourceBucketLifecycleConfiguration = resourceBucketLifecycleConfiguration ResourceBucketPolicy = resourceBucketPolicy + ResourceMultiRegionAccessPoint = resourceMultiRegionAccessPoint + ResourceMultiRegionAccessPointPolicy = resourceMultiRegionAccessPointPolicy ResourceStorageLensConfiguration = resourceStorageLensConfiguration ) diff --git a/internal/service/s3control/find.go b/internal/service/s3control/find.go index fa760728ae94..61bdd52f9781 100644 --- a/internal/service/s3control/find.go +++ b/internal/service/s3control/find.go @@ -8,84 +8,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -func FindMultiRegionAccessPointByAccountIDAndName(conn *s3control.S3Control, accountID string, name string) (*s3control.MultiRegionAccessPointReport, error) { - input := &s3control.GetMultiRegionAccessPointInput{ - AccountId: aws.String(accountID), - Name: aws.String(name), - } - - output, err := conn.GetMultiRegionAccessPoint(input) - - if tfawserr.ErrCodeEquals(err, errCodeNoSuchMultiRegionAccessPoint) { - return nil, &resource.NotFoundError{ - LastError: err, - LastRequest: input, - } - } - - if err != nil { - return nil, err - } - - if output == nil || output.AccessPoint == nil { - return nil, tfresource.NewEmptyResultError(input) - } - - return output.AccessPoint, nil -} - -func findMultiRegionAccessPointOperationByAccountIDAndTokenARN(conn *s3control.S3Control, accountID string, requestTokenARN string) (*s3control.AsyncOperation, error) { - input := &s3control.DescribeMultiRegionAccessPointOperationInput{ - AccountId: aws.String(accountID), - RequestTokenARN: aws.String(requestTokenARN), - } - - output, err := conn.DescribeMultiRegionAccessPointOperation(input) - - if tfawserr.ErrCodeEquals(err, errCodeNoSuchAsyncRequest) { - return nil, &resource.NotFoundError{ - LastError: err, - LastRequest: input, - } - } - - if err != nil { - return nil, err - } - - if output == nil || output.AsyncOperation == nil { - return nil, tfresource.NewEmptyResultError(input) - } - - return output.AsyncOperation, nil -} - -func FindMultiRegionAccessPointPolicyDocumentByAccountIDAndName(conn *s3control.S3Control, accountID string, name string) (*s3control.MultiRegionAccessPointPolicyDocument, error) { - input := &s3control.GetMultiRegionAccessPointPolicyInput{ - AccountId: aws.String(accountID), - Name: aws.String(name), - } - - output, err := conn.GetMultiRegionAccessPointPolicy(input) - - if tfawserr.ErrCodeEquals(err, errCodeNoSuchMultiRegionAccessPoint) { - return nil, &resource.NotFoundError{ - LastError: err, - LastRequest: input, - } - } - - if err != nil { - return nil, err - } - - if output == nil || output.Policy == nil { - return nil, tfresource.NewEmptyResultError(input) - } - - return output.Policy, nil -} - func FindObjectLambdaAccessPointByAccountIDAndName(conn *s3control.S3Control, accountID string, name string) (*s3control.ObjectLambdaConfiguration, error) { input := &s3control.GetAccessPointConfigurationForObjectLambdaInput{ AccountId: aws.String(accountID), diff --git a/internal/service/s3control/multi_region_access_point.go b/internal/service/s3control/multi_region_access_point.go index c090fb83dec4..8c37944ce651 100644 --- a/internal/service/s3control/multi_region_access_point.go +++ b/internal/service/s3control/multi_region_access_point.go @@ -1,6 +1,7 @@ package s3control import ( + "context" "fmt" "log" "strings" @@ -11,6 +12,8 @@ import ( "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/s3control" "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/resource" "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" @@ -18,11 +21,15 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -func ResourceMultiRegionAccessPoint() *schema.Resource { +func init() { + _sp.registerSDKResourceFactory("aws_s3control_multi_region_access_point", resourceMultiRegionAccessPoint) +} + +func resourceMultiRegionAccessPoint() *schema.Resource { return &schema.Resource{ - Create: resourceMultiRegionAccessPointCreate, - Read: resourceMultiRegionAccessPointRead, - Delete: resourceMultiRegionAccessPointDelete, + CreateWithoutTimeout: resourceMultiRegionAccessPointCreate, + ReadWithoutTimeout: resourceMultiRegionAccessPointRead, + DeleteWithoutTimeout: resourceMultiRegionAccessPointDelete, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, @@ -131,11 +138,11 @@ func ResourceMultiRegionAccessPoint() *schema.Resource { } } -func resourceMultiRegionAccessPointCreate(d *schema.ResourceData, meta interface{}) error { +func resourceMultiRegionAccessPointCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn, err := ConnForMRAP(meta.(*conns.AWSClient)) if err != nil { - return err + return diag.FromErr(err) } accountID := meta.(*conns.AWSClient).AccountID @@ -153,38 +160,37 @@ func resourceMultiRegionAccessPointCreate(d *schema.ResourceData, meta interface resourceID := MultiRegionAccessPointCreateResourceID(accountID, aws.StringValue(input.Details.Name)) - log.Printf("[DEBUG] Creating S3 Multi-Region Access Point: %s", input) - output, err := conn.CreateMultiRegionAccessPoint(input) + output, err := conn.CreateMultiRegionAccessPointWithContext(ctx, input) if err != nil { - return fmt.Errorf("error creating S3 Multi-Region Access Point (%s): %w", resourceID, err) + return diag.Errorf("creating S3 Multi-Region Access Point (%s): %s", resourceID, err) } d.SetId(resourceID) - _, err = waitMultiRegionAccessPointRequestSucceeded(conn, accountID, aws.StringValue(output.RequestTokenARN), d.Timeout(schema.TimeoutCreate)) + _, err = waitMultiRegionAccessPointRequestSucceeded(ctx, conn, accountID, aws.StringValue(output.RequestTokenARN), d.Timeout(schema.TimeoutCreate)) if err != nil { - return fmt.Errorf("error waiting for Multi-Region Access Point (%s) create: %s", d.Id(), err) + return diag.Errorf("waiting for Multi-Region Access Point (%s) create: %s", d.Id(), err) } - return resourceMultiRegionAccessPointRead(d, meta) + return resourceMultiRegionAccessPointRead(ctx, d, meta) } -func resourceMultiRegionAccessPointRead(d *schema.ResourceData, meta interface{}) error { +func resourceMultiRegionAccessPointRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn, err := ConnForMRAP(meta.(*conns.AWSClient)) if err != nil { - return err + return diag.FromErr(err) } accountID, name, err := MultiRegionAccessPointParseResourceID(d.Id()) if err != nil { - return err + return diag.FromErr(err) } - accessPoint, err := FindMultiRegionAccessPointByAccountIDAndName(conn, accountID, name) + accessPoint, err := FindMultiRegionAccessPointByTwoPartKey(ctx, conn, accountID, name) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] S3 Multi-Region Access Point (%s) not found, removing from state", d.Id()) @@ -193,7 +199,7 @@ func resourceMultiRegionAccessPointRead(d *schema.ResourceData, meta interface{} } if err != nil { - return fmt.Errorf("error reading S3 Multi-Region Access Point (%s): %w", d.Id(), err) + return diag.Errorf("reading S3 Multi-Region Access Point (%s): %s", d.Id(), err) } alias := aws.StringValue(accessPoint.Alias) @@ -207,7 +213,7 @@ func resourceMultiRegionAccessPointRead(d *schema.ResourceData, meta interface{} d.Set("alias", alias) d.Set("arn", arn) if err := d.Set("details", []interface{}{flattenMultiRegionAccessPointReport(accessPoint)}); err != nil { - return fmt.Errorf("error setting details: %w", err) + return diag.Errorf("setting details: %s", err) } // https://docs.aws.amazon.com/AmazonS3/latest/userguide//MultiRegionAccessPointRequests.html#MultiRegionAccessPointHostnames. d.Set("domain_name", meta.(*conns.AWSClient).PartitionHostname(fmt.Sprintf("%s.accesspoint.s3-global", alias))) @@ -216,21 +222,21 @@ func resourceMultiRegionAccessPointRead(d *schema.ResourceData, meta interface{} return nil } -func resourceMultiRegionAccessPointDelete(d *schema.ResourceData, meta interface{}) error { +func resourceMultiRegionAccessPointDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn, err := ConnForMRAP(meta.(*conns.AWSClient)) if err != nil { - return err + return diag.FromErr(err) } accountID, name, err := MultiRegionAccessPointParseResourceID(d.Id()) if err != nil { - return err + return diag.FromErr(err) } log.Printf("[DEBUG] Deleting S3 Multi-Region Access Point: %s", d.Id()) - output, err := conn.DeleteMultiRegionAccessPoint(&s3control.DeleteMultiRegionAccessPointInput{ + output, err := conn.DeleteMultiRegionAccessPointWithContext(ctx, &s3control.DeleteMultiRegionAccessPointInput{ AccountId: aws.String(accountID), Details: &s3control.DeleteMultiRegionAccessPointInput_{ Name: aws.String(name), @@ -242,13 +248,13 @@ func resourceMultiRegionAccessPointDelete(d *schema.ResourceData, meta interface } if err != nil { - return fmt.Errorf("error deleting S3 Multi-Region Access Point (%s): %w", d.Id(), err) + return diag.Errorf("deleting S3 Multi-Region Access Point (%s): %s", d.Id(), err) } - _, err = waitMultiRegionAccessPointRequestSucceeded(conn, accountID, aws.StringValue(output.RequestTokenARN), d.Timeout(schema.TimeoutDelete)) + _, err = waitMultiRegionAccessPointRequestSucceeded(ctx, conn, accountID, aws.StringValue(output.RequestTokenARN), d.Timeout(schema.TimeoutDelete)) if err != nil { - return fmt.Errorf("error waiting for S3 Multi-Region Access Point (%s) delete: %w", d.Id(), err) + return diag.Errorf("error waiting for S3 Multi-Region Access Point (%s) delete: %s", d.Id(), err) } return nil @@ -266,12 +272,117 @@ func ConnForMRAP(client *conns.AWSClient) (*s3control.S3Control, error) { sess, err := conns.NewSessionForRegion(&originalConn.Config, region, client.TerraformVersion) if err != nil { - return nil, fmt.Errorf("error creating AWS session: %w", err) + return nil, fmt.Errorf("creating AWS session: %w", err) } return s3control.New(sess), nil } +func FindMultiRegionAccessPointByTwoPartKey(ctx context.Context, conn *s3control.S3Control, accountID string, name string) (*s3control.MultiRegionAccessPointReport, error) { + input := &s3control.GetMultiRegionAccessPointInput{ + AccountId: aws.String(accountID), + Name: aws.String(name), + } + + output, err := conn.GetMultiRegionAccessPointWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, errCodeNoSuchMultiRegionAccessPoint) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil || output.AccessPoint == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.AccessPoint, nil +} + +func findMultiRegionAccessPointOperationByAccountIDAndTokenARN(ctx context.Context, conn *s3control.S3Control, accountID string, requestTokenARN string) (*s3control.AsyncOperation, error) { + input := &s3control.DescribeMultiRegionAccessPointOperationInput{ + AccountId: aws.String(accountID), + RequestTokenARN: aws.String(requestTokenARN), + } + + output, err := conn.DescribeMultiRegionAccessPointOperationWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, errCodeNoSuchAsyncRequest) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil || output.AsyncOperation == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.AsyncOperation, nil +} + +func statusMultiRegionAccessPointRequest(ctx context.Context, conn *s3control.S3Control, accountID string, requestTokenARN string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + output, err := findMultiRegionAccessPointOperationByAccountIDAndTokenARN(ctx, conn, accountID, requestTokenARN) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return output, aws.StringValue(output.RequestStatus), nil + } +} + +const ( + // Minimum amount of times to verify change propagation + propagationContinuousTargetOccurence = 2 + + // Minimum amount of time to wait between S3control change polls + propagationMinTimeout = 5 * time.Second + + // Maximum amount of time to wait for S3control changes to propagate + propagationTimeout = 1 * time.Minute + + multiRegionAccessPointRequestSucceededMinTimeout = 5 * time.Second + + multiRegionAccessPointRequestSucceededDelay = 15 * time.Second +) + +func waitMultiRegionAccessPointRequestSucceeded(ctx context.Context, conn *s3control.S3Control, accountID string, requestTokenArn string, timeout time.Duration) (*s3control.AsyncOperation, error) { //nolint:unparam + stateConf := &resource.StateChangeConf{ + Target: []string{RequestStatusSucceeded}, + Timeout: timeout, + Refresh: statusMultiRegionAccessPointRequest(ctx, conn, accountID, requestTokenArn), + MinTimeout: multiRegionAccessPointRequestSucceededMinTimeout, + Delay: multiRegionAccessPointRequestSucceededDelay, + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*s3control.AsyncOperation); ok { + if status, responseDetails := aws.StringValue(output.RequestStatus), output.ResponseDetails; status == RequestStatusFailed && responseDetails != nil && responseDetails.ErrorDetails != nil { + tfresource.SetLastError(err, fmt.Errorf("%s: %s", aws.StringValue(responseDetails.ErrorDetails.Code), aws.StringValue(responseDetails.ErrorDetails.Message))) + } + + return output, err + } + + return nil, err +} + const multiRegionAccessPointResourceIDSeparator = ":" func MultiRegionAccessPointCreateResourceID(accountID, accessPointName string) string { diff --git a/internal/service/s3control/multi_region_access_point_data_source.go b/internal/service/s3control/multi_region_access_point_data_source.go index 6aee766d8546..d7c3b8aac815 100644 --- a/internal/service/s3control/multi_region_access_point_data_source.go +++ b/internal/service/s3control/multi_region_access_point_data_source.go @@ -13,7 +13,11 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -func DataSourceMultiRegionAccessPoint() *schema.Resource { +func init() { + _sp.registerSDKDataSourceFactory("aws_s3control_multi_region_access_point", dataSourceMultiRegionAccessPoint) +} + +func dataSourceMultiRegionAccessPoint() *schema.Resource { return &schema.Resource{ ReadWithoutTimeout: dataSourceMultiRegionAccessPointBlockRead, @@ -105,7 +109,7 @@ func dataSourceMultiRegionAccessPointBlockRead(ctx context.Context, d *schema.Re } name := d.Get("name").(string) - accessPoint, err := FindMultiRegionAccessPointByAccountIDAndName(conn, accountID, name) + accessPoint, err := FindMultiRegionAccessPointByTwoPartKey(ctx, conn, accountID, name) if err != nil { return diag.Errorf("reading S3 Multi Region Access Point (%s): %s", name, err) diff --git a/internal/service/s3control/multi_region_access_point_data_source_test.go b/internal/service/s3control/multi_region_access_point_data_source_test.go index 2102d36e97bd..58171c2ff994 100644 --- a/internal/service/s3control/multi_region_access_point_data_source_test.go +++ b/internal/service/s3control/multi_region_access_point_data_source_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/s3control" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -18,12 +19,12 @@ func TestAccS3ControlMultiRegionAccessPointDataSource_basic(t *testing.T) { bucket2Name := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckMultipleRegion(t, 2) }, + PreCheck: func() { + acctest.PreCheck(t) + acctest.PreCheckMultipleRegion(t, 2) + acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) + }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(t, 2), Steps: []resource.TestStep{ diff --git a/internal/service/s3control/multi_region_access_point_policy.go b/internal/service/s3control/multi_region_access_point_policy.go index e60271e2b6ab..f93b90d67237 100644 --- a/internal/service/s3control/multi_region_access_point_policy.go +++ b/internal/service/s3control/multi_region_access_point_policy.go @@ -1,12 +1,15 @@ package s3control import ( - "fmt" + "context" "log" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3control" + "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/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -15,12 +18,16 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -func ResourceMultiRegionAccessPointPolicy() *schema.Resource { +func init() { + _sp.registerSDKResourceFactory("aws_s3control_multi_region_access_point_policy", resourceMultiRegionAccessPointPolicy) +} + +func resourceMultiRegionAccessPointPolicy() *schema.Resource { return &schema.Resource{ - Create: resourceMultiRegionAccessPointPolicyCreate, - Read: resourceMultiRegionAccessPointPolicyRead, - Update: resourceMultiRegionAccessPointPolicyUpdate, - Delete: schema.Noop, + CreateWithoutTimeout: resourceMultiRegionAccessPointPolicyCreate, + ReadWithoutTimeout: resourceMultiRegionAccessPointPolicyRead, + UpdateWithoutTimeout: resourceMultiRegionAccessPointPolicyUpdate, + DeleteWithoutTimeout: schema.NoopContext, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, @@ -77,11 +84,11 @@ func ResourceMultiRegionAccessPointPolicy() *schema.Resource { } } -func resourceMultiRegionAccessPointPolicyCreate(d *schema.ResourceData, meta interface{}) error { +func resourceMultiRegionAccessPointPolicyCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn, err := ConnForMRAP(meta.(*conns.AWSClient)) if err != nil { - return err + return diag.FromErr(err) } accountID := meta.(*conns.AWSClient).AccountID @@ -99,38 +106,37 @@ func resourceMultiRegionAccessPointPolicyCreate(d *schema.ResourceData, meta int resourceID := MultiRegionAccessPointCreateResourceID(accountID, aws.StringValue(input.Details.Name)) - log.Printf("[DEBUG] Creating S3 Multi-Region Access Point Policy: %s", input) - output, err := conn.PutMultiRegionAccessPointPolicy(input) + output, err := conn.PutMultiRegionAccessPointPolicyWithContext(ctx, input) if err != nil { - return fmt.Errorf("error creating S3 Multi-Region Access Point (%s) Policy: %w", resourceID, err) + return diag.Errorf("creating S3 Multi-Region Access Point (%s) Policy: %s", resourceID, err) } d.SetId(resourceID) - _, err = waitMultiRegionAccessPointRequestSucceeded(conn, accountID, aws.StringValue(output.RequestTokenARN), d.Timeout(schema.TimeoutCreate)) + _, err = waitMultiRegionAccessPointRequestSucceeded(ctx, conn, accountID, aws.StringValue(output.RequestTokenARN), d.Timeout(schema.TimeoutCreate)) if err != nil { - return fmt.Errorf("error waiting for S3 Multi-Region Access Point Policy (%s) create: %w", d.Id(), err) + return diag.Errorf("waiting for S3 Multi-Region Access Point Policy (%s) create: %s", d.Id(), err) } - return resourceMultiRegionAccessPointPolicyRead(d, meta) + return resourceMultiRegionAccessPointPolicyRead(ctx, d, meta) } -func resourceMultiRegionAccessPointPolicyRead(d *schema.ResourceData, meta interface{}) error { +func resourceMultiRegionAccessPointPolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn, err := ConnForMRAP(meta.(*conns.AWSClient)) if err != nil { - return err + return diag.FromErr(err) } accountID, name, err := MultiRegionAccessPointParseResourceID(d.Id()) if err != nil { - return err + return diag.FromErr(err) } - policyDocument, err := FindMultiRegionAccessPointPolicyDocumentByAccountIDAndName(conn, accountID, name) + policyDocument, err := FindMultiRegionAccessPointPolicyDocumentByTwoPartKey(ctx, conn, accountID, name) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] S3 Multi-Region Access Point Policy (%s) not found, removing from state", d.Id()) @@ -139,7 +145,7 @@ func resourceMultiRegionAccessPointPolicyRead(d *schema.ResourceData, meta inter } if err != nil { - return fmt.Errorf("error reading S3 Multi-Region Access Point Policy (%s): %w", d.Id(), err) + return diag.Errorf("reading S3 Multi-Region Access Point Policy (%s): %s", d.Id(), err) } d.Set("account_id", accountID) @@ -150,7 +156,7 @@ func resourceMultiRegionAccessPointPolicyRead(d *schema.ResourceData, meta inter } if err := d.Set("details", []interface{}{flattenMultiRegionAccessPointPolicyDocument(name, policyDocument, oldDetails)}); err != nil { - return fmt.Errorf("error setting details: %w", err) + return diag.Errorf("setting details: %s", err) } } else { d.Set("details", nil) @@ -169,17 +175,17 @@ func resourceMultiRegionAccessPointPolicyRead(d *schema.ResourceData, meta inter return nil } -func resourceMultiRegionAccessPointPolicyUpdate(d *schema.ResourceData, meta interface{}) error { +func resourceMultiRegionAccessPointPolicyUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn, err := ConnForMRAP(meta.(*conns.AWSClient)) if err != nil { - return err + return diag.FromErr(err) } accountID, _, err := MultiRegionAccessPointParseResourceID(d.Id()) if err != nil { - return err + return diag.FromErr(err) } input := &s3control.PutMultiRegionAccessPointPolicyInput{ @@ -190,20 +196,45 @@ func resourceMultiRegionAccessPointPolicyUpdate(d *schema.ResourceData, meta int input.Details = expandPutMultiRegionAccessPointPolicyInput_(v.([]interface{})[0].(map[string]interface{})) } - log.Printf("[DEBUG] Updating S3 Multi-Region Access Point Policy: %s", input) - output, err := conn.PutMultiRegionAccessPointPolicy(input) + output, err := conn.PutMultiRegionAccessPointPolicyWithContext(ctx, input) + + if err != nil { + return diag.Errorf("updating S3 Multi-Region Access Point Policy (%s): %s", d.Id(), err) + } + + _, err = waitMultiRegionAccessPointRequestSucceeded(ctx, conn, accountID, aws.StringValue(output.RequestTokenARN), d.Timeout(schema.TimeoutUpdate)) if err != nil { - return fmt.Errorf("error updating S3 Multi-Region Access Point Policy (%s): %w", d.Id(), err) + return diag.Errorf("waiting for S3 Multi-Region Access Point Policy (%s) update: %s", d.Id(), err) + } + + return resourceMultiRegionAccessPointPolicyRead(ctx, d, meta) +} + +func FindMultiRegionAccessPointPolicyDocumentByTwoPartKey(ctx context.Context, conn *s3control.S3Control, accountID string, name string) (*s3control.MultiRegionAccessPointPolicyDocument, error) { + input := &s3control.GetMultiRegionAccessPointPolicyInput{ + AccountId: aws.String(accountID), + Name: aws.String(name), } - _, err = waitMultiRegionAccessPointRequestSucceeded(conn, accountID, aws.StringValue(output.RequestTokenARN), d.Timeout(schema.TimeoutUpdate)) + output, err := conn.GetMultiRegionAccessPointPolicyWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, errCodeNoSuchMultiRegionAccessPoint) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } if err != nil { - return fmt.Errorf("error waiting for S3 Multi-Region Access Point Policy (%s) update: %w", d.Id(), err) + return nil, err + } + + if output == nil || output.Policy == nil { + return nil, tfresource.NewEmptyResultError(input) } - return resourceMultiRegionAccessPointPolicyRead(d, meta) + return output.Policy, nil } func expandPutMultiRegionAccessPointPolicyInput_(tfMap map[string]interface{}) *s3control.PutMultiRegionAccessPointPolicyInput_ { diff --git a/internal/service/s3control/multi_region_access_point_policy_test.go b/internal/service/s3control/multi_region_access_point_policy_test.go index 057a21a5dcad..609dcf0c608e 100644 --- a/internal/service/s3control/multi_region_access_point_policy_test.go +++ b/internal/service/s3control/multi_region_access_point_policy_test.go @@ -1,10 +1,12 @@ package s3control_test import ( + "context" "fmt" "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/s3control" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -20,12 +22,8 @@ func TestAccS3ControlMultiRegionAccessPointPolicy_basic(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) multiRegionAccessPointName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, // Multi-Region Access Point Policy cannot be deleted once applied. @@ -61,12 +59,8 @@ func TestAccS3ControlMultiRegionAccessPointPolicy_disappears_MultiRegionAccessPo bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, // Multi-Region Access Point Policy cannot be deleted once applied. @@ -91,12 +85,8 @@ func TestAccS3ControlMultiRegionAccessPointPolicy_details_policy(t *testing.T) { multiRegionAccessPointName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, // Multi-Region Access Point Policy cannot be deleted once applied. @@ -132,12 +122,8 @@ func TestAccS3ControlMultiRegionAccessPointPolicy_details_name(t *testing.T) { multiRegionAccessPointName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, // Multi-Region Access Point Policy cannot be deleted once applied. @@ -190,7 +176,7 @@ func testAccCheckMultiRegionAccessPointPolicyExists(n string, v *s3control.Multi return err } - output, err := tfs3control.FindMultiRegionAccessPointPolicyDocumentByAccountIDAndName(conn, accountID, name) + output, err := tfs3control.FindMultiRegionAccessPointPolicyDocumentByTwoPartKey(context.Background(), conn, accountID, name) if err != nil { return err diff --git a/internal/service/s3control/multi_region_access_point_test.go b/internal/service/s3control/multi_region_access_point_test.go index a11479659947..36b451872ee3 100644 --- a/internal/service/s3control/multi_region_access_point_test.go +++ b/internal/service/s3control/multi_region_access_point_test.go @@ -1,11 +1,13 @@ package s3control_test import ( + "context" "fmt" "regexp" "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/s3control" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -22,12 +24,8 @@ func TestAccS3ControlMultiRegionAccessPoint_basic(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckMultiRegionAccessPointDestroy, @@ -69,12 +67,8 @@ func TestAccS3ControlMultiRegionAccessPoint_disappears(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckMultiRegionAccessPointDestroy, @@ -97,12 +91,8 @@ func TestAccS3ControlMultiRegionAccessPoint_PublicAccessBlock(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckMultiRegionAccessPointDestroy, @@ -134,12 +124,8 @@ func TestAccS3ControlMultiRegionAccessPoint_name(t *testing.T) { rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckMultiRegionAccessPointDestroy, @@ -176,12 +162,12 @@ func TestAccS3ControlMultiRegionAccessPoint_threeRegions(t *testing.T) { bucket3Name := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - if acctest.Partition() == "aws-us-gov" { - t.Skip("S3 Multi-Region Access Point is not supported in GovCloud partition") - } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckMultipleRegion(t, 3) }, + PreCheck: func() { + acctest.PreCheck(t) + acctest.PreCheckMultipleRegion(t, 3) + acctest.PreCheckPartitionNot(t, endpoints.AwsUsGovPartitionID) + }, ErrorCheck: acctest.ErrorCheck(t, s3control.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5FactoriesMultipleRegions(t, 3), CheckDestroy: testAccCheckMultiRegionAccessPointDestroy, @@ -230,7 +216,7 @@ func testAccCheckMultiRegionAccessPointDestroy(s *terraform.State) error { return err } - _, err = tfs3control.FindMultiRegionAccessPointByAccountIDAndName(conn, accountID, name) + _, err = tfs3control.FindMultiRegionAccessPointByTwoPartKey(context.Background(), conn, accountID, name) if tfresource.NotFound(err) { continue @@ -269,7 +255,7 @@ func testAccCheckMultiRegionAccessPointExists(n string, v *s3control.MultiRegion return err } - output, err := tfs3control.FindMultiRegionAccessPointByAccountIDAndName(conn, accountID, name) + output, err := tfs3control.FindMultiRegionAccessPointByTwoPartKey(context.Background(), conn, accountID, name) if err != nil { return err diff --git a/internal/service/s3control/status.go b/internal/service/s3control/status.go deleted file mode 100644 index 51ed667e6053..000000000000 --- a/internal/service/s3control/status.go +++ /dev/null @@ -1,24 +0,0 @@ -package s3control - -import ( - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/s3control" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/hashicorp/terraform-provider-aws/internal/tfresource" -) - -func statusMultiRegionAccessPointRequest(conn *s3control.S3Control, accountID string, requestTokenARN string) resource.StateRefreshFunc { - return func() (interface{}, string, error) { - output, err := findMultiRegionAccessPointOperationByAccountIDAndTokenARN(conn, accountID, requestTokenARN) - - if tfresource.NotFound(err) { - return nil, "", nil - } - - if err != nil { - return nil, "", err - } - - return output, aws.StringValue(output.RequestStatus), nil - } -} diff --git a/internal/service/s3control/sweep.go b/internal/service/s3control/sweep.go index 60ccfb4994c3..4ed1c23a78ff 100644 --- a/internal/service/s3control/sweep.go +++ b/internal/service/s3control/sweep.go @@ -116,7 +116,7 @@ func sweepMultiRegionAccessPoints(region string) error { } for _, v := range page.AccessPoints { - r := ResourceMultiRegionAccessPoint() + r := resourceMultiRegionAccessPoint() d := r.Data(nil) d.SetId(MultiRegionAccessPointCreateResourceID(accountID, aws.StringValue(v.Name))) diff --git a/internal/service/s3control/wait.go b/internal/service/s3control/wait.go deleted file mode 100644 index eb779f941fc7..000000000000 --- a/internal/service/s3control/wait.go +++ /dev/null @@ -1,48 +0,0 @@ -package s3control - -import ( - "fmt" - "time" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/s3control" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/hashicorp/terraform-provider-aws/internal/tfresource" -) - -const ( - // Minimum amount of times to verify change propagation - propagationContinuousTargetOccurence = 2 - - // Minimum amount of time to wait between S3control change polls - propagationMinTimeout = 5 * time.Second - - // Maximum amount of time to wait for S3control changes to propagate - propagationTimeout = 1 * time.Minute - - multiRegionAccessPointRequestSucceededMinTimeout = 5 * time.Second - - multiRegionAccessPointRequestSucceededDelay = 15 * time.Second -) - -func waitMultiRegionAccessPointRequestSucceeded(conn *s3control.S3Control, accountID string, requestTokenArn string, timeout time.Duration) (*s3control.AsyncOperation, error) { //nolint:unparam - stateConf := &resource.StateChangeConf{ - Target: []string{RequestStatusSucceeded}, - Timeout: timeout, - Refresh: statusMultiRegionAccessPointRequest(conn, accountID, requestTokenArn), - MinTimeout: multiRegionAccessPointRequestSucceededMinTimeout, - Delay: multiRegionAccessPointRequestSucceededDelay, // Wait 15 secs before starting - } - - outputRaw, err := stateConf.WaitForState() - - if output, ok := outputRaw.(*s3control.AsyncOperation); ok { - if status, responseDetails := aws.StringValue(output.RequestStatus), output.ResponseDetails; status == RequestStatusFailed && responseDetails != nil && responseDetails.ErrorDetails != nil { - tfresource.SetLastError(err, fmt.Errorf("%s: %s", aws.StringValue(responseDetails.ErrorDetails.Code), aws.StringValue(responseDetails.ErrorDetails.Message))) - } - - return output, err - } - - return nil, err -}