From 6f5c153a3f7e9cef96dbfeaa1d9d181e1d27c888 Mon Sep 17 00:00:00 2001 From: siddsriv Date: Wed, 6 Sep 2023 17:50:54 +0000 Subject: [PATCH 1/8] feat(middleware-sdk-s3): add middleware for following region redirects --- packages/middleware-sdk-s3/src/index.ts | 1 + .../src/region-redirect-middleware.ts | 117 ++++++++++++++++++ .../middleware-sdk-s3/src/s3Configuration.ts | 9 +- 3 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 packages/middleware-sdk-s3/src/region-redirect-middleware.ts diff --git a/packages/middleware-sdk-s3/src/index.ts b/packages/middleware-sdk-s3/src/index.ts index 92c9f7291a67..02aeff027f0f 100644 --- a/packages/middleware-sdk-s3/src/index.ts +++ b/packages/middleware-sdk-s3/src/index.ts @@ -1,4 +1,5 @@ export * from "./check-content-length-header"; +export * from "./region-redirect-middleware"; export * from "./s3Configuration"; export * from "./throw-200-exceptions"; export * from "./validate-bucket-name"; diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts new file mode 100644 index 000000000000..ab56bc6d9899 --- /dev/null +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts @@ -0,0 +1,117 @@ +import { + HandlerExecutionContext, + InitializeHandler, + InitializeHandlerArguments, + InitializeHandlerOptions, + InitializeHandlerOutput, + InitializeMiddleware, + MetadataBearer, + Pluggable, + Provider, + RelativeMiddlewareOptions, + SerializeHandler, + SerializeHandlerArguments, + SerializeHandlerOutput, + SerializeMiddleware, +} from "@smithy/types"; + +/** + * @internal + */ +interface PreviouslyResolved { + region: Provider; + followRegionRedirects: boolean; +} + +/** + * @internal + */ +export function regionRedirectMiddleware(clientConfig: PreviouslyResolved): InitializeMiddleware { + return ( + next: InitializeHandler, + context: HandlerExecutionContext + ): InitializeHandler => + async (args: InitializeHandlerArguments): Promise> => { + try { + return next(args); + } catch (err) { + if ( + clientConfig.followRegionRedirects && + err.Code === "PermanentRedirect" && + err.$metadata.httpStatusCode === 301 + ) { + try { + const actualRegion = err.$response.headers["x-amz-bucket-region"]; + context.logger?.debug(`Redirecting from ${await clientConfig.region()} to ${actualRegion}`); + context.__s3RegionRedirect = actualRegion; + } catch (e) { + throw new Error("Region redirect failed: " + e); + } + return next(args); + } else { + throw err; + } + } + }; +} + +/** + * @internal + */ +export const regionRedirectEndpointMiddleware = (config: PreviouslyResolved): SerializeMiddleware => { + return ( + next: SerializeHandler, + context: HandlerExecutionContext + ): SerializeHandler => + async (args: SerializeHandlerArguments): Promise> => { + const originalRegion = await config.region(); + if (context.__s3RegionRedirect) { + const regionProviderRef = config.region; + config.region = async () => { + config.region = regionProviderRef; + return context.__s3RegionRedirect; + }; + } + const result = await next({ + ...args, + }); + if (context.__s3RegionRedirect) { + const region = await config.region(); + if (originalRegion !== region) { + throw new Error("Region was not restored following S3 region redirect."); + } + } + return result; + }; +}; + +/** + * @internal + */ +export const regionRedirectMiddlewareOptions: InitializeHandlerOptions = { + step: "initialize", + tags: ["REGION_REDIRECT", "S3"], + name: "regionRedirectMiddleware", + override: true, +}; + +/** + * @internal + */ +export const regionRedirectEndpointMiddlewareOptions: RelativeMiddlewareOptions = { + tags: ["REGION_REDIRECT", "S3"], + name: "regionRedirectEndpointMiddleware", + override: true, + relation: "before", + toMiddleware: "endpointV2Middleware", +}; + +/** + * @internal + */ +export const getRegionRedirectMiddlewarePlugin = (clientConfig: PreviouslyResolved): Pluggable => ({ + applyToStack: (clientStack) => { + clientStack.add(regionRedirectMiddleware(clientConfig), regionRedirectMiddlewareOptions); + clientStack.addRelativeTo(regionRedirectEndpointMiddleware(clientConfig), regionRedirectEndpointMiddlewareOptions); + }, +}); diff --git a/packages/middleware-sdk-s3/src/s3Configuration.ts b/packages/middleware-sdk-s3/src/s3Configuration.ts index ba91f3f16293..6aee45ceffd9 100644 --- a/packages/middleware-sdk-s3/src/s3Configuration.ts +++ b/packages/middleware-sdk-s3/src/s3Configuration.ts @@ -1,6 +1,6 @@ /** * @public - * + * * All endpoint parameters with built-in bindings of AWS::S3::* */ export interface S3InputConfig { @@ -17,12 +17,18 @@ export interface S3InputConfig { * Whether multi-region access points (MRAP) should be disabled. */ disableMultiregionAccessPoints?: boolean; + /** + * If you receive a permanent redirect with status 301, + * the client will retry your request with the corrected region. + */ + followRegionRedirects?: boolean; } export interface S3ResolvedConfig { forcePathStyle: boolean; useAccelerateEndpoint: boolean; disableMultiregionAccessPoints: boolean; + followRegionRedirects: boolean; } export const resolveS3Config = (input: T & S3InputConfig): T & S3ResolvedConfig => ({ @@ -30,4 +36,5 @@ export const resolveS3Config = (input: T & S3InputConfig): T & S3ResolvedConf forcePathStyle: input.forcePathStyle ?? false, useAccelerateEndpoint: input.useAccelerateEndpoint ?? false, disableMultiregionAccessPoints: input.disableMultiregionAccessPoints ?? false, + followRegionRedirects: input.followRegionRedirects ?? false, }); From a6073225d80cbe0623e3ecc67073905ee077f221 Mon Sep 17 00:00:00 2001 From: siddsriv Date: Wed, 13 Sep 2023 17:55:55 +0000 Subject: [PATCH 2/8] test(middleware-sdk-s3): add initial unit tests for region redirect middleware --- .../src/region-redirect-middleware.spec.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts new file mode 100644 index 000000000000..a08e9db28972 --- /dev/null +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts @@ -0,0 +1,32 @@ +import { HandlerExecutionContext } from "@smithy/types"; + +import { regionRedirectMiddleware } from "./region-redirect-middleware"; + +describe(regionRedirectMiddleware.name, () => { + const region = async () => "us-east-1"; + const redirectRegion = "us-west-2"; + let call = 0; + const next = (arg: any) => { + if (call === 0) { + call++; + throw Object.assign(new Error(), { + Code: "PermanentRedirect", + $metadata: { httpStatusCode: 301 }, + $response: { headers: { "x-amz-bucket-region": redirectRegion } }, + }); + } + return null as any; + }; + + beforeEach(() => { + call = 0; + }); + + it("set S3 region redirect on context if receiving a PermanentRedirect error code with status 301", async () => { + const middleware = regionRedirectMiddleware({ region, followRegionRedirects: true }); + const context = {} as HandlerExecutionContext; + const handler = middleware(next, context); + await handler({ input: null }); + expect(context.__s3RegionRedirect).toEqual(redirectRegion); + }); +}); From 9e907a6a537c3dec7698d751e151c103339b7735 Mon Sep 17 00:00:00 2001 From: siddsriv Date: Thu, 21 Sep 2023 17:50:18 +0000 Subject: [PATCH 3/8] test(middleware-sdk-s3): unit test addition and initial E2E test --- packages/middleware-sdk-s3/jest.config.e2e.js | 4 ++ packages/middleware-sdk-s3/package.json | 1 + .../region-redirect-middleware.e2e.spec.ts | 54 +++++++++++++++++++ .../src/region-redirect-middleware.spec.ts | 18 +++++++ 4 files changed, 77 insertions(+) create mode 100644 packages/middleware-sdk-s3/jest.config.e2e.js create mode 100644 packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts diff --git a/packages/middleware-sdk-s3/jest.config.e2e.js b/packages/middleware-sdk-s3/jest.config.e2e.js new file mode 100644 index 000000000000..b4d9bee23f48 --- /dev/null +++ b/packages/middleware-sdk-s3/jest.config.e2e.js @@ -0,0 +1,4 @@ +module.exports = { + preset: "ts-jest", + testMatch: ["**/*.e2e.spec.ts"], +}; diff --git a/packages/middleware-sdk-s3/package.json b/packages/middleware-sdk-s3/package.json index 4df6f1f61fab..60b8e15bc00c 100644 --- a/packages/middleware-sdk-s3/package.json +++ b/packages/middleware-sdk-s3/package.json @@ -11,6 +11,7 @@ "clean": "rimraf ./dist-* && rimraf *.tsbuildinfo", "test": "jest", "test:integration": "jest -c jest.config.integ.js", + "test:e2e": "jest -c jest.config.e2e.js", "extract:docs": "api-extractor run --local" }, "main": "./dist-cjs/index.js", diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts new file mode 100644 index 000000000000..65aabfd45496 --- /dev/null +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts @@ -0,0 +1,54 @@ +import { + CreateBucketCommand, + DeleteBucketCommand, + DeleteObjectCommand, + GetObjectCommand, + PutObjectCommand, + S3Client, +} from "@aws-sdk/client-s3"; +import { STS } from "@aws-sdk/client-sts"; + +const regionConfigs = [{ region: "us-east-1" }, { region: "us-west-2" }, { region: "eu-west-1" }]; + +const s3Clients = regionConfigs.map((config) => new S3Client(config)); + +const testValue = "Hello S3 global client!"; + +async function testS3GlobalClient() { + const stsClient = new STS({}); + + const callerID = await stsClient.getCallerIdentity({}); + + const bucketNames = regionConfigs.map((config) => `${callerID.Account}-Redirect-${config.region}`); + await Promise.all( + bucketNames.map((bucketName, index) => s3Clients[index].send(new CreateBucketCommand({ Bucket: bucketName }))) + ); + // Upload objects to each bucket + for (const bucketName of bucketNames) { + for (const s3Client of s3Clients) { + const objKey = `object-from-${await s3Client.config.region()}-client`; + await s3Client.send(new PutObjectCommand({ Bucket: bucketName, Key: objKey, Body: testValue })); + } + } + + // Fetch, assert, and delete objects + for (const bucketName of bucketNames) { + for (const s3Client of s3Clients) { + const objKey = `object-from-${await s3Client.config.region()}-client`; + const { Body } = await s3Client.send(new GetObjectCommand({ Bucket: bucketName, Key: objKey })); + const data = await Body?.transformToString(); + expect(data).toEqual(testValue); + await s3Client.send(new DeleteObjectCommand({ Bucket: bucketName, Key: objKey })); + } + } + + for (let i = 0; i < s3Clients.length; ++i) { + s3Clients[i].send(new DeleteBucketCommand({ Bucket: bucketNames[(i + 1) % bucketNames.length] })); + } +} + +describe("S3 Global Client Test", () => { + it("Can perform all operations cross-regionally by following region redirect", async () => { + await testS3GlobalClient(); + }); +}); diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts index a08e9db28972..6f7f4e1c2d55 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts @@ -29,4 +29,22 @@ describe(regionRedirectMiddleware.name, () => { await handler({ input: null }); expect(context.__s3RegionRedirect).toEqual(redirectRegion); }); + + it("does not follow the redirect when followRegionRedirects is false", async () => { + const middleware = regionRedirectMiddleware({ region, followRegionRedirects: false }); + const context = {} as HandlerExecutionContext; + const handler = middleware(next, context); + // Simulating a PermanentRedirect error with status 301 + await expect(async () => { + await handler({ input: null }); + }).rejects.toThrowError( + Object.assign(new Error(), { + Code: "PermanentRedirect", + $metadata: { httpStatusCode: 301 }, + $response: { headers: { "x-amz-bucket-region": redirectRegion } }, + }) + ); + // Ensure that context.__s3RegionRedirect is not set + expect(context.__s3RegionRedirect).toBeUndefined(); + }); }); From a157deaa9beadaa37878d07c630a84c5623a0756 Mon Sep 17 00:00:00 2001 From: siddsriv Date: Thu, 21 Sep 2023 19:42:52 +0000 Subject: [PATCH 4/8] test(middleware-sdk-s3): increase timeout for E2E test --- .../src/region-redirect-middleware.e2e.spec.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts index 65aabfd45496..de9e2fe2fc0b 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts @@ -8,7 +8,11 @@ import { } from "@aws-sdk/client-s3"; import { STS } from "@aws-sdk/client-sts"; -const regionConfigs = [{ region: "us-east-1" }, { region: "us-west-2" }, { region: "eu-west-1" }]; +const regionConfigs = [ + { region: "us-east-1", followRegionRedirects: true }, + { region: "us-west-2", followRegionRedirects: true }, + { region: "us-west-1", followRegionRedirects: true }, +]; const s3Clients = regionConfigs.map((config) => new S3Client(config)); @@ -19,7 +23,7 @@ async function testS3GlobalClient() { const callerID = await stsClient.getCallerIdentity({}); - const bucketNames = regionConfigs.map((config) => `${callerID.Account}-Redirect-${config.region}`); + const bucketNames = regionConfigs.map((config) => `${callerID.Account}-redirect-testing-${config.region}`); await Promise.all( bucketNames.map((bucketName, index) => s3Clients[index].send(new CreateBucketCommand({ Bucket: bucketName }))) ); @@ -50,5 +54,5 @@ async function testS3GlobalClient() { describe("S3 Global Client Test", () => { it("Can perform all operations cross-regionally by following region redirect", async () => { await testS3GlobalClient(); - }); + }, 50000); }); From e133bb7a92a780d099a1f1a7c86e1806cd842ae2 Mon Sep 17 00:00:00 2001 From: siddsriv Date: Mon, 25 Sep 2023 16:45:02 +0000 Subject: [PATCH 5/8] chore(middleware-sdk-s3): adding an await and nit fix --- .../src/region-redirect-middleware.e2e.spec.ts | 2 +- .../middleware-sdk-s3/src/region-redirect-middleware.ts | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts index de9e2fe2fc0b..3f2bb42bb55f 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts @@ -47,7 +47,7 @@ async function testS3GlobalClient() { } for (let i = 0; i < s3Clients.length; ++i) { - s3Clients[i].send(new DeleteBucketCommand({ Bucket: bucketNames[(i + 1) % bucketNames.length] })); + await s3Clients[i].send(new DeleteBucketCommand({ Bucket: bucketNames[(i + 1) % bucketNames.length] })); } } diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts index ab56bc6d9899..e7235be2c237 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts @@ -37,7 +37,7 @@ export function regionRedirectMiddleware(clientConfig: PreviouslyResolved): Init } catch (err) { if ( clientConfig.followRegionRedirects && - err.Code === "PermanentRedirect" && + err.name === "PermanentRedirect" && err.$metadata.httpStatusCode === 301 ) { try { @@ -72,9 +72,7 @@ export const regionRedirectEndpointMiddleware = (config: PreviouslyResolved): Se return context.__s3RegionRedirect; }; } - const result = await next({ - ...args, - }); + const result = await next(args); if (context.__s3RegionRedirect) { const region = await config.region(); if (originalRegion !== region) { From c66cd1f9e864ca81a419137eaee863140cbbc1ca Mon Sep 17 00:00:00 2001 From: siddsriv Date: Mon, 25 Sep 2023 19:34:18 +0000 Subject: [PATCH 6/8] chore(middleware-sdk-s3): split region redirect middlewares in different files --- packages/middleware-sdk-s3/src/index.ts | 1 + .../region-redirect-endpoint-middleware.ts | 58 +++++++++++++++++++ .../region-redirect-middleware.e2e.spec.ts | 4 +- .../src/region-redirect-middleware.spec.ts | 2 +- .../src/region-redirect-middleware.ts | 47 +-------------- 5 files changed, 63 insertions(+), 49 deletions(-) create mode 100644 packages/middleware-sdk-s3/src/region-redirect-endpoint-middleware.ts diff --git a/packages/middleware-sdk-s3/src/index.ts b/packages/middleware-sdk-s3/src/index.ts index 02aeff027f0f..a23a0aadceba 100644 --- a/packages/middleware-sdk-s3/src/index.ts +++ b/packages/middleware-sdk-s3/src/index.ts @@ -1,4 +1,5 @@ export * from "./check-content-length-header"; +export * from "./region-redirect-endpoint-middleware"; export * from "./region-redirect-middleware"; export * from "./s3Configuration"; export * from "./throw-200-exceptions"; diff --git a/packages/middleware-sdk-s3/src/region-redirect-endpoint-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-endpoint-middleware.ts new file mode 100644 index 000000000000..902f4b37e3fd --- /dev/null +++ b/packages/middleware-sdk-s3/src/region-redirect-endpoint-middleware.ts @@ -0,0 +1,58 @@ +import { + HandlerExecutionContext, + MetadataBearer, + Pluggable, + RelativeMiddlewareOptions, + SerializeHandler, + SerializeHandlerArguments, + SerializeHandlerOutput, + SerializeMiddleware, +} from "@smithy/types"; + +import { PreviouslyResolved } from "./region-redirect-middleware"; + +/** + * @internal + */ +export const regionRedirectEndpointMiddleware = (config: PreviouslyResolved): SerializeMiddleware => { + return ( + next: SerializeHandler, + context: HandlerExecutionContext + ): SerializeHandler => + async (args: SerializeHandlerArguments): Promise> => { + const originalRegion = await config.region(); + if (context.__s3RegionRedirect) { + config.region = async () => { + return context.__s3RegionRedirect; + }; + } + const result = await next(args); + if (context.__s3RegionRedirect) { + const region = await config.region(); + if (originalRegion !== region) { + throw new Error("Region was not restored following S3 region redirect."); + } + } + return result; + }; +}; + +/** + * @internal + */ +export const regionRedirectEndpointMiddlewareOptions: RelativeMiddlewareOptions = { + tags: ["REGION_REDIRECT", "S3"], + name: "regionRedirectEndpointMiddleware", + override: true, + relation: "before", + toMiddleware: "endpointV2Middleware", +}; + +/** + * @internal + */ +export const getRegionRedirectEndpointMiddlewarePlugin = (clientConfig: PreviouslyResolved): Pluggable => ({ + applyToStack: (clientStack) => { + clientStack.addRelativeTo(regionRedirectEndpointMiddleware(clientConfig), regionRedirectEndpointMiddlewareOptions); + }, +}); diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts index 3f2bb42bb55f..de23996d17ff 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts @@ -10,8 +10,8 @@ import { STS } from "@aws-sdk/client-sts"; const regionConfigs = [ { region: "us-east-1", followRegionRedirects: true }, + { region: "eu-west-1", followRegionRedirects: true }, { region: "us-west-2", followRegionRedirects: true }, - { region: "us-west-1", followRegionRedirects: true }, ]; const s3Clients = regionConfigs.map((config) => new S3Client(config)); @@ -23,7 +23,7 @@ async function testS3GlobalClient() { const callerID = await stsClient.getCallerIdentity({}); - const bucketNames = regionConfigs.map((config) => `${callerID.Account}-redirect-testing-${config.region}`); + const bucketNames = regionConfigs.map((config) => `${callerID.Account}-redirect-${config.region}`); await Promise.all( bucketNames.map((bucketName, index) => s3Clients[index].send(new CreateBucketCommand({ Bucket: bucketName }))) ); diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts index 6f7f4e1c2d55..b159333623ca 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts @@ -10,7 +10,7 @@ describe(regionRedirectMiddleware.name, () => { if (call === 0) { call++; throw Object.assign(new Error(), { - Code: "PermanentRedirect", + name: "PermanentRedirect", $metadata: { httpStatusCode: 301 }, $response: { headers: { "x-amz-bucket-region": redirectRegion } }, }); diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts index e7235be2c237..408b0f1c119e 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts @@ -8,17 +8,12 @@ import { MetadataBearer, Pluggable, Provider, - RelativeMiddlewareOptions, - SerializeHandler, - SerializeHandlerArguments, - SerializeHandlerOutput, - SerializeMiddleware, } from "@smithy/types"; /** * @internal */ -interface PreviouslyResolved { +export interface PreviouslyResolved { region: Provider; followRegionRedirects: boolean; } @@ -55,34 +50,6 @@ export function regionRedirectMiddleware(clientConfig: PreviouslyResolved): Init }; } -/** - * @internal - */ -export const regionRedirectEndpointMiddleware = (config: PreviouslyResolved): SerializeMiddleware => { - return ( - next: SerializeHandler, - context: HandlerExecutionContext - ): SerializeHandler => - async (args: SerializeHandlerArguments): Promise> => { - const originalRegion = await config.region(); - if (context.__s3RegionRedirect) { - const regionProviderRef = config.region; - config.region = async () => { - config.region = regionProviderRef; - return context.__s3RegionRedirect; - }; - } - const result = await next(args); - if (context.__s3RegionRedirect) { - const region = await config.region(); - if (originalRegion !== region) { - throw new Error("Region was not restored following S3 region redirect."); - } - } - return result; - }; -}; - /** * @internal */ @@ -93,23 +60,11 @@ export const regionRedirectMiddlewareOptions: InitializeHandlerOptions = { override: true, }; -/** - * @internal - */ -export const regionRedirectEndpointMiddlewareOptions: RelativeMiddlewareOptions = { - tags: ["REGION_REDIRECT", "S3"], - name: "regionRedirectEndpointMiddleware", - override: true, - relation: "before", - toMiddleware: "endpointV2Middleware", -}; - /** * @internal */ export const getRegionRedirectMiddlewarePlugin = (clientConfig: PreviouslyResolved): Pluggable => ({ applyToStack: (clientStack) => { clientStack.add(regionRedirectMiddleware(clientConfig), regionRedirectMiddlewareOptions); - clientStack.addRelativeTo(regionRedirectEndpointMiddleware(clientConfig), regionRedirectEndpointMiddlewareOptions); }, }); From 9eaefc247b4659f91fc9b416beef8794c8596b0c Mon Sep 17 00:00:00 2001 From: siddsriv Date: Wed, 27 Sep 2023 18:08:36 +0000 Subject: [PATCH 7/8] fix(middleware-sdk-s3): bug fix for middleware and test refactor --- clients/client-s3/src/S3Client.ts | 2 + .../aws/typescript/codegen/AddS3Config.java | 5 + .../region-redirect-endpoint-middleware.ts | 12 +- .../region-redirect-middleware.e2e.spec.ts | 140 ++++++++++++------ .../src/region-redirect-middleware.ts | 9 +- 5 files changed, 111 insertions(+), 57 deletions(-) diff --git a/clients/client-s3/src/S3Client.ts b/clients/client-s3/src/S3Client.ts index 57d8b7200ed1..6f72b42688f8 100644 --- a/clients/client-s3/src/S3Client.ts +++ b/clients/client-s3/src/S3Client.ts @@ -9,6 +9,7 @@ import { import { getLoggerPlugin } from "@aws-sdk/middleware-logger"; import { getRecursionDetectionPlugin } from "@aws-sdk/middleware-recursion-detection"; import { + getRegionRedirectMiddlewarePlugin, getValidateBucketNamePlugin, resolveS3Config, S3InputConfig, @@ -778,6 +779,7 @@ export class S3Client extends __Client< this.middlewareStack.use(getAwsAuthPlugin(this.config)); this.middlewareStack.use(getValidateBucketNamePlugin(this.config)); this.middlewareStack.use(getAddExpectContinuePlugin(this.config)); + this.middlewareStack.use(getRegionRedirectMiddlewarePlugin(this.config)); this.middlewareStack.use(getUserAgentPlugin(this.config)); } diff --git a/codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3Config.java b/codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3Config.java index 4d86227d61c6..39fa5aea7299 100644 --- a/codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3Config.java +++ b/codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3Config.java @@ -226,6 +226,11 @@ && isS3(s)) && isS3(s) && !isEndpointsV2Service(s) && containsInputMembers(m, o, BUCKET_ENDPOINT_INPUT_KEYS)) + .build(), + RuntimeClientPlugin.builder() + .withConventions(AwsDependency.S3_MIDDLEWARE.dependency, "RegionRedirectMiddleware", + HAS_MIDDLEWARE) + .servicePredicate((m, s) -> isS3(s)) .build() ); } diff --git a/packages/middleware-sdk-s3/src/region-redirect-endpoint-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-endpoint-middleware.ts index 902f4b37e3fd..ece8a964ace1 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-endpoint-middleware.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-endpoint-middleware.ts @@ -1,7 +1,6 @@ import { HandlerExecutionContext, MetadataBearer, - Pluggable, RelativeMiddlewareOptions, SerializeHandler, SerializeHandlerArguments, @@ -21,8 +20,10 @@ export const regionRedirectEndpointMiddleware = (config: PreviouslyResolved): Se ): SerializeHandler => async (args: SerializeHandlerArguments): Promise> => { const originalRegion = await config.region(); + const regionProviderRef = config.region; if (context.__s3RegionRedirect) { config.region = async () => { + config.region = regionProviderRef; return context.__s3RegionRedirect; }; } @@ -47,12 +48,3 @@ export const regionRedirectEndpointMiddlewareOptions: RelativeMiddlewareOptions relation: "before", toMiddleware: "endpointV2Middleware", }; - -/** - * @internal - */ -export const getRegionRedirectEndpointMiddlewarePlugin = (clientConfig: PreviouslyResolved): Pluggable => ({ - applyToStack: (clientStack) => { - clientStack.addRelativeTo(regionRedirectEndpointMiddleware(clientConfig), regionRedirectEndpointMiddlewareOptions); - }, -}); diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts index de23996d17ff..7d68644570d0 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts @@ -1,58 +1,106 @@ -import { - CreateBucketCommand, - DeleteBucketCommand, - DeleteObjectCommand, - GetObjectCommand, - PutObjectCommand, - S3Client, -} from "@aws-sdk/client-s3"; -import { STS } from "@aws-sdk/client-sts"; - -const regionConfigs = [ - { region: "us-east-1", followRegionRedirects: true }, - { region: "eu-west-1", followRegionRedirects: true }, - { region: "us-west-2", followRegionRedirects: true }, -]; - -const s3Clients = regionConfigs.map((config) => new S3Client(config)); +import { S3 } from "@aws-sdk/client-s3"; +import { GetCallerIdentityCommandOutput, STS } from "@aws-sdk/client-sts"; const testValue = "Hello S3 global client!"; -async function testS3GlobalClient() { +describe("S3 Global Client Test", () => { + const regionConfigs = [ + { region: "us-east-1", followRegionRedirects: true }, + { region: "eu-west-1", followRegionRedirects: true }, + { region: "us-west-2", followRegionRedirects: true }, + ]; + const s3Clients = regionConfigs.map((config) => new S3(config)); const stsClient = new STS({}); - const callerID = await stsClient.getCallerIdentity({}); - - const bucketNames = regionConfigs.map((config) => `${callerID.Account}-redirect-${config.region}`); - await Promise.all( - bucketNames.map((bucketName, index) => s3Clients[index].send(new CreateBucketCommand({ Bucket: bucketName }))) - ); - // Upload objects to each bucket - for (const bucketName of bucketNames) { - for (const s3Client of s3Clients) { - const objKey = `object-from-${await s3Client.config.region()}-client`; - await s3Client.send(new PutObjectCommand({ Bucket: bucketName, Key: objKey, Body: testValue })); + let callerID = null as unknown as GetCallerIdentityCommandOutput; + let bucketNames = [] as string[]; + + beforeAll(async () => { + jest.setTimeout(500000); + callerID = await stsClient.getCallerIdentity({}); + bucketNames = regionConfigs.map((config) => `${callerID.Account}-redirect-${config.region}`); + await Promise.all(bucketNames.map((bucketName, index) => deleteBucket(s3Clients[index], bucketName))); + await Promise.all(bucketNames.map((bucketName, index) => s3Clients[index].createBucket({ Bucket: bucketName }))); + }); + + afterAll(async () => { + await Promise.all(bucketNames.map((bucketName, index) => deleteBucket(s3Clients[index], bucketName))); + }); + + it("Should be able to put objects following region redirect", async () => { + // Upload objects to each bucket + for (const bucketName of bucketNames) { + for (const s3Client of s3Clients) { + const objKey = `object-from-${await s3Client.config.region()}-client`; + await s3Client.putObject({ Bucket: bucketName, Key: objKey, Body: testValue }); + } } - } + }, 50000); - // Fetch, assert, and delete objects - for (const bucketName of bucketNames) { - for (const s3Client of s3Clients) { - const objKey = `object-from-${await s3Client.config.region()}-client`; - const { Body } = await s3Client.send(new GetObjectCommand({ Bucket: bucketName, Key: objKey })); - const data = await Body?.transformToString(); - expect(data).toEqual(testValue); - await s3Client.send(new DeleteObjectCommand({ Bucket: bucketName, Key: objKey })); + it("Should be able to get objects following region redirect", async () => { + // Fetch and assert objects + for (const bucketName of bucketNames) { + for (const s3Client of s3Clients) { + const objKey = `object-from-${await s3Client.config.region()}-client`; + const { Body } = await s3Client.getObject({ Bucket: bucketName, Key: objKey }); + const data = await Body?.transformToString(); + expect(data).toEqual(testValue); + } } + }, 50000); + + it("Should delete objects following region redirect", async () => { + for (const bucketName of bucketNames) { + for (const s3Client of s3Clients) { + const objKey = `object-from-${await s3Client.config.region()}-client`; + await s3Client.deleteObject({ Bucket: bucketName, Key: objKey }); + } + } + }, 50000); +}); + +async function deleteBucket(s3: S3, bucketName: string) { + const Bucket = bucketName; + + try { + await s3.headBucket({ + Bucket, + }); + } catch (e) { + return; } - for (let i = 0; i < s3Clients.length; ++i) { - await s3Clients[i].send(new DeleteBucketCommand({ Bucket: bucketNames[(i + 1) % bucketNames.length] })); + const list = await s3 + .listObjects({ + Bucket, + }) + .catch((e) => { + if (!String(e).includes("NoSuchBucket")) { + throw e; + } + return { + Contents: [], + }; + }); + + const promises = [] as any[]; + for (const key of list.Contents ?? []) { + promises.push( + s3.deleteObject({ + Bucket, + Key: key.Key, + }) + ); } -} + await Promise.all(promises); -describe("S3 Global Client Test", () => { - it("Can perform all operations cross-regionally by following region redirect", async () => { - await testS3GlobalClient(); - }, 50000); -}); + try { + return await s3.deleteBucket({ + Bucket, + }); + } catch (e) { + if (!String(e).includes("NoSuchBucket")) { + throw e; + } + } +} diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts index 408b0f1c119e..f16bfd7465ff 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts @@ -10,6 +10,11 @@ import { Provider, } from "@smithy/types"; +import { + regionRedirectEndpointMiddleware, + regionRedirectEndpointMiddlewareOptions, +} from "./region-redirect-endpoint-middleware"; + /** * @internal */ @@ -28,8 +33,9 @@ export function regionRedirectMiddleware(clientConfig: PreviouslyResolved): Init ): InitializeHandler => async (args: InitializeHandlerArguments): Promise> => { try { - return next(args); + return await next(args); } catch (err) { + // console.log("Region Redirect", clientConfig.followRegionRedirects, err.name, err.$metadata.httpStatusCode); if ( clientConfig.followRegionRedirects && err.name === "PermanentRedirect" && @@ -66,5 +72,6 @@ export const regionRedirectMiddlewareOptions: InitializeHandlerOptions = { export const getRegionRedirectMiddlewarePlugin = (clientConfig: PreviouslyResolved): Pluggable => ({ applyToStack: (clientStack) => { clientStack.add(regionRedirectMiddleware(clientConfig), regionRedirectMiddlewareOptions); + clientStack.addRelativeTo(regionRedirectEndpointMiddleware(clientConfig), regionRedirectEndpointMiddlewareOptions); }, }); From a36ca30fef5a085cb0d9a1f319974b33abfa02c8 Mon Sep 17 00:00:00 2001 From: siddsriv Date: Tue, 3 Oct 2023 19:19:30 +0000 Subject: [PATCH 8/8] chore(middleware-sdk-s3): doc update --- packages/middleware-sdk-s3/src/s3Configuration.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/middleware-sdk-s3/src/s3Configuration.ts b/packages/middleware-sdk-s3/src/s3Configuration.ts index 6aee45ceffd9..33993c23aafa 100644 --- a/packages/middleware-sdk-s3/src/s3Configuration.ts +++ b/packages/middleware-sdk-s3/src/s3Configuration.ts @@ -18,8 +18,10 @@ export interface S3InputConfig { */ disableMultiregionAccessPoints?: boolean; /** - * If you receive a permanent redirect with status 301, - * the client will retry your request with the corrected region. + * This feature was previously called the S3 Global Client. + * This can result in additional latency as failed requests are retried + * with a corrected region when receiving a permanent redirect error with status 301. + * This feature should only be used as a last resort if you do not know the region of your bucket(s) ahead of time. */ followRegionRedirects?: boolean; }