From 5d0e1363329c63ef34e50fa0806761ce919c1405 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 13 Oct 2020 11:13:16 -0400 Subject: [PATCH 1/3] resource/aws_s3_access_point: Support S3 on Outposts Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/15412 Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/15616 Output from acceptance testing: ``` --- SKIP: TestAccAWSS3AccessPoint_Bucket_Arn (2.35s) --- PASS: TestAccAWSS3AccessPoint_bucketDisappears (25.12s) --- PASS: TestAccAWSS3AccessPoint_disappears (30.49s) --- PASS: TestAccAWSS3AccessPoint_VpcConfiguration (33.78s) --- PASS: TestAccAWSS3AccessPoint_PublicAccessBlockConfiguration (34.38s) --- PASS: TestAccAWSS3AccessPoint_basic (34.93s) --- PASS: TestAccAWSS3AccessPoint_Policy (91.95s) ``` --- aws/resource_aws_s3_access_point.go | 49 +++++++++---- aws/resource_aws_s3_access_point_test.go | 72 ++++++++++++++++++++ website/docs/r/s3_access_point.html.markdown | 23 ++++--- 3 files changed, 123 insertions(+), 21 deletions(-) diff --git a/aws/resource_aws_s3_access_point.go b/aws/resource_aws_s3_access_point.go index 393557580d53..009977f31d4a 100644 --- a/aws/resource_aws_s3_access_point.go +++ b/aws/resource_aws_s3_access_point.go @@ -137,12 +137,24 @@ func resourceAwsS3AccessPointCreate(d *schema.ResourceData, meta interface{}) er } log.Printf("[DEBUG] Creating S3 Access Point: %s", input) - _, err := conn.CreateAccessPoint(input) + output, err := conn.CreateAccessPoint(input) + if err != nil { - return fmt.Errorf("error creating S3 Access Point: %s", err) + return fmt.Errorf("error creating S3 Control Access Point (%s): %w", name, err) + } + + if output == nil { + return fmt.Errorf("error creating S3 Control Access Point (%s): empty response", name) } - d.SetId(fmt.Sprintf("%s:%s", accountId, name)) + parsedARN, err := arn.Parse(aws.StringValue(output.AccessPointArn)) + + if err == nil && strings.HasPrefix(parsedARN.Resource, "outpost/") { + d.SetId(aws.StringValue(output.AccessPointArn)) + name = aws.StringValue(output.AccessPointArn) + } else { + d.SetId(fmt.Sprintf("%s:%s", accountId, name)) + } if v, ok := d.GetOk("policy"); ok { log.Printf("[DEBUG] Putting S3 Access Point policy: %s", d.Id()) @@ -183,19 +195,23 @@ func resourceAwsS3AccessPointRead(d *schema.ResourceData, meta interface{}) erro return fmt.Errorf("error reading S3 Access Point (%s): %s", d.Id(), err) } - name = aws.StringValue(output.Name) - arn := arn.ARN{ - AccountID: accountId, - Partition: meta.(*AWSClient).partition, - Region: meta.(*AWSClient).region, - Resource: fmt.Sprintf("accesspoint/%s", name), - Service: "s3", + if strings.HasPrefix(name, "arn:") { + d.Set("arn", name) + } else { + builtARN := arn.ARN{ + AccountID: accountId, + Partition: meta.(*AWSClient).partition, + Region: meta.(*AWSClient).region, + Resource: fmt.Sprintf("accesspoint/%s", aws.StringValue(output.Name)), + Service: "s3", + } + d.Set("arn", builtARN.String()) } + d.Set("account_id", accountId) - d.Set("arn", arn.String()) d.Set("bucket", output.Bucket) - d.Set("domain_name", meta.(*AWSClient).RegionalHostname(fmt.Sprintf("%s-%s.s3-accesspoint", name, accountId))) - d.Set("name", name) + d.Set("domain_name", meta.(*AWSClient).RegionalHostname(fmt.Sprintf("%s-%s.s3-accesspoint", aws.StringValue(output.Name), accountId))) + d.Set("name", output.Name) d.Set("network_origin", output.NetworkOrigin) if err := d.Set("public_access_block_configuration", flattenS3AccessPointPublicAccessBlockConfiguration(output.PublicAccessBlockConfiguration)); err != nil { return fmt.Errorf("error setting public_access_block_configuration: %s", err) @@ -298,7 +314,14 @@ func resourceAwsS3AccessPointDelete(d *schema.ResourceData, meta interface{}) er return nil } +// s3AccessPointParseId returns the Account ID and Access Point Name (S3) or ARN (S3 on Outposts) func s3AccessPointParseId(id string) (string, string, error) { + parsedARN, err := arn.Parse(id) + + if err == nil { + return parsedARN.AccountID, id, nil + } + parts := strings.SplitN(id, ":", 2) if len(parts) != 2 || parts[0] == "" || parts[1] == "" { diff --git a/aws/resource_aws_s3_access_point_test.go b/aws/resource_aws_s3_access_point_test.go index f9c92f80b234..cf2357b5d68d 100644 --- a/aws/resource_aws_s3_access_point_test.go +++ b/aws/resource_aws_s3_access_point_test.go @@ -165,6 +165,46 @@ func TestAccAWSS3AccessPoint_bucketDisappears(t *testing.T) { }) } +func TestAccAWSS3AccessPoint_Bucket_Arn(t *testing.T) { + var v s3control.GetAccessPointOutput + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_s3_access_point.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSOutpostsOutposts(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSS3AccessPointDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSS3AccessPointConfig_Bucket_Arn(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3AccessPointExists(resourceName, &v), + testAccCheckResourceAttrAccountID(resourceName, "account_id"), + testAccCheckResourceAttrRegionalARN(resourceName, "arn", "s3-outposts", fmt.Sprintf("outpost/[^/]+/accesspoint/%s", rName)), + resource.TestCheckResourceAttrPair(resourceName, "bucket", "aws_s3control_bucket.test", "arn"), + testAccMatchResourceAttrRegionalHostname(resourceName, "domain_name", "s3-accesspoint", regexp.MustCompile(fmt.Sprintf("^%s-\\d{12}", rName))), + resource.TestCheckResourceAttr(resourceName, "has_public_access_policy", "false"), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "network_origin", "VPC"), + resource.TestCheckResourceAttr(resourceName, "policy", ""), + resource.TestCheckResourceAttr(resourceName, "public_access_block_configuration.#", "1"), + resource.TestCheckResourceAttr(resourceName, "public_access_block_configuration.0.block_public_acls", "true"), + resource.TestCheckResourceAttr(resourceName, "public_access_block_configuration.0.block_public_policy", "true"), + resource.TestCheckResourceAttr(resourceName, "public_access_block_configuration.0.ignore_public_acls", "true"), + resource.TestCheckResourceAttr(resourceName, "public_access_block_configuration.0.restrict_public_buckets", "true"), + resource.TestCheckResourceAttr(resourceName, "vpc_configuration.#", "1"), + resource.TestCheckResourceAttrPair(resourceName, "vpc_configuration.0.vpc_id", "aws_vpc.test", "id"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccAWSS3AccessPoint_Policy(t *testing.T) { var v s3control.GetAccessPointOutput rName := acctest.RandomWithPrefix("tf-acc-test") @@ -477,6 +517,38 @@ resource "aws_s3_access_point" "test" { `, bucketName, accessPointName) } +func testAccAWSS3AccessPointConfig_Bucket_Arn(rName string) string { + return fmt.Sprintf(` +data "aws_outposts_outposts" "test" {} + +data "aws_outposts_outpost" "test" { + id = tolist(data.aws_outposts_outposts.test.ids)[0] +} + +resource "aws_s3control_bucket" "test" { + bucket = %[1]q + outpost_id = data.aws_outposts_outpost.test.id +} + +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_s3_access_point" "test" { + bucket = aws_s3_bucket.test.arn + name = %[1]q + + vpc_configuration { + vpc_id = aws_vpc.test.id + } +} +`, rName) +} + func testAccAWSS3AccessPointConfig_policy(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { diff --git a/website/docs/r/s3_access_point.html.markdown b/website/docs/r/s3_access_point.html.markdown index a2245f3ea9b0..555a222dcc4d 100644 --- a/website/docs/r/s3_access_point.html.markdown +++ b/website/docs/r/s3_access_point.html.markdown @@ -14,7 +14,7 @@ Provides a resource to manage an S3 Access Point. ## Example Usage -### Basic Usage +### AWS Partition Bucket ```hcl resource "aws_s3_bucket" "example" { @@ -27,17 +27,18 @@ resource "aws_s3_access_point" "example" { } ``` -### Access Point Restricted to a VPC +### S3 on Outposts Bucket ```hcl -resource "aws_s3_bucket" "example" { +resource "aws_s3control_bucket" "example" { bucket = "example" } resource "aws_s3_access_point" "example" { - bucket = aws_s3_bucket.example.id + bucket = aws_s3control_bucket.example.arn name = "example" + # VPC must be specified for S3 on Outposts vpc_configuration { vpc_id = aws_vpc.example.id } @@ -52,7 +53,7 @@ resource "aws_vpc" "example" { The following arguments are required: -* `bucket` - (Required) The name of the bucket that you want to associate this access point with. +* `bucket` - (Required) The name of an AWS Partition S3 Bucket or the Amazon Resource Name (ARN) of S3 on Outposts Bucket that you want to associate this access point with. * `name` - (Required) The name you want to assign to this access point. The following arguments are optional: @@ -60,7 +61,7 @@ The following arguments are optional: * `account_id` - (Optional) The AWS account ID for the owner of the bucket for which you want to create an access point. Defaults to automatically determined account ID of the Terraform AWS provider. * `policy` - (Optional) A valid JSON document that specifies the policy that you want to apply to this access point. * `public_access_block_configuration` - (Optional) Configuration block to manage the `PublicAccessBlock` configuration that you want to apply to this Amazon S3 bucket. You can enable the configuration options in any combination. Detailed below. -* `vpc_configuration` - (Optional) Configuration block to restrict access to this access point to requests from the specified Virtual Private Cloud (VPC). Detailed below. +* `vpc_configuration` - (Optional) Configuration block to restrict access to this access point to requests from the specified Virtual Private Cloud (VPC). Required for S3 on Outposts. Detailed below. ### public_access_block_configuration Configuration Block @@ -91,13 +92,19 @@ In addition to all arguments above, the following attributes are exported: * `domain_name` - The DNS domain name of the S3 Access Point in the format _`name`_-_`account_id`_.s3-accesspoint._region_.amazonaws.com. Note: S3 access points only support secure access by HTTPS. HTTP isn't supported. * `has_public_access_policy` - Indicates whether this access point currently has a policy that allows public access. -* `id` - AWS account ID and access point name separated by a colon (`:`). +* `id` - For Access Point of an AWS Partition S3 Bucket, the AWS account ID and access point name separated by a colon (`:`). For S3 on Outposts Bucket, the Amazon Resource Name (ARN) of the Access Point. * `network_origin` - Indicates whether this access point allows access from the public Internet. Values are `VPC` (the access point doesn't allow access from the public Internet) and `Internet` (the access point allows access from the public Internet, subject to the access point and bucket access policies). ## Import -S3 Access Points can be imported using the `account_id` and `name` separated by a colon (`:`), e.g. +For Access Points associated with an AWS Partition S3 Bucket, this resource can be imported using the `account_id` and `name` separated by a colon (`:`), e.g. ``` $ terraform import aws_s3_access_point.example 123456789012:example ``` + +For Access Points associated with an S3 on Outposts Bucket, this resource can be imported using the Amazon Resource Name (ARN), e.g. + +``` +$ terraform import aws_s3_access_point.example arn:aws:s3-outposts:us-east-1:123456789012:outpost/op-1234567890123456/accesspoint/example +``` From c7aa8e5d0fa012cb43644455658a0e34f22bdccf Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 15 Oct 2020 15:00:05 -0400 Subject: [PATCH 2/3] resource/aws_s3_access_point: Handle bucket as ARN and skip GetAccessPointPolicyStatus API call with S3 on Outposts Output from acceptance testing: ``` --- SKIP: TestAccAWSS3AccessPoint_Bucket_Arn (2.80s) --- PASS: TestAccAWSS3AccessPoint_bucketDisappears (23.75s) --- PASS: TestAccAWSS3AccessPoint_disappears (28.66s) --- PASS: TestAccAWSS3AccessPoint_PublicAccessBlockConfiguration (32.91s) --- PASS: TestAccAWSS3AccessPoint_VpcConfiguration (33.05s) --- PASS: TestAccAWSS3AccessPoint_basic (33.64s) --- PASS: TestAccAWSS3AccessPoint_Policy (89.22s) ``` --- aws/resource_aws_s3_access_point.go | 34 +++++++++++++++++++++--- aws/resource_aws_s3_access_point_test.go | 2 +- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_s3_access_point.go b/aws/resource_aws_s3_access_point.go index 009977f31d4a..8baebe03359a 100644 --- a/aws/resource_aws_s3_access_point.go +++ b/aws/resource_aws_s3_access_point.go @@ -196,20 +196,41 @@ func resourceAwsS3AccessPointRead(d *schema.ResourceData, meta interface{}) erro } if strings.HasPrefix(name, "arn:") { + parsedAccessPointARN, err := arn.Parse(name) + + if err != nil { + return fmt.Errorf("error parsing S3 Control Access Point ARN (%s): %w", name, err) + } + + bucketARN := arn.ARN{ + AccountID: parsedAccessPointARN.AccountID, + Partition: parsedAccessPointARN.Partition, + Region: parsedAccessPointARN.Region, + Resource: strings.Replace( + parsedAccessPointARN.Resource, + fmt.Sprintf("accesspoint/%s", aws.StringValue(output.Name)), + fmt.Sprintf("bucket/%s", aws.StringValue(output.Bucket)), + 1, + ), + Service: parsedAccessPointARN.Service, + } + d.Set("arn", name) + d.Set("bucket", bucketARN.String()) } else { - builtARN := arn.ARN{ + accessPointARN := arn.ARN{ AccountID: accountId, Partition: meta.(*AWSClient).partition, Region: meta.(*AWSClient).region, Resource: fmt.Sprintf("accesspoint/%s", aws.StringValue(output.Name)), Service: "s3", } - d.Set("arn", builtARN.String()) + + d.Set("arn", accessPointARN.String()) + d.Set("bucket", output.Bucket) } d.Set("account_id", accountId) - d.Set("bucket", output.Bucket) d.Set("domain_name", meta.(*AWSClient).RegionalHostname(fmt.Sprintf("%s-%s.s3-accesspoint", aws.StringValue(output.Name), accountId))) d.Set("name", output.Name) d.Set("network_origin", output.NetworkOrigin) @@ -235,6 +256,13 @@ func resourceAwsS3AccessPointRead(d *schema.ResourceData, meta interface{}) erro d.Set("policy", policyOutput.Policy) } + // Return early since S3 on Outposts cannot have public policies + if strings.HasPrefix(name, "arn:") { + d.Set("has_public_access_policy", false) + + return nil + } + policyStatusOutput, err := conn.GetAccessPointPolicyStatus(&s3control.GetAccessPointPolicyStatusInput{ AccountId: aws.String(accountId), Name: aws.String(name), diff --git a/aws/resource_aws_s3_access_point_test.go b/aws/resource_aws_s3_access_point_test.go index cf2357b5d68d..4357e68103db 100644 --- a/aws/resource_aws_s3_access_point_test.go +++ b/aws/resource_aws_s3_access_point_test.go @@ -185,7 +185,7 @@ func TestAccAWSS3AccessPoint_Bucket_Arn(t *testing.T) { testAccMatchResourceAttrRegionalHostname(resourceName, "domain_name", "s3-accesspoint", regexp.MustCompile(fmt.Sprintf("^%s-\\d{12}", rName))), resource.TestCheckResourceAttr(resourceName, "has_public_access_policy", "false"), resource.TestCheckResourceAttr(resourceName, "name", rName), - resource.TestCheckResourceAttr(resourceName, "network_origin", "VPC"), + resource.TestCheckResourceAttr(resourceName, "network_origin", "Vpc"), resource.TestCheckResourceAttr(resourceName, "policy", ""), resource.TestCheckResourceAttr(resourceName, "public_access_block_configuration.#", "1"), resource.TestCheckResourceAttr(resourceName, "public_access_block_configuration.0.block_public_acls", "true"), From 3f5d0077011b965e15a82a3aded0fb4fd5f0535c Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 27 Oct 2020 16:43:41 -0400 Subject: [PATCH 3/3] Update aws/resource_aws_s3_access_point_test.go Co-authored-by: angie pinilla --- aws/resource_aws_s3_access_point_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_s3_access_point_test.go b/aws/resource_aws_s3_access_point_test.go index 4357e68103db..8dd9b0409558 100644 --- a/aws/resource_aws_s3_access_point_test.go +++ b/aws/resource_aws_s3_access_point_test.go @@ -539,7 +539,7 @@ resource "aws_vpc" "test" { } resource "aws_s3_access_point" "test" { - bucket = aws_s3_bucket.test.arn + bucket = aws_s3control_bucket.test.arn name = %[1]q vpc_configuration {