Skip to content

Commit

Permalink
Merge pull request #40929 from acwwat/f-aws_datasync_location_s3-fix_…
Browse files Browse the repository at this point in the history
…s3_on_outposts_read_err

fix: Fix S3 on Outposts read error for aws_datasync_location_s3
  • Loading branch information
ewbankkit authored Jan 15, 2025
2 parents 97c4c3e + 7276597 commit d98c23d
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 11 deletions.
3 changes: 3 additions & 0 deletions .changelog/40929.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_datasync_location_s3: Fix `location URI global ID and subdirectory (...) does not match pattern "..."` errors on Read when `s3_bucket_arn` is an S3 on Outposts access point
```
15 changes: 11 additions & 4 deletions internal/service/datasync/location_s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package datasync

import (
"context"
"fmt"
"log"
"strings"

Expand Down Expand Up @@ -175,7 +174,7 @@ func resourceLocationS3Read(ctx context.Context, d *schema.ResourceData, meta in
}

uri := aws.ToString(output.LocationUri)
s3BucketName, err := globalIDFromLocationURI(aws.ToString(output.LocationUri))
globalID, err := globalIDFromLocationURI(aws.ToString(output.LocationUri))
if err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
Expand All @@ -190,8 +189,16 @@ func resourceLocationS3Read(ctx context.Context, d *schema.ResourceData, meta in

d.Set("agent_arns", output.AgentArns)
d.Set(names.AttrARN, output.LocationArn)
s3BucketArn := fmt.Sprintf("arn:%s:s3:::%s", locationARN.Partition, s3BucketName)
d.Set("s3_bucket_arn", s3BucketArn)
if arn.IsARN(globalID) {
d.Set("s3_bucket_arn", globalID)
} else {
arn := arn.ARN{
Partition: locationARN.Partition,
Service: "s3",
Resource: globalID,
}.String()
d.Set("s3_bucket_arn", arn)
}
if err := d.Set("s3_config", flattenS3Config(output.S3Config)); err != nil {
return sdkdiag.AppendErrorf(diags, "setting s3_config: %s", err)
}
Expand Down
23 changes: 20 additions & 3 deletions internal/service/datasync/uri.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
var (
locationURIPattern = regexache.MustCompile(`^(azure-blob|efs|fsx[0-9a-z-]+|hdfs|nfs|s3|smb)://(.+)$`)
locationURIGlobalIDAndSubdirPattern = regexache.MustCompile(`^([0-9A-Za-z.-]+)(?::\d{0,5})?(/.*)$`)
s3OutpostsAccessPointARNResourcePattern = regexache.MustCompile(`^outpost/.*/accesspoint/.*?(/.*)$`)
s3OutpostsAccessPointARNResourcePattern = regexache.MustCompile(`^(outpost/.*/accesspoint/.*?)(/.*)$`)
)

// globalIDFromLocationURI extracts the global ID from a location URI.
Expand All @@ -26,6 +26,23 @@ func globalIDFromLocationURI(uri string) (string, error) {
}

globalIDAndSubdir := submatches[2]
if parsedARN, err := arn.Parse(globalIDAndSubdir); err == nil {
submatches = s3OutpostsAccessPointARNResourcePattern.FindStringSubmatch(parsedARN.Resource)

if len(submatches) != 3 {
return "", fmt.Errorf("location URI S3 on Outposts access point ARN resource (%s) does not match pattern %q", parsedARN.Resource, s3OutpostsAccessPointARNResourcePattern)
}

s3OutpostsAccessPointARN := arn.ARN{
Partition: parsedARN.Partition,
Service: parsedARN.Service,
Region: parsedARN.Region,
AccountID: parsedARN.AccountID,
Resource: submatches[1],
}.String()

return s3OutpostsAccessPointARN, nil
}

submatches = locationURIGlobalIDAndSubdirPattern.FindStringSubmatch(globalIDAndSubdir)

Expand All @@ -51,11 +68,11 @@ func subdirectoryFromLocationURI(uri string) (string, error) {
if err == nil {
submatches = s3OutpostsAccessPointARNResourcePattern.FindStringSubmatch(parsedARN.Resource)

if len(submatches) != 2 {
if len(submatches) != 3 {
return "", fmt.Errorf("location URI S3 on Outposts access point ARN resource (%s) does not match pattern %q", parsedARN.Resource, s3OutpostsAccessPointARNResourcePattern)
}

return submatches[1], nil
return submatches[2], nil
}

submatches = locationURIGlobalIDAndSubdirPattern.FindStringSubmatch(globalIDAndSubdir)
Expand Down
188 changes: 187 additions & 1 deletion internal/service/datasync/uri_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,193 @@

package datasync

import "testing"
import (
"testing"

"github.com/hashicorp/terraform-provider-aws/names"
)

func TestGlobalIDFromLocationURI(t *testing.T) {
t.Parallel()

testCases := []struct {
TestName string
InputURI string
ExpectedError bool
ExpectedSubdirectory string
}{
{
TestName: "empty URI",
InputURI: "",
ExpectedError: true,
},
{
TestName: "invalid URI scheme",
InputURI: "test://testing/",
ExpectedError: true,
},
{
TestName: "S3 bucket URI no bucket name (1)",
InputURI: "s3://",
ExpectedError: true,
},
{
TestName: "S3 bucket URI no bucket name (2)",
InputURI: "s3:///",
ExpectedError: true,
},
{
TestName: "S3 bucket URI top level",
InputURI: "s3://bucket/",
ExpectedSubdirectory: names.AttrBucket,
},
{
TestName: "S3 bucket URI one level",
InputURI: "s3://bucket/my-folder-1/",
ExpectedSubdirectory: names.AttrBucket,
},
{
TestName: "S3 bucket URI two levels",
InputURI: "s3://bucket/my-folder-1/my-folder-2",
ExpectedSubdirectory: names.AttrBucket,
},
{
TestName: "S3 Outposts ARN URI top level",
InputURI: "s3://arn:aws:s3-outposts:eu-west-3:123456789012:outpost/op-YYYYYYYYYY/accesspoint/my-access-point/", //lintignore:AWSAT003,AWSAT005
ExpectedSubdirectory: "arn:aws:s3-outposts:eu-west-3:123456789012:outpost/op-YYYYYYYYYY/accesspoint/my-access-point", //lintignore:AWSAT003,AWSAT005
},
{
TestName: "S3 Outposts ARN URI one level",
InputURI: "s3://arn:aws:s3-outposts:eu-west-3:123456789012:outpost/op-YYYYYYYYYY/accesspoint/my-access-point/my-folder-1/", //lintignore:AWSAT003,AWSAT005
ExpectedSubdirectory: "arn:aws:s3-outposts:eu-west-3:123456789012:outpost/op-YYYYYYYYYY/accesspoint/my-access-point", //lintignore:AWSAT003,AWSAT005
},
{
TestName: "S3 Outposts ARN URI two levels",
InputURI: "s3://arn:aws:s3-outposts:eu-west-3:123456789012:outpost/op-YYYYYYYYYY/accesspoint/my-access-point/my-folder-1/my-folder-2", //lintignore:AWSAT003,AWSAT005
ExpectedSubdirectory: "arn:aws:s3-outposts:eu-west-3:123456789012:outpost/op-YYYYYYYYYY/accesspoint/my-access-point", //lintignore:AWSAT003,AWSAT005
},
{
TestName: "EFS URI top level",
InputURI: "efs://us-west-2.fs-abcdef01/", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef01", //lintignore:AWSAT003
},
{
TestName: "EFS URI one level",
InputURI: "efs://us-west-2.fs-abcdef01/my-folder-1/", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef01", //lintignore:AWSAT003
},
{
TestName: "EFS URI two levels",
InputURI: "efs://us-west-2.fs-abcdef01/my-folder-1/my-folder-2", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef01", //lintignore:AWSAT003
},
{
TestName: "NFS URI top level",
InputURI: "nfs://example.com/",
ExpectedSubdirectory: "example.com",
},
{
TestName: "NFS URI one level",
InputURI: "nfs://example.com/my-folder-1/",
ExpectedSubdirectory: "example.com",
},
{
TestName: "NFS URI two levels",
InputURI: "nfs://example.com/my-folder-1/my-folder-2",
ExpectedSubdirectory: "example.com",
},
{
TestName: "SMB URI top level",
InputURI: "smb://192.168.1.1/",
ExpectedSubdirectory: "192.168.1.1",
},
{
TestName: "SMB URI one level",
InputURI: "smb://192.168.1.1/my-folder-1/",
ExpectedSubdirectory: "192.168.1.1",
},
{
TestName: "SMB URI two levels",
InputURI: "smb://192.168.1.1/my-folder-1/my-folder-2",
ExpectedSubdirectory: "192.168.1.1",
},
{
TestName: "HDFS URI top level",
InputURI: "hdfs://192.168.1.1:80/",
ExpectedSubdirectory: "192.168.1.1",
},
{
TestName: "HDFS URI one level",
InputURI: "hdfs://192.168.1.1:80/my-folder-1/",
ExpectedSubdirectory: "192.168.1.1",
},
{
TestName: "HDFS URI two levels",
InputURI: "hdfs://192.168.1.1:80/my-folder-1/my-folder-2",
ExpectedSubdirectory: "192.168.1.1",
},
{
TestName: "FSx Windows URI top level",
InputURI: "fsxw://us-west-2.fs-abcdef123456789012/", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef123456789012", //lintignore:AWSAT003
},
{
TestName: "FSx Windows URI one level",
InputURI: "fsxw://us-west-2.fs-abcdef123456789012/my-folder-1/", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef123456789012", //lintignore:AWSAT003
},
{
TestName: "FSx Windows URI two levels",
InputURI: "fsxw://us-west-2.fs-abcdef123456789012/my-folder-1/my-folder-2", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef123456789012", //lintignore:AWSAT003
},
{
TestName: "FSx Zfs URI top level",
InputURI: "fsxz://us-west-2.fs-abcdef123456789012/", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef123456789012", //lintignore:AWSAT003
},
{
TestName: "FSx Zfs URI one level",
InputURI: "fsxz://us-west-2.fs-abcdef123456789012/my-folder-1/", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef123456789012", //lintignore:AWSAT003
},
{
TestName: "FSx Zfs URI two levels",
InputURI: "fsxz://us-west-2.fs-abcdef123456789012/my-folder-1/my-folder-2", //lintignore:AWSAT003
ExpectedSubdirectory: "us-west-2.fs-abcdef123456789012", //lintignore:AWSAT003
},
{
TestName: "Object storage two levels",
InputURI: "object-storage://192.168.1.1/tf-acc-test-5815577519131245007/tf-acc-test-5815577519131245008/",
ExpectedError: true,
},
{
TestName: "Azure blob URI one level",
InputURI: "azure-blob://example.com/path/",
ExpectedSubdirectory: "example.com",
},
}

for _, testCase := range testCases {
t.Run(testCase.TestName, func(t *testing.T) {
t.Parallel()

got, err := globalIDFromLocationURI(testCase.InputURI)

if err == nil && testCase.ExpectedError {
t.Fatalf("expected error")
}

if err != nil && !testCase.ExpectedError {
t.Fatalf("unexpected error: %s", err)
}

if got != testCase.ExpectedSubdirectory {
t.Errorf("got %s, expected %s", got, testCase.ExpectedSubdirectory)
}
})
}
}

func TestSubdirectoryFromLocationURI(t *testing.T) {
t.Parallel()
Expand Down
23 changes: 20 additions & 3 deletions website/docs/r/datasync_location_s3.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Manages an S3 Location within AWS DataSync.

## Example Usage

### Basic Usage

```terraform
resource "aws_datasync_location_s3" "example" {
s3_bucket_arn = aws_s3_bucket.example.arn
Expand All @@ -23,14 +25,29 @@ resource "aws_datasync_location_s3" "example" {
}
```

### S3 Bucket on AWS Outposts

```terraform
resource "aws_datasync_location_s3" "destination" {
agent_arns = [aws_datasync_agent.example.arn]
s3_bucket_arn = aws_s3_access_point.example.arn
s3_storage_class = "OUTPOSTS"
subdirectory = "/example/prefix"
s3_config {
bucket_access_role_arn = aws_iam_role.example.arn
}
}
```

## Argument Reference

This resource supports the following arguments:

* `agent_arns` - (Optional) A list of DataSync Agent ARNs with which this location will be associated.
* `s3_bucket_arn` - (Required) Amazon Resource Name (ARN) of the S3 Bucket.
* `agent_arns` - (Optional) (Amazon S3 on Outposts only) Amazon Resource Name (ARN) of the DataSync agent on the Outpost.
* `s3_bucket_arn` - (Required) Amazon Resource Name (ARN) of the S3 bucket, or the Amazon S3 access point if the S3 bucket is located on an AWS Outposts resource.
* `s3_config` - (Required) Configuration block containing information for connecting to S3.
* `s3_storage_class` - (Optional) The Amazon S3 storage class that you want to store your files in when this location is used as a task destination. [Valid values](https://docs.aws.amazon.com/datasync/latest/userguide/create-s3-location.html#using-storage-classes)
* `s3_storage_class` - (Optional) Amazon S3 storage class that you want to store your files in when this location is used as a task destination. [Valid values](https://docs.aws.amazon.com/datasync/latest/userguide/create-s3-location.html#using-storage-classes)
* `subdirectory` - (Required) Prefix to perform actions as source or destination.
* `tags` - (Optional) Key-value pairs of resource tags to assign to the DataSync Location. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level.

Expand Down

0 comments on commit d98c23d

Please sign in to comment.