Skip to content

Commit c3389f4

Browse files
committed
fix(client-s3): defer endpoint validations to ruleset
1 parent 2e76c7e commit c3389f4

File tree

15 files changed

+108
-132
lines changed

15 files changed

+108
-132
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ test-integration: bundles
4141
make test-endpoints
4242

4343
test-endpoints:
44-
npx jest -c ./tests/endpoints-2.0/jest.config.js --bail --watch --verbose false
44+
npx jest -c ./tests/endpoints-2.0/jest.config.js --bail --verbose false
4545

4646
test-e2e: bundles
4747
yarn g:vitest run -c vitest.config.e2e.ts --retry=4

clients/client-s3/src/S3Client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -755,9 +755,9 @@ export interface ClientDefaults extends Partial<__SmithyConfiguration<__HttpHand
755755
signingEscapePath?: boolean;
756756

757757
/**
758-
* Whether to override the request region with the region inferred from requested resource's ARN. Defaults to false.
758+
* Whether to override the request region with the region inferred from requested resource's ARN. Defaults to undefined.
759759
*/
760-
useArnRegion?: boolean | Provider<boolean>;
760+
useArnRegion?: boolean | undefined | Provider<boolean | undefined>;
761761
/**
762762
* The internal function that inject utilities to runtime-specific stream to help users consume the data
763763
* @internal

clients/client-s3/src/models/models_0.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// smithy-typescript generated code
22
import { ExceptionOptionType as __ExceptionOptionType, SENSITIVE_STRING } from "@smithy/smithy-client";
3-
43
import { StreamingBlobTypes } from "@smithy/types";
54

65
import { S3ServiceException as __BaseException } from "./S3ServiceException";

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// smithy-typescript generated code
22
import { ExceptionOptionType as __ExceptionOptionType, SENSITIVE_STRING } from "@smithy/smithy-client";
3-
43
import { StreamingBlobTypes } from "@smithy/types";
54

65
import {
@@ -41,7 +40,6 @@ import {
4140
Tag,
4241
TransitionDefaultMinimumObjectSize,
4342
} from "./models_0";
44-
4543
import { S3ServiceException as __BaseException } from "./S3ServiceException";
4644

4745
/**

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

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,7 @@
22
import { loadRestXmlErrorCode, parseXmlBody as parseBody, parseXmlErrorBody as parseErrorBody } from "@aws-sdk/core";
33
import { XmlNode as __XmlNode, XmlText as __XmlText } from "@aws-sdk/xml-builder";
44
import { requestBuilder as rb } from "@smithy/core";
5-
import {
6-
HttpRequest as __HttpRequest,
7-
HttpResponse as __HttpResponse,
8-
isValidHostname as __isValidHostname,
9-
} from "@smithy/protocol-http";
5+
import { HttpRequest as __HttpRequest, HttpResponse as __HttpResponse } from "@smithy/protocol-http";
106
import {
117
collectBody,
128
dateToUtcString as __dateToUtcString,
@@ -3215,18 +3211,6 @@ export const se_WriteGetObjectResponseCommand = async (
32153211
contents = input.Body;
32163212
body = contents;
32173213
}
3218-
let { hostname: resolvedHostname } = await context.endpoint();
3219-
if (context.disableHostPrefix !== true) {
3220-
resolvedHostname = "{RequestRoute}." + resolvedHostname;
3221-
if (input.RequestRoute === undefined) {
3222-
throw new Error("Empty value provided for input host prefix: RequestRoute.");
3223-
}
3224-
resolvedHostname = resolvedHostname.replace("{RequestRoute}", input.RequestRoute!);
3225-
if (!__isValidHostname(resolvedHostname)) {
3226-
throw new Error("ValidationError: prefixed hostname must be hostname compatible.");
3227-
}
3228-
}
3229-
b.hn(resolvedHostname);
32303214
b.m("POST").h(headers).b(body);
32313215
return b.build();
32323216
};

clients/client-s3/src/runtimeConfig.shared.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export const getRuntimeConfig = (config: S3ClientConfig) => {
4343
signerConstructor: config?.signerConstructor ?? SignatureV4MultiRegion,
4444
signingEscapePath: config?.signingEscapePath ?? false,
4545
urlParser: config?.urlParser ?? parseUrl,
46-
useArnRegion: config?.useArnRegion ?? false,
46+
useArnRegion: config?.useArnRegion ?? undefined,
4747
utf8Decoder: config?.utf8Decoder ?? fromUtf8,
4848
utf8Encoder: config?.utf8Encoder ?? toUtf8,
4949
};

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ public Model preprocessModel(Model model, TypeScriptSettings settings) {
224224

225225
Model builtModel = modelBuilder.addShapes(inputShapes).build();
226226
if (hasRuleset) {
227-
ModelTransformer.create().mapShapes(
227+
return ModelTransformer.create().mapShapes(
228228
builtModel, AddS3Config::removeHostPrefixTrait
229229
);
230230
}
@@ -246,9 +246,9 @@ public void addConfigInterfaceFields(
246246
.write("signingEscapePath?: boolean;\n");
247247
writer.writeDocs(
248248
"Whether to override the request region with the region inferred from requested resource's ARN."
249-
+ " Defaults to false.")
249+
+ " Defaults to undefined.")
250250
.addImport("Provider", "Provider", TypeScriptDependency.SMITHY_TYPES)
251-
.write("useArnRegion?: boolean | Provider<boolean>;");
251+
.write("useArnRegion?: boolean | undefined | Provider<boolean | undefined>;");
252252
}
253253

254254
@Override
@@ -266,7 +266,7 @@ public Map<String, Consumer<TypeScriptWriter>> getRuntimeConfigWriters(
266266
writer.write("false");
267267
},
268268
"useArnRegion", writer -> {
269-
writer.write("false");
269+
writer.write("undefined");
270270
}
271271
);
272272
case NODE:

packages/middleware-bucket-endpoint/src/NodeUseArnRegionConfigOptions.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,15 @@ export const NODE_USE_ARN_REGION_INI_NAME = "s3_use_arn_region";
77
/**
88
* Config to load useArnRegion from environment variables and shared INI files
99
*
10-
* @api private
10+
* @internal
1111
*/
12-
export const NODE_USE_ARN_REGION_CONFIG_OPTIONS: LoadedConfigSelectors<boolean> = {
12+
export const NODE_USE_ARN_REGION_CONFIG_OPTIONS: LoadedConfigSelectors<boolean | undefined> = {
1313
environmentVariableSelector: (env: NodeJS.ProcessEnv) =>
1414
booleanSelector(env, NODE_USE_ARN_REGION_ENV_NAME, SelectorType.ENV),
1515
configFileSelector: (profile) => booleanSelector(profile, NODE_USE_ARN_REGION_INI_NAME, SelectorType.CONFIG),
16-
default: false,
16+
/**
17+
* useArnRegion has specific behavior when undefined instead of false.
18+
* We therefore use undefined as the default value instead of false.
19+
*/
20+
default: undefined,
1721
};

packages/middleware-bucket-endpoint/src/bucketHostname.ts

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,9 @@ import {
1414
validateCustomEndpoint,
1515
validateDNSHostLabel,
1616
validateMrapAlias,
17-
validateNoDualstack,
1817
validateNoFIPS,
1918
validateOutpostService,
2019
validatePartition,
21-
validateRegion,
2220
validateRegionalClient,
2321
validateS3Service,
2422
validateService,
@@ -121,14 +119,6 @@ const getEndpointFromObjectLambdaArn = ({
121119
}): BucketHostname => {
122120
const { accountId, region, service } = bucketName;
123121
validateRegionalClient(clientRegion);
124-
validateRegion(region, {
125-
useArnRegion,
126-
clientRegion,
127-
clientSigningRegion,
128-
allowFipsRegion: true,
129-
useFipsEndpoint: fipsEndpoint,
130-
});
131-
validateNoDualstack(dualstackEndpoint);
132122
const DNSHostLabel = `${accesspointName}-${accountId}`;
133123
validateDNSHostLabel(DNSHostLabel, { tlsCompatible });
134124

@@ -155,7 +145,6 @@ const getEndpointFromMRAPArn = ({
155145
throw new Error("SDK is attempting to use a MRAP ARN. Please enable to feature.");
156146
}
157147
validateMrapAlias(mrapAlias);
158-
validateNoDualstack(dualstackEndpoint);
159148
return {
160149
bucketEndpoint: true,
161150
hostname: `${mrapAlias}${isCustomEndpoint ? "" : `.accesspoint.s3-global`}.${hostnameSuffix}`,
@@ -178,14 +167,12 @@ const getEndpointFromOutpostArn = ({
178167
}: ArnHostnameParams & { outpostId: string; accesspointName: string; hostnameSuffix: string }): BucketHostname => {
179168
// if this is an Outpost ARN
180169
validateRegionalClient(clientRegion);
181-
validateRegion(bucketName.region, { useArnRegion, clientRegion, clientSigningRegion, useFipsEndpoint: fipsEndpoint });
182170
const DNSHostLabel = `${accesspointName}-${bucketName.accountId}`;
183171
validateDNSHostLabel(DNSHostLabel, { tlsCompatible });
184172
const endpointRegion = useArnRegion ? bucketName.region : clientRegion;
185173
const signingRegion = useArnRegion ? bucketName.region : clientSigningRegion;
186174
validateOutpostService(bucketName.service);
187175
validateDNSHostLabel(outpostId, { tlsCompatible });
188-
validateNoDualstack(dualstackEndpoint);
189176
validateNoFIPS(fipsEndpoint);
190177
const hostnamePrefix = `${DNSHostLabel}.${outpostId}`;
191178
return {
@@ -210,13 +197,6 @@ const getEndpointFromAccessPointArn = ({
210197
}: ArnHostnameParams & { accesspointName: string; hostnameSuffix: string }): BucketHostname => {
211198
// construct endpoint from Accesspoint ARN
212199
validateRegionalClient(clientRegion);
213-
validateRegion(bucketName.region, {
214-
useArnRegion,
215-
clientRegion,
216-
clientSigningRegion,
217-
allowFipsRegion: true,
218-
useFipsEndpoint: fipsEndpoint,
219-
});
220200
const hostnamePrefix = `${accesspointName}-${bucketName.accountId}`;
221201
validateDNSHostLabel(hostnamePrefix, { tlsCompatible });
222202
const endpointRegion = useArnRegion ? bucketName.region : clientRegion;

packages/middleware-bucket-endpoint/src/bucketHostnameUtils.ts

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,14 @@ export const validatePartition = (partition: string, options: { clientPartition:
111111
};
112112

113113
/**
114+
* (Previous to deprecation)
114115
* validate region value inferred from ARN. If `options.useArnRegion` is set, it validates the region is not a FIPS
115116
* region. If `options.useArnRegion` is unset, it validates the region is equal to `options.clientRegion` or
116117
* `options.clientSigningRegion`.
118+
*
117119
* @internal
120+
*
121+
* @deprecated validation is deferred to the endpoint ruleset.
118122
*/
119123
export const validateRegion = (
120124
region: string,
@@ -125,28 +129,9 @@ export const validateRegion = (
125129
clientSigningRegion: string;
126130
useFipsEndpoint: boolean;
127131
}
128-
) => {
129-
if (region === "") {
130-
throw new Error("ARN region is empty");
131-
}
132-
if (options.useFipsEndpoint) {
133-
if (!options.allowFipsRegion) {
134-
throw new Error("FIPS region is not supported");
135-
} else if (!isEqualRegions(region, options.clientRegion)) {
136-
throw new Error(`Client FIPS region ${options.clientRegion} doesn't match region ${region} in ARN`);
137-
}
138-
}
139-
if (
140-
!options.useArnRegion &&
141-
!isEqualRegions(region, options.clientRegion || "") &&
142-
!isEqualRegions(region, options.clientSigningRegion || "")
143-
) {
144-
throw new Error(`Region in ARN is incompatible, got ${region} but expected ${options.clientRegion}`);
145-
}
146-
};
132+
) => {};
147133

148134
/**
149-
*
150135
* @param region
151136
*/
152137
export const validateRegionalClient = (region: string) => {
@@ -231,13 +216,12 @@ export const getArnResources = (
231216
};
232217

233218
/**
234-
* Throw if dual stack configuration is set to true.
219+
* (Prior to deprecation) Throw if dual stack configuration is set to true.
235220
* @internal
221+
*
222+
* @deprecated validation deferred to endpoints ruleset.
236223
*/
237-
export const validateNoDualstack = (dualstackEndpoint?: boolean) => {
238-
if (dualstackEndpoint)
239-
throw new Error("Dualstack endpoint is not supported with Outpost or Multi-region Access Point ARN.");
240-
};
224+
export const validateNoDualstack = (dualstackEndpoint?: boolean) => {};
241225

242226
/**
243227
* Validate fips endpoint is not set up.

0 commit comments

Comments
 (0)