Skip to content

Commit 6c590fc

Browse files
smilkurikuhe
andauthored
fix(client-s3-control): remove host prefix behavior (#7025)
* fix(s3ControlClient): removing host prefix * fix(client-s3-control): codegen remove host prefix behavior * fix(middleware-sdk-s3-control): deprecate superseded middleware * test: add ep2 tests to integ suite --------- Co-authored-by: George Fu <[email protected]>
1 parent d854030 commit 6c590fc

File tree

10 files changed

+136
-1181
lines changed

10 files changed

+136
-1181
lines changed

Makefile

+6-2
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,12 @@ test-integration: build-s3-browser-bundle
2929
rm -rf ./clients/client-sso/node_modules/\@smithy # todo(yarn) incompatible redundant nesting.
3030
yarn g:vitest run -c vitest.config.integ.ts
3131
npx jest -c jest.config.integ.js
32-
make test-protocols;
33-
make test-types;
32+
make test-protocols
33+
make test-types
34+
make test-endpoints
35+
36+
test-endpoints:
37+
npx jest -c ./tests/endpoints-2.0/jest.config.js --bail
3438

3539
test-e2e: build-s3-browser-bundle
3640
yarn g:vitest run -c vitest.config.e2e.ts --retry=4

clients/client-s3-control/src/models/models_1.ts

-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import {
1919
StorageLensTag,
2020
Tag,
2121
} from "./models_0";
22-
2322
import { S3ControlServiceException as __BaseException } from "./S3ControlServiceException";
2423

2524
/**

clients/client-s3-control/src/protocols/Aws_restXml.ts

+1-1,109
Large diffs are not rendered by default.

codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3Config.java

+24-1
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,12 @@
4343
import software.amazon.smithy.model.shapes.TimestampShape;
4444
import software.amazon.smithy.model.traits.DeprecatedTrait;
4545
import software.amazon.smithy.model.traits.DocumentationTrait;
46+
import software.amazon.smithy.model.traits.EndpointTrait;
4647
import software.amazon.smithy.model.traits.HttpHeaderTrait;
4748
import software.amazon.smithy.model.traits.HttpPayloadTrait;
4849
import software.amazon.smithy.model.traits.StreamingTrait;
50+
import software.amazon.smithy.model.transform.ModelTransformer;
51+
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
4952
import software.amazon.smithy.typescript.codegen.LanguageTarget;
5053
import software.amazon.smithy.typescript.codegen.TypeScriptDependency;
5154
import software.amazon.smithy.typescript.codegen.TypeScriptSettings;
@@ -79,6 +82,19 @@ public final class AddS3Config implements TypeScriptIntegration {
7982
+ " you need to install the \"@aws-sdk/signature-v4-crt\" package to your project dependencies. \n"
8083
+ "For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>";
8184

85+
/**
86+
* Remove the hostPrefix functionality by removing the endpoint traits.
87+
* Only applied if endpointRuleSet trait present on S3/S3Control services.
88+
*/
89+
public static Shape removeHostPrefixTrait(Shape shape) {
90+
return shape.asOperationShape()
91+
.map(OperationShape::shapeToBuilder)
92+
.map(builder -> ((OperationShape.Builder) builder).removeTrait(EndpointTrait.ID))
93+
.map(OperationShape.Builder::build)
94+
.map(s -> (Shape) s)
95+
.orElse(shape);
96+
}
97+
8298
@Override
8399
public List<String> runAfter() {
84100
return List.of(
@@ -96,6 +112,7 @@ public Model preprocessModel(Model model, TypeScriptSettings settings) {
96112
}
97113

98114
Model.Builder modelBuilder = model.toBuilder();
115+
boolean hasRuleset = !model.getServiceShapesWithTrait(EndpointRuleSetTrait.class).isEmpty();
99116

100117
TopDownIndex topDownIndex = TopDownIndex.of(model);
101118
Set<StructureShape> inputShapes = new HashSet<>();
@@ -206,7 +223,13 @@ public Model preprocessModel(Model model, TypeScriptSettings settings) {
206223
}
207224
}
208225

209-
return modelBuilder.addShapes(inputShapes).build();
226+
Model builtModel = modelBuilder.addShapes(inputShapes).build();
227+
if (hasRuleset) {
228+
ModelTransformer.create().mapShapes(
229+
builtModel, AddS3Config::removeHostPrefixTrait
230+
);
231+
}
232+
return builtModel;
210233
}
211234

212235
@Override

codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3ControlDependency.java

+12-6
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.Map;
2424
import java.util.Optional;
2525
import java.util.function.Consumer;
26+
import java.util.function.Function;
2627
import software.amazon.smithy.aws.traits.ServiceTrait;
2728
import software.amazon.smithy.codegen.core.SymbolProvider;
2829
import software.amazon.smithy.model.Model;
@@ -32,6 +33,7 @@
3233
import software.amazon.smithy.model.shapes.Shape;
3334
import software.amazon.smithy.model.traits.RequiredTrait;
3435
import software.amazon.smithy.model.transform.ModelTransformer;
36+
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
3537
import software.amazon.smithy.typescript.codegen.LanguageTarget;
3638
import software.amazon.smithy.typescript.codegen.TypeScriptSettings;
3739
import software.amazon.smithy.typescript.codegen.TypeScriptWriter;
@@ -90,15 +92,19 @@ public Model preprocessModel(Model model, TypeScriptSettings settings) {
9092
if (!isS3Control(settings.getService(model))) {
9193
return model;
9294
}
95+
boolean hasRuleset = !model.getServiceShapesWithTrait(EndpointRuleSetTrait.class).isEmpty();
9396
ServiceShape serviceShape = model.expectShape(settings.getService(), ServiceShape.class);
94-
return ModelTransformer.create().mapShapes(model, shape -> {
97+
Function<Shape, Shape> removeRequired = shape -> {
9598
Optional<MemberShape> modified = shape.asMemberShape()
96-
.filter(memberShape -> memberShape.getTarget().getName(serviceShape).equals("AccountId"))
97-
.filter(memberShape -> model.expectShape(memberShape.getTarget()).isStringShape())
98-
.filter(memberShape -> memberShape.isRequired())
99-
.map(memberShape -> Shape.shapeToBuilder(memberShape).removeTrait(RequiredTrait.ID).build());
99+
.filter(memberShape -> memberShape.getTarget().getName(serviceShape).equals("AccountId"))
100+
.filter(memberShape -> model.expectShape(memberShape.getTarget()).isStringShape())
101+
.filter(MemberShape::isRequired)
102+
.map(memberShape -> Shape.shapeToBuilder(memberShape).removeTrait(RequiredTrait.ID).build());
100103
return modified.isPresent() ? modified.get() : shape;
101-
});
104+
};
105+
return ModelTransformer.create().mapShapes(
106+
model, hasRuleset ? removeRequired.andThen(AddS3Config::removeHostPrefixTrait) : removeRequired
107+
);
102108
}
103109

104110
@Override

packages/middleware-sdk-s3-control/src/host-prefix-deduplication/deduplicateHostPrefix.spec.ts

-26
This file was deleted.

packages/middleware-sdk-s3-control/src/host-prefix-deduplication/deduplicateHostPrefix.ts

-16
This file was deleted.

packages/middleware-sdk-s3-control/src/host-prefix-deduplication/hostPrefixDeduplicationMiddleware.ts

+5-12
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import {
22
HandlerExecutionContext,
3-
HttpRequest,
43
Pluggable,
54
RelativeMiddlewareOptions,
65
SerializeHandler,
@@ -9,28 +8,21 @@ import {
98
SerializeMiddleware,
109
} from "@smithy/types";
1110

12-
import { deduplicateHostPrefix } from "./deduplicateHostPrefix";
13-
1411
/**
1512
* @internal
16-
* This customization handles an edge case where
17-
* a hostprefix may be duplicated in the endpoint ruleset resolution
18-
* and hostPrefix serialization via the pre-endpoints 2.0 trait,
19-
* and which cannot be reconciled automatically.
13+
* @deprecated - the middleware is no longer necessary since hostPrefix was
14+
* removed by S3Control codegen customization's model preprocessing.
2015
*/
2116
export const hostPrefixDeduplicationMiddleware = (): SerializeMiddleware<any, any> => {
2217
return (next: SerializeHandler<any, any>, context: HandlerExecutionContext): SerializeHandler<any, any> =>
23-
async (args: SerializeHandlerArguments<any>): Promise<SerializeHandlerOutput<any>> => {
24-
const httpRequest: HttpRequest = (args.request ?? {}) as HttpRequest;
25-
if (httpRequest?.hostname) {
26-
httpRequest.hostname = deduplicateHostPrefix(httpRequest.hostname);
27-
}
18+
(args: SerializeHandlerArguments<any>): Promise<SerializeHandlerOutput<any>> => {
2819
return next(args);
2920
};
3021
};
3122

3223
/**
3324
* @internal
25+
* @deprecated
3426
*/
3527
export const hostPrefixDeduplicationMiddlewareOptions: RelativeMiddlewareOptions = {
3628
tags: ["HOST_PREFIX_DEDUPLICATION", "ENDPOINT_V2", "ENDPOINT"],
@@ -42,6 +34,7 @@ export const hostPrefixDeduplicationMiddlewareOptions: RelativeMiddlewareOptions
4234

4335
/**
4436
* @internal
37+
* @deprecated
4538
*/
4639
export const getHostPrefixDeduplicationPlugin = <T>(config: T): Pluggable<any, any> => ({
4740
applyToStack: (clientStack) => {

packages/middleware-sdk-s3-control/src/middleware-sdk-s3-control.integ.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ describe("middleware-sdk-s3-control", () => {
7878

7979
requireRequestsFrom(client).toMatch({
8080
method: "GET",
81-
hostname: "123456789012.s3-outposts.us-west-2.amazonaws.com",
81+
hostname: "s3-outposts.us-west-2.amazonaws.com",
8282
headers: {
83-
host: "123456789012.s3-outposts.us-west-2.amazonaws.com",
83+
host: "s3-outposts.us-west-2.amazonaws.com",
8484
},
8585
protocol: "https:",
8686
path: "/v20180820/bucket",

tests/endpoints-2.0/endpoints-integration.spec.ts

+86-6
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { resolveParams } from "@smithy/middleware-endpoint";
2-
import { EndpointV2 } from "@smithy/types";
3-
import { resolveEndpoint, EndpointParams } from "@smithy/util-endpoints";
2+
import { EndpointV2, RelativeMiddlewareOptions } from "@smithy/types";
3+
import { EndpointParams, resolveEndpoint } from "@smithy/util-endpoints";
44
import { existsSync, readdirSync } from "fs";
55
import { join } from "path";
66

77
import { EndpointExpectation, ServiceModel, ServiceNamespace } from "./integration-test-types";
8+
import { HttpRequest } from "@smithy/protocol-http";
89

910
describe("client list", () => {
1011
const root = join(__dirname, "..", "..");
11-
const clientPackageNameList = readdirSync(join(root, "clients"));
12+
const clientPackageNameList = readdirSync(join(root, "clients")).filter((f) => f.startsWith("client-"));
1213

1314
it("should be at least 300 clients", () => {
1415
expect(clientPackageNameList.length).toBeGreaterThan(300);
@@ -37,6 +38,7 @@ describe("client list", () => {
3738
function runTestCases(service: ServiceModel, namespace: ServiceNamespace) {
3839
const serviceId = service.traits["aws.api#service"].serviceId;
3940
const testCases = service.traits["smithy.rules#endpointTests"]?.testCases;
41+
const Client: any = Object.entries(namespace).find(([k, v]) => k.endsWith("Client"))![1];
4042

4143
const ruleSet = service.traits["smithy.rules#endpointRuleSet"];
4244
const defaultEndpointResolver = (endpointParams: EndpointParams) => resolveEndpoint(ruleSet, { endpointParams });
@@ -46,14 +48,23 @@ function runTestCases(service: ServiceModel, namespace: ServiceNamespace) {
4648
const { documentation, params = {}, expect: expectation, operationInputs } = testCase;
4749
params.serviceId = serviceId;
4850

49-
it(documentation || "undocumented testcase", async () => {
51+
const test = Client.name === "DynamoDBClient" && "AccountId" in params ? it.skip : it;
52+
53+
test(documentation || "undocumented testcase", async () => {
5054
if ("endpoint" in expectation) {
5155
const { endpoint } = expectation;
5256
if (operationInputs) {
5357
for (const operationInput of operationInputs) {
5458
const { operationName, operationParams = {} } = operationInput;
55-
const command = namespace[`${operationName}Command`];
56-
const endpointParams = await resolveParams(operationParams, command, params);
59+
const Command = namespace[`${operationName}Command`];
60+
const endpointParams = await resolveParams(operationParams, Command, mapClientConfig(params));
61+
62+
// todo: Use an actual client for a more integrated test.
63+
// todo: This call returns an intercepted EndpointV2 object that can replace the one
64+
// todo: used below.
65+
void useClient;
66+
void [Client, params, Command, operationParams];
67+
5768
const observed = defaultEndpointResolver(endpointParams as EndpointParams);
5869
assertEndpointResolvedCorrectly(endpoint, observed);
5970
}
@@ -106,3 +117,72 @@ function assertEndpointResolvedCorrectly(expected: EndpointExpectation["endpoint
106117
expect(observed.properties?.authSchemes).toEqual(authSchemes);
107118
}
108119
}
120+
121+
/**
122+
* Makes a client operation return its EndpointV2 instead of making a request.
123+
*/
124+
const requestInterceptorMiddleware = (next: any, context: any) => async (args: any) => {
125+
const { request } = args;
126+
if (HttpRequest.isInstance(request)) {
127+
const endpoint = context.endpointV2;
128+
return {
129+
response: {
130+
statusCode: 200,
131+
},
132+
output: {
133+
...endpoint,
134+
url: {
135+
protocol: request.protocol,
136+
hostname: request.hostname,
137+
pathname: request.path,
138+
href: `${request.protocol}//${request.hostname}${request.path}`,
139+
} as URL,
140+
},
141+
} as {
142+
output: EndpointV2;
143+
};
144+
}
145+
throw new Error("Request must not continue beyond serialization step.");
146+
};
147+
const requestInterceptorMiddlewareOptions: RelativeMiddlewareOptions = {
148+
name: "requestInterceptorMiddleware",
149+
override: true,
150+
toMiddleware: "serializerMiddleware",
151+
relation: "after",
152+
};
153+
154+
const paramMap = {
155+
Region: "region",
156+
UseFIPS: "useFipsEndpoint",
157+
UseDualStack: "useDualstackEndpoint",
158+
ForcePathStyle: "forcePathStyle",
159+
Accelerate: "useAccelerateEndpoint",
160+
DisableMRAP: "disableMultiregionAccessPoints",
161+
DisableMultiRegionAccessPoints: "disableMultiregionAccessPoints",
162+
UseArnRegion: "useArnRegion",
163+
Endpoint: "endpoint",
164+
UseGlobalEndpoint: "useGlobalEndpoint",
165+
};
166+
167+
async function useClient(Client: any, Command: any, clientConfig: any, input: any): Promise<EndpointV2> {
168+
const client = new Client({
169+
...mapClientConfig(clientConfig),
170+
credentials: {
171+
accessKeyId: "ENDPOINTS_TEST",
172+
secretAccessKey: "ENDPOINTS_TEST",
173+
},
174+
});
175+
client.middlewareStack.addRelativeTo(requestInterceptorMiddleware, requestInterceptorMiddlewareOptions);
176+
const command = new Command(input);
177+
const observed: EndpointV2 = await client.send(command);
178+
return observed;
179+
}
180+
181+
function mapClientConfig(params: any) {
182+
return Object.entries(params).reduce((acc: any, cur: [string, any]) => {
183+
const [k, v] = cur;
184+
const key = paramMap[k as keyof typeof paramMap] ?? k;
185+
acc[key] = v;
186+
return acc;
187+
}, {} as any);
188+
}

0 commit comments

Comments
 (0)