Skip to content

Commit

Permalink
fix(middleware-bucket-endpoint): remove dualstack from hostname befor…
Browse files Browse the repository at this point in the history
…e processing (#3003)
  • Loading branch information
trivikr committed Nov 8, 2021
1 parent f2fb280 commit d7aa2c3
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 7 deletions.
18 changes: 13 additions & 5 deletions packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ describe("bucketHostname", () => {
const region = "us-west-2";
describe("from bucket name", () => {
[
{ baseHostname: "s3.dualstack.us-west-2.amazonaws.com", isCustomEndpoint: false, dualstackEndpoint: true },
{ baseHostname: "s3.us-west-2.amazonaws.com", isCustomEndpoint: false },
{ baseHostname: "beta.example.com", isCustomEndpoint: true },
].forEach(({ baseHostname, isCustomEndpoint }) => {
Expand Down Expand Up @@ -187,20 +188,24 @@ describe("bucketHostname", () => {

describe("from Access Point ARN", () => {
describe("populates access point endpoint from ARN", () => {
const s3Hostname = "s3.us-west-2.amazonaws.com";
const customHostname = "example.com";

describe(`baseHostname: ${s3Hostname}`, () => {
const baseHostname = s3Hostname;
describe.each([
["s3.us-west-2.amazonaws.com", false],
["s3.dualstack.us-west-2.amazonaws.com", true],
])(`baseHostname: %s, dualstackEndpoint: %s`, (baseHostname, dualstackEndpoint) => {
it("should use client region", () => {
const { bucketEndpoint, hostname } = bucketHostname({
bucketName: parseArn("arn:aws:s3:us-west-2:123456789012:accesspoint:myendpoint"),
baseHostname,
isCustomEndpoint: false,
clientRegion: region,
dualstackEndpoint,
});
expect(bucketEndpoint).toBe(true);
expect(hostname).toBe("myendpoint-123456789012.s3-accesspoint.us-west-2.amazonaws.com");
expect(hostname).toBe(
`myendpoint-123456789012.s3-accesspoint${dualstackEndpoint ? ".dualstack" : ""}.us-west-2.amazonaws.com`
);
});

it("should use ARN region", () => {
Expand All @@ -210,9 +215,12 @@ describe("bucketHostname", () => {
isCustomEndpoint: false,
clientRegion: region,
useArnRegion: true,
dualstackEndpoint,
});
expect(bucketEndpoint).toBe(true);
expect(hostname).toBe("myendpoint-123456789012.s3-accesspoint.us-east-1.amazonaws.com");
expect(hostname).toBe(
`myendpoint-123456789012.s3-accesspoint${dualstackEndpoint ? ".dualstack" : ""}.us-east-1.amazonaws.com`
);
});
});

Expand Down
9 changes: 7 additions & 2 deletions packages/middleware-bucket-endpoint/src/bucketHostname.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,16 @@ export interface BucketHostname {

export const bucketHostname = (options: BucketHostnameParams | ArnHostnameParams): BucketHostname => {
validateCustomEndpoint(options);

// TODO: Remove checks for ".dualstack" from entire middleware.
const { dualstackEndpoint, baseHostname } = options;
const updatedBaseHostname = dualstackEndpoint ? baseHostname.replace(".dualstack", "") : baseHostname;

return isBucketNameOptions(options)
? // Construct endpoint when bucketName is a string referring to a bucket name
getEndpointFromBucketName(options)
getEndpointFromBucketName({ ...options, baseHostname: updatedBaseHostname })
: // Construct endpoint when bucketName is an ARN referring to an S3 resource like Access Point
getEndpointFromArn(options);
getEndpointFromArn({ ...options, baseHostname: updatedBaseHostname });
};

const getEndpointFromBucketName = ({
Expand Down

0 comments on commit d7aa2c3

Please sign in to comment.