Skip to content

fix(endpoint): misc fixes for endpoints 2.0 based on service unit tests #4002

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { EndpointParameters, EndpointV2, HandlerExecutionContext } from "@aws-sdk/types";

import { EndpointResolvedConfig } from "../resolveEndpointConfig";
import { resolveParamsForS3 } from "../service-customizations";
import { EndpointParameterInstructions } from "../types";

export type EndpointParameterInstructionsSupplier = Partial<{
Expand Down Expand Up @@ -31,13 +32,28 @@ export const getEndpointFromInstructions = async <
clientConfig: Partial<EndpointResolvedConfig<T>> & Config,
context?: HandlerExecutionContext
): Promise<EndpointV2> => {
const endpointParams: EndpointParameters = {};
const instructions: EndpointParameterInstructions =
(instructionsSupplier.getEndpointParameterInstructions || (() => null))() || {};
const endpointParams = await resolveParams(commandInput, instructionsSupplier, clientConfig);

if (typeof clientConfig.endpointProvider !== "function") {
throw new Error("config.endpointProvider is not set.");
}
const endpoint: EndpointV2 = clientConfig.endpointProvider!(endpointParams as T, context);

return endpoint;
};

export const resolveParams = async <
T extends EndpointParameters,
CommandInput extends Record<string, unknown>,
Config extends Record<string, unknown>
>(
commandInput: CommandInput,
instructionsSupplier: EndpointParameterInstructionsSupplier,
clientConfig: Partial<EndpointResolvedConfig<T>> & Config
) => {
const endpointParams: EndpointParameters = {};
const instructions: EndpointParameterInstructions =
(instructionsSupplier.getEndpointParameterInstructions || (() => null))() || {};

for (const [name, instruction] of Object.entries(instructions)) {
switch (instruction.type) {
Expand All @@ -49,25 +65,35 @@ export const getEndpointFromInstructions = async <
break;
case "clientContextParams":
case "builtInParams":
endpointParams[name] = await createConfigProvider<Config>(instruction.name, clientConfig)();
endpointParams[name] = await createConfigProvider<Config>(instruction.name, name, clientConfig)();
break;
default:
throw new Error("Unrecognized endpoint parameter instruction: " + JSON.stringify(instruction));
}
}

const endpoint: EndpointV2 = clientConfig.endpointProvider!(endpointParams as T, context);
if (Object.keys(instructions).length === 0) {
Object.assign(endpointParams, clientConfig);
}

return endpoint;
if (String(clientConfig.serviceId).toLowerCase() === "s3") {
resolveParamsForS3(endpointParams);
}

return endpointParams;
};

/**
* Normalize some key of the client config to an async provider.
* @private
*/
const createConfigProvider = <Config extends Record<string, unknown>>(configKey: string, config: Config) => {
const createConfigProvider = <Config extends Record<string, unknown>>(
configKey: string,
canonicalEndpointParamKey: string,
config: Config
) => {
const configProvider = async () => {
const configValue: unknown = config[configKey];
const configValue: unknown = config[configKey] || config[canonicalEndpointParamKey];
if (typeof configValue === "function") {
return configValue();
}
Expand Down
10 changes: 1 addition & 9 deletions packages/middleware-endpoint/src/endpointMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,8 @@ import {

import { getEndpointFromInstructions } from "./adaptors/getEndpointFromInstructions";
import { EndpointResolvedConfig } from "./resolveEndpointConfig";
import { s3Customizations } from "./service-customizations";
import { EndpointParameterInstructions } from "./types";

export type PreviouslyResolvedServiceId = {
serviceId?: string;
};

/**
* @private
Expand All @@ -26,7 +22,7 @@ export const endpointMiddleware = <T extends EndpointParameters>({
config,
instructions,
}: {
config: EndpointResolvedConfig<T> & PreviouslyResolvedServiceId;
config: EndpointResolvedConfig<T>;
instructions: EndpointParameterInstructions;
}): SerializeMiddleware<any, any> => {
return <Output extends MetadataBearer>(
Expand Down Expand Up @@ -54,10 +50,6 @@ export const endpointMiddleware = <T extends EndpointParameters>({
context["signing_service"] = authScheme.signingName;
}

if (config.serviceId === "S3") {
await s3Customizations(config, instructions, args, context);
}

return next({
...args,
});
Expand Down
4 changes: 2 additions & 2 deletions packages/middleware-endpoint/src/getEndpointPlugin.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { serializerMiddlewareOption } from "@aws-sdk/middleware-serde";
import { EndpointParameters, Pluggable, RelativeMiddlewareOptions, SerializeHandlerOptions } from "@aws-sdk/types";

import { endpointMiddleware, PreviouslyResolvedServiceId } from "./endpointMiddleware";
import { endpointMiddleware } from "./endpointMiddleware";
import { EndpointResolvedConfig } from "./resolveEndpointConfig";
import { EndpointParameterInstructions } from "./types";

Expand All @@ -15,7 +15,7 @@ export const endpointMiddlewareOptions: SerializeHandlerOptions & RelativeMiddle
};

export const getEndpointPlugin = <T extends EndpointParameters>(
config: EndpointResolvedConfig<T> & PreviouslyResolvedServiceId,
config: EndpointResolvedConfig<T>,
instructions: EndpointParameterInstructions
): Pluggable<any, any> => ({
applyToStack: (clientStack) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { EndpointParameterInstructionsSupplier } from "../adaptors";

export interface EndpointTestCase {
documentation?: string;
params?: Record<string, boolean | string | unknown>;
tags?: string[];
expect: EndpointExpectation | ErrorExpectation;
operationInputs?: OperationInput[];
}

export type OperationInput = {
operationName: string;
operationParams?: Record<string, string | boolean | unknown>;
builtInParams?: Record<string, string | boolean | unknown>;
clientParams?: Record<string, string | boolean | unknown>;
};

export type EndpointExpectation = {
endpoint: {
url: string;
properties?: Record<string, unknown>;
headers?: Record<string, string[]>;
};
};

export type ErrorExpectation = {
error: string;
};

export interface ServiceNamespace {
[Command: string]: EndpointParameterInstructionsSupplier;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import * as S3Namespace from "@aws-sdk/client-s3";
import { EndpointV2 } from "@aws-sdk/types";

// import { defaultEndpointResolver } from "../../../../clients/client-s3/src/endpoint/endpointResolver";
const defaultEndpointResolver = (...args: any): any => {};

import { resolveParams } from "../adaptors";
import { EndpointExpectation, EndpointTestCase, ErrorExpectation, ServiceNamespace } from "./integration-test-types";
import customTests from "./s3/custom-tests.json";

describe("endpoints 2.0 service integration", () => {
describe("s3", () => {
it("placeholder", () => {});
runTestCases(customTests, S3Namespace as ServiceNamespace);
});
});

function runTestCases({ testCases }: { testCases: EndpointTestCase[] }, service: ServiceNamespace) {
const start = 0; //67;
const end = 100; //68;
for (const testCase of testCases.slice(start, end)) {
runTestCase(testCase, service);
}
}

async function runTestCase(testCase: EndpointTestCase, service: ServiceNamespace) {
const { documentation, params = {}, expect: expectation, operationInputs } = testCase;

it(documentation || "undocumented testcase", async () => {
if (isEndpointExpectation(expectation)) {
const { endpoint } = expectation;
if (operationInputs) {
for (const operationInput of operationInputs) {
const { operationName, operationParams = {} } = operationInput;
const endpointParams = await resolveParams(operationParams, service[`${operationName}Command`], params);
const observed = defaultEndpointResolver(endpointParams as any);
console.log("params were", endpointParams);
assertEndpointResolvedCorrectly(endpoint, observed);
}
} else {
const endpointParams = await resolveParams({}, {}, params);
const observed = defaultEndpointResolver(endpointParams as any);
console.log("params were", endpointParams);
assertEndpointResolvedCorrectly(endpoint, observed);
}
}
if (isErrorExpectation(expectation)) {
const { error } = expectation;
const pass = (err) => err;
const normalizeQuotes = (s) => s.replace(/`/g, "");
if (operationInputs) {
for (const operationInput of operationInputs) {
const { operationName, operationParams = {} } = operationInput;
const endpointParams = await resolveParams(operationParams, service[`${operationName}Command`], {
...params,
endpointProvider: defaultEndpointResolver,
});
const observedError = await (async () => defaultEndpointResolver(endpointParams as any))().catch(pass);
expect(observedError).not.toBeUndefined();
// expect(normalizeQuotes(String(observedError))).toContain(normalizeQuotes(error));
}
} else {
const endpointParams = await resolveParams({}, {}, params);
const observedError = await (async () => defaultEndpointResolver(endpointParams as any))().catch(pass);
console.error(observedError);
expect(observedError).not.toBeUndefined();
// expect(normalizeQuotes(String(observedError))).toContain(normalizeQuotes(error));
}
}
});
}

function assertEndpointResolvedCorrectly(expected: EndpointExpectation["endpoint"], observed: EndpointV2) {
const { url, headers, properties } = expected;
const { authSchemes } = properties || {};
if (url) {
expect(observed.url.href).toContain(url);
expect(Math.abs(observed.url.href.length - url.length)).toBeLessThan(2);
}
if (headers) {
expect(observed.headers).toEqual(headers);
}
if (authSchemes) {
// expect(observed.properties?.authSchemes).toEqual(authSchemes);
}
}

function isEndpointExpectation(expectation: object): expectation is EndpointExpectation {
return "endpoint" in expectation;
}

function isErrorExpectation(expectation: object): expectation is ErrorExpectation {
return "error" in expectation;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"testCases": [],
"version": "1.0"
}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from './s3'
export * from "./s3";
34 changes: 6 additions & 28 deletions packages/middleware-endpoint/src/service-customizations/s3.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,13 @@
import { EndpointParameters, HandlerExecutionContext, SerializeHandlerArguments } from "@aws-sdk/types";
import { EndpointParameters } from "@aws-sdk/types";

import { getEndpointFromInstructions } from "../adaptors";
import { PreviouslyResolvedServiceId } from "../endpointMiddleware";
import { EndpointResolvedConfig } from "../resolveEndpointConfig";
import { EndpointParameterInstructions } from "../types";

/**
* Bucket name DNS compatibility customization.
*/
export const s3Customizations = async <T extends EndpointParameters>(
config: EndpointResolvedConfig<T> & PreviouslyResolvedServiceId,
instructions: EndpointParameterInstructions,
args: SerializeHandlerArguments<any>,
context: HandlerExecutionContext
) => {
const endpoint = context.endpointV2;
const bucket: string | undefined = args.input?.Bucket || void 0;

if (!endpoint || !bucket) {
return;
}
export const resolveParamsForS3 = async (endpointParams: EndpointParameters) => {
const bucket = (endpointParams?.Bucket as string) || "";

if (!isDnsCompatibleBucketName(bucket) || bucket.indexOf(".") !== -1) {
context.endpointV2 = await getEndpointFromInstructions(
args.input,
{
getEndpointParameterInstructions: () => instructions,
},
{ ...config, forcePathStyle: true }
);
endpointParams.ForcePathStyle = true;
}

return endpointParams;
};

const DOMAIN_PATTERN = /^[a-z0-9][a-z0-9\.\-]{1,61}[a-z0-9]$/;
Expand Down
2 changes: 1 addition & 1 deletion packages/util-endpoints/src/lib/isValidHostLabel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { isValidHostLabel } from "./isValidHostLabel";

describe(isValidHostLabel.name, () => {
const testCases: Array<[boolean, string]> = [
[false, "01010"],
[true, "01010"],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this because I saw that subdomains can be made of only numbers.
Hostnames can start with a number too.

[true, "abc"],
[true, "A0c"],
[false, "A0c-"],
Expand Down
2 changes: 1 addition & 1 deletion packages/util-endpoints/src/lib/isValidHostLabel.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const VALID_HOST_LABEL_REGEX = new RegExp(`^(?![0-9]+$)(?!.*-$)(?!-)[a-zA-Z0-9-]{1,63}$`);
const VALID_HOST_LABEL_REGEX = new RegExp(`^(?!.*-$)(?!-)[a-zA-Z0-9-]{1,63}$`);

/**
* Evaluates whether one or more string values are valid host labels per RFC 1123.
Expand Down
7 changes: 6 additions & 1 deletion packages/util-endpoints/src/utils/getEndpointUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ import { evaluateExpression } from "./evaluateExpression";
export const getEndpointUrl = (endpointUrl: Expression, options: EvaluateOptions): URL => {
const expression = evaluateExpression(endpointUrl, "Endpoint URL", options);
if (typeof expression === "string") {
return new URL(expression);
try {
return new URL(expression);
} catch (error) {
console.error(`Failed to construct URL with ${expression}`, error);
throw error;
}
}
throw new EndpointError(`Endpoint URL must be a string, got ${typeof expression}`);
};