Skip to content

Commit

Permalink
fix(middleware-sdk-s3): bug fix for middleware and test refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
siddsriv committed Sep 27, 2023
1 parent c66cd1f commit 9eaefc2
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 57 deletions.
2 changes: 2 additions & 0 deletions clients/client-s3/src/S3Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
HandlerExecutionContext,
MetadataBearer,
Pluggable,
RelativeMiddlewareOptions,
SerializeHandler,
SerializeHandlerArguments,
Expand All @@ -21,8 +20,10 @@ export const regionRedirectEndpointMiddleware = (config: PreviouslyResolved): Se
): SerializeHandler<any, Output> =>
async (args: SerializeHandlerArguments<any>): Promise<SerializeHandlerOutput<Output>> => {
const originalRegion = await config.region();
const regionProviderRef = config.region;
if (context.__s3RegionRedirect) {
config.region = async () => {
config.region = regionProviderRef;
return context.__s3RegionRedirect;
};
}
Expand All @@ -47,12 +48,3 @@ export const regionRedirectEndpointMiddlewareOptions: RelativeMiddlewareOptions
relation: "before",
toMiddleware: "endpointV2Middleware",
};

/**
* @internal
*/
export const getRegionRedirectEndpointMiddlewarePlugin = (clientConfig: PreviouslyResolved): Pluggable<any, any> => ({
applyToStack: (clientStack) => {
clientStack.addRelativeTo(regionRedirectEndpointMiddleware(clientConfig), regionRedirectEndpointMiddlewareOptions);
},
});
140 changes: 94 additions & 46 deletions packages/middleware-sdk-s3/src/region-redirect-middleware.e2e.spec.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
9 changes: 8 additions & 1 deletion packages/middleware-sdk-s3/src/region-redirect-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import {
Provider,
} from "@smithy/types";

import {
regionRedirectEndpointMiddleware,
regionRedirectEndpointMiddlewareOptions,
} from "./region-redirect-endpoint-middleware";

/**
* @internal
*/
Expand All @@ -28,8 +33,9 @@ export function regionRedirectMiddleware(clientConfig: PreviouslyResolved): Init
): InitializeHandler<any, Output> =>
async (args: InitializeHandlerArguments<any>): Promise<InitializeHandlerOutput<Output>> => {
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" &&
Expand Down Expand Up @@ -66,5 +72,6 @@ export const regionRedirectMiddlewareOptions: InitializeHandlerOptions = {
export const getRegionRedirectMiddlewarePlugin = (clientConfig: PreviouslyResolved): Pluggable<any, any> => ({
applyToStack: (clientStack) => {
clientStack.add(regionRedirectMiddleware(clientConfig), regionRedirectMiddlewareOptions);
clientStack.addRelativeTo(regionRedirectEndpointMiddleware(clientConfig), regionRedirectEndpointMiddlewareOptions);
},
});

0 comments on commit 9eaefc2

Please sign in to comment.