From b404c8bb29a6685facbfb48cc4f4110a30ff3c05 Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 8 Nov 2021 18:58:23 +0000 Subject: [PATCH 1/4] fix(middleware-bucket-endpoint): remove dualstack from hostname before processing --- packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts | 1 + packages/middleware-bucket-endpoint/src/bucketHostname.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts b/packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts index 8f8cc9a55c2e..7cb10fe9a788 100644 --- a/packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts +++ b/packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts @@ -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 }) => { diff --git a/packages/middleware-bucket-endpoint/src/bucketHostname.ts b/packages/middleware-bucket-endpoint/src/bucketHostname.ts index f715f0d6f0f3..2dc5d072c3e2 100644 --- a/packages/middleware-bucket-endpoint/src/bucketHostname.ts +++ b/packages/middleware-bucket-endpoint/src/bucketHostname.ts @@ -51,7 +51,8 @@ const getEndpointFromBucketName = ({ tlsCompatible = true, isCustomEndpoint = false, }: BucketHostnameParams): BucketHostname => { - const [clientRegion, hostnameSuffix] = isCustomEndpoint ? [region, baseHostname] : getSuffix(baseHostname); + const suffixHostname = dualstackEndpoint ? baseHostname.replace(".dualstack", "") : baseHostname; + const [clientRegion, hostnameSuffix] = isCustomEndpoint ? [region, baseHostname] : getSuffix(suffixHostname); if (pathStyleEndpoint || !isDnsCompatibleBucketName(bucketName) || (tlsCompatible && DOT_PATTERN.test(bucketName))) { return { bucketEndpoint: false, From 7a57d5bfd1e87b4b370b17148c7e20104c00230e Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Mon, 8 Nov 2021 11:18:45 -0800 Subject: [PATCH 2/4] chore(docs): add a TODO to remove `.dualstack` checks from middleware --- packages/middleware-bucket-endpoint/src/bucketHostname.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/middleware-bucket-endpoint/src/bucketHostname.ts b/packages/middleware-bucket-endpoint/src/bucketHostname.ts index 2dc5d072c3e2..3ca767ebad5e 100644 --- a/packages/middleware-bucket-endpoint/src/bucketHostname.ts +++ b/packages/middleware-bucket-endpoint/src/bucketHostname.ts @@ -51,6 +51,7 @@ const getEndpointFromBucketName = ({ tlsCompatible = true, isCustomEndpoint = false, }: BucketHostnameParams): BucketHostname => { + // TODO: Remove checks for ".dualstack" from entire middleware. const suffixHostname = dualstackEndpoint ? baseHostname.replace(".dualstack", "") : baseHostname; const [clientRegion, hostnameSuffix] = isCustomEndpoint ? [region, baseHostname] : getSuffix(suffixHostname); if (pathStyleEndpoint || !isDnsCompatibleBucketName(bucketName) || (tlsCompatible && DOT_PATTERN.test(bucketName))) { From ce4db0dd499e3aea0b0f7aabea6a41d11bd3b4bf Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 8 Nov 2021 19:28:31 +0000 Subject: [PATCH 3/4] chore: move replacing of dualstack to bucketHostname --- .../src/bucketHostname.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/middleware-bucket-endpoint/src/bucketHostname.ts b/packages/middleware-bucket-endpoint/src/bucketHostname.ts index 3ca767ebad5e..546c459c2388 100644 --- a/packages/middleware-bucket-endpoint/src/bucketHostname.ts +++ b/packages/middleware-bucket-endpoint/src/bucketHostname.ts @@ -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 = ({ @@ -51,9 +56,7 @@ const getEndpointFromBucketName = ({ tlsCompatible = true, isCustomEndpoint = false, }: BucketHostnameParams): BucketHostname => { - // TODO: Remove checks for ".dualstack" from entire middleware. - const suffixHostname = dualstackEndpoint ? baseHostname.replace(".dualstack", "") : baseHostname; - const [clientRegion, hostnameSuffix] = isCustomEndpoint ? [region, baseHostname] : getSuffix(suffixHostname); + const [clientRegion, hostnameSuffix] = isCustomEndpoint ? [region, baseHostname] : getSuffix(baseHostname); if (pathStyleEndpoint || !isDnsCompatibleBucketName(bucketName) || (tlsCompatible && DOT_PATTERN.test(bucketName))) { return { bucketEndpoint: false, From ad25cece52bc5c2ecddb30558b53493bcd005904 Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 8 Nov 2021 19:29:13 +0000 Subject: [PATCH 4/4] test: accesspoint ARN with dualstack enabled --- .../src/bucketHostname.spec.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts b/packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts index 7cb10fe9a788..9b73384978ca 100644 --- a/packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts +++ b/packages/middleware-bucket-endpoint/src/bucketHostname.spec.ts @@ -188,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", () => { @@ -211,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` + ); }); });