diff --git a/util/pkg/vfs/s3fs.go b/util/pkg/vfs/s3fs.go index 3501a284826cf..df12a130fde2e 100644 --- a/util/pkg/vfs/s3fs.go +++ b/util/pkg/vfs/s3fs.go @@ -561,13 +561,18 @@ func (p *S3Path) GetHTTPsUrl(dualstack bool) (string, error) { return "", fmt.Errorf("failed to get bucket details for %q: %w", p.String(), err) } - var url string - if dualstack { - url = fmt.Sprintf("https://s3.dualstack.%s.amazonaws.com/%s/%s", bucketDetails.region, bucketDetails.name, p.Key()) - } else { - url = fmt.Sprintf("https://%s.s3.%s.amazonaws.com/%s", bucketDetails.name, bucketDetails.region, p.Key()) + resolver := s3.NewDefaultEndpointResolverV2() + endpoint, err := resolver.ResolveEndpoint(ctx, s3.EndpointParameters{ + Bucket: aws.String(bucketDetails.name), + Region: aws.String(bucketDetails.region), + UseDualStack: aws.Bool(dualstack), + }) + if err != nil { + return "", fmt.Errorf("failed to resolve endpoint for %q: %w", p.String(), err) } - return strings.TrimSuffix(url, "/"), nil + + endpoint.URI.Path = path.Join(endpoint.URI.Path, p.Key()) + return endpoint.URI.String(), nil } func (p *S3Path) IsBucketPublic(ctx context.Context) (bool, error) { diff --git a/util/pkg/vfs/s3fs_test.go b/util/pkg/vfs/s3fs_test.go index 647a362e50e97..e167fffffd91d 100644 --- a/util/pkg/vfs/s3fs_test.go +++ b/util/pkg/vfs/s3fs_test.go @@ -64,3 +64,77 @@ func Test_S3Path_Parse(t *testing.T) { } } } + +func TestGetHTTPsUrl(t *testing.T) { + grid := []struct { + Path string + Dualstack bool + Region string + ExpectedURL string + }{ + { + Path: "s3://bucket", + Region: "us-east-1", + ExpectedURL: "https://bucket.s3.us-east-1.amazonaws.com", + }, + { + Path: "s3://bucket.with.forced.path.style/subpath", + Region: "us-east-1", + ExpectedURL: "https://s3.us-east-1.amazonaws.com/bucket.with.forced.path.style/subpath", + }, + { + Path: "s3://bucket/path", + Region: "us-east-2", + ExpectedURL: "https://bucket.s3.us-east-2.amazonaws.com/path", + }, + { + Path: "s3://bucket2/path/subpath", + Region: "us-east-1", + ExpectedURL: "https://bucket2.s3.us-east-1.amazonaws.com/path/subpath", + }, + { + Path: "s3://bucket2-ds/path/subpath", + Dualstack: true, + Region: "us-east-1", + ExpectedURL: "https://bucket2-ds.s3.dualstack.us-east-1.amazonaws.com/path/subpath", + }, + { + Path: "s3://bucket2-cn/path/subpath", + Region: "cn-north-1", + ExpectedURL: "https://bucket2-cn.s3.cn-north-1.amazonaws.com.cn/path/subpath", + }, + { + Path: "s3://bucket2-cn-ds/path/subpath", + Dualstack: true, + Region: "cn-north-1", + ExpectedURL: "https://bucket2-cn-ds.s3.dualstack.cn-north-1.amazonaws.com.cn/path/subpath", + }, + { + Path: "s3://bucket2-gov/path/subpath", + Region: "us-gov-west-1", + ExpectedURL: "https://bucket2-gov.s3.us-gov-west-1.amazonaws.com/path/subpath", + }, + { + Path: "s3://bucket2-gov-ds/path/subpath", + Dualstack: true, + Region: "us-gov-west-1", + ExpectedURL: "https://bucket2-gov-ds.s3.dualstack.us-gov-west-1.amazonaws.com/path/subpath", + }, + } + for _, g := range grid { + t.Run(g.Path, func(t *testing.T) { + // Must be nonempty in order to force S3_REGION usage + // rather than querying S3 for the region. + t.Setenv("S3_ENDPOINT", "1") + t.Setenv("S3_REGION", g.Region) + s3path, _ := Context.buildS3Path(g.Path) + url, err := s3path.GetHTTPsUrl(g.Dualstack) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if url != g.ExpectedURL { + t.Fatalf("expected url: %v vs actual url: %v", g.ExpectedURL, url) + } + }) + } +}