Skip to content

Commit

Permalink
chore(cli): do not hardcode AWS::URLSuffix in hotswapping (#17104)
Browse files Browse the repository at this point in the history
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
comcalvi authored Oct 25, 2021
1 parent 8343bec commit 06fb9f9
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 25 deletions.
20 changes: 20 additions & 0 deletions packages/aws-cdk/lib/api/aws-auth/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ import { cached } from '../../util/functions';
import { AccountAccessKeyCache } from './account-cache';
import { Account } from './sdk-provider';

// We need to map regions to domain suffixes, and the SDK already has a function to do this.
// It's not part of the public API, but it's also unlikely to go away.
//
// Reuse that function, and add a safety check so we don't accidentally break if they ever
// refactor that away.

/* eslint-disable @typescript-eslint/no-require-imports */
const regionUtil = require('aws-sdk/lib/region_config');
/* eslint-enable @typescript-eslint/no-require-imports */

if (!regionUtil.getEndpointSuffix) {
throw new Error('This version of AWS SDK for JS does not have the \'getEndpointSuffix\' function!');
}

export interface ISDK {
/**
* The region this SDK has been instantiated for
Expand All @@ -22,6 +36,8 @@ export interface ISDK {
*/
currentAccount(): Promise<Account>;

getEndpointSuffix(region: string): string;

lambda(): AWS.Lambda;
cloudFormation(): AWS.CloudFormation;
ec2(): AWS.EC2;
Expand Down Expand Up @@ -190,6 +206,10 @@ export class SDK implements ISDK {
}
}

public getEndpointSuffix(region: string): string {
return regionUtil.getEndpointSuffix(region);
}

/**
* Return a wrapping object for the underlying service object
*
Expand Down
26 changes: 6 additions & 20 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,6 @@ import {
} from './util/cloudformation';
import { StackActivityMonitor, StackActivityProgress } from './util/cloudformation/stack-activity-monitor';

// We need to map regions to domain suffixes, and the SDK already has a function to do this.
// It's not part of the public API, but it's also unlikely to go away.
//
// Reuse that function, and add a safety check so we don't accidentally break if they ever
// refactor that away.

/* eslint-disable @typescript-eslint/no-require-imports */
const regionUtil = require('aws-sdk/lib/region_config');
/* eslint-enable @typescript-eslint/no-require-imports */

if (!regionUtil.getEndpointSuffix) {
throw new Error('This version of AWS SDK for JS does not have the \'getEndpointSuffix\' function!');
}

type TemplateBodyParameter = {
TemplateBody?: string
TemplateURL?: string
Expand Down Expand Up @@ -247,8 +233,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
debug(`${deployName}: deploying...`);
}

const bodyParameter = await makeBodyParameter(stackArtifact, options.resolvedEnvironment, legacyAssets, options.toolkitInfo);

const bodyParameter = await makeBodyParameter(stackArtifact, options.resolvedEnvironment, legacyAssets, options.toolkitInfo, options.sdk);
await publishAssets(legacyAssets.toManifest(stackArtifact.assembly.directory), options.sdkProvider, stackEnv);

if (options.hotswap) {
Expand Down Expand Up @@ -382,11 +367,12 @@ async function makeBodyParameter(
stack: cxapi.CloudFormationStackArtifact,
resolvedEnvironment: cxapi.Environment,
assetManifest: AssetManifestBuilder,
toolkitInfo: ToolkitInfo): Promise<TemplateBodyParameter> {
toolkitInfo: ToolkitInfo,
sdk: ISDK): Promise<TemplateBodyParameter> {

// If the template has already been uploaded to S3, just use it from there.
if (stack.stackTemplateAssetObjectUrl) {
return { TemplateURL: restUrlFromManifest(stack.stackTemplateAssetObjectUrl, resolvedEnvironment) };
return { TemplateURL: restUrlFromManifest(stack.stackTemplateAssetObjectUrl, resolvedEnvironment, sdk) };
}

// Otherwise, pass via API call (if small) or upload here (if large)
Expand Down Expand Up @@ -553,7 +539,7 @@ function compareTags(a: Tag[], b: Tag[]): boolean {
* and reformats s3://.../... urls into S3 REST URLs (which CloudFormation
* expects)
*/
function restUrlFromManifest(url: string, environment: cxapi.Environment): string {
function restUrlFromManifest(url: string, environment: cxapi.Environment, sdk: ISDK): string {
const doNotUseMarker = '**DONOTUSE**';
// This URL may contain placeholders, so still substitute those.
url = cxapi.EnvironmentPlaceholders.replace(url, {
Expand All @@ -576,6 +562,6 @@ function restUrlFromManifest(url: string, environment: cxapi.Environment): strin
const bucketName = s3Url[1];
const objectKey = s3Url[2];

const urlSuffix: string = regionUtil.getEndpointSuffix(environment.region);
const urlSuffix: string = sdk.getEndpointSuffix(environment.region);
return `https://s3.${environment.region}.${urlSuffix}/${bucketName}/${objectKey}`;
}
3 changes: 1 addition & 2 deletions packages/aws-cdk/lib/api/hotswap-deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ export async function tryHotswapDeployment(
account: resolvedEnv.account,
region: resolvedEnv.region,
partition: (await sdk.currentAccount()).partition,
// ToDo make this better:
urlSuffix: 'amazonaws.com',
urlSuffix: sdk.getEndpointSuffix,
listStackResources,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export interface EvaluateCloudFormationTemplateProps {
readonly account: string;
readonly region: string;
readonly partition: string;
readonly urlSuffix: string;
readonly urlSuffix: (region: string) => string;
readonly listStackResources: ListStackResources;
}

Expand All @@ -27,6 +27,8 @@ export class EvaluateCloudFormationTemplate {
private readonly account: string;
private readonly region: string;
private readonly partition: string;
private readonly urlSuffix: (region: string) => string;
private cachedUrlSuffix: string | undefined;

constructor(props: EvaluateCloudFormationTemplateProps) {
this.stackResources = props.listStackResources;
Expand All @@ -35,12 +37,12 @@ export class EvaluateCloudFormationTemplate {
'AWS::AccountId': props.account,
'AWS::Region': props.region,
'AWS::Partition': props.partition,
'AWS::URLSuffix': props.urlSuffix,
...props.parameters,
};
this.account = props.account;
this.region = props.region;
this.partition = props.partition;
this.urlSuffix = props.urlSuffix;
}

public async findPhysicalNameFor(logicalId: string): Promise<string | undefined> {
Expand Down Expand Up @@ -184,6 +186,14 @@ export class EvaluateCloudFormationTemplate {

private async findRefTarget(logicalId: string): Promise<string | undefined> {
// first, check to see if the Ref is a Parameter who's value we have
if (logicalId === 'AWS::URLSuffix') {
if (!this.cachedUrlSuffix) {
this.cachedUrlSuffix = this.urlSuffix(this.region);
}

return this.cachedUrlSuffix;
}

const parameterTarget = this.context[logicalId];
if (parameterTarget) {
return parameterTarget;
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/test/api/deploy-stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const FAKE_STACK_TERMINATION_PROTECTION = testStack({
let sdk: MockSdk;
let sdkProvider: MockSdkProvider;
let cfnMocks: MockedObject<SyncHandlerSubsetOf<AWS.CloudFormation>>;

beforeEach(() => {
jest.resetAllMocks();

Expand Down Expand Up @@ -62,7 +63,7 @@ beforeEach(() => {
updateTerminationProtection: jest.fn((_o) => ({ StackId: 'stack-id' })),
};
sdk.stubCloudFormation(cfnMocks as any);

sdk.stubGetEndpointSuffix(() => 'amazonaws.com');
});

function standardDeployStackArguments(): DeployStackOptions {
Expand Down
69 changes: 69 additions & 0 deletions packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import * as setup from './hotswap-test-setup';
let cfnMockProvider: setup.CfnMockProvider;
let mockUpdateLambdaCode: (params: Lambda.Types.UpdateFunctionCodeRequest) => Lambda.Types.FunctionConfiguration;
let mockUpdateMachineDefinition: (params: StepFunctions.Types.UpdateStateMachineInput) => StepFunctions.Types.UpdateStateMachineOutput;
let mockGetEndpointSuffix: () => string;

beforeEach(() => {
cfnMockProvider = setup.setupHotswapTests();
mockUpdateLambdaCode = jest.fn();
mockUpdateMachineDefinition = jest.fn();
mockGetEndpointSuffix = jest.fn(() => 'amazonaws.com');
cfnMockProvider.setUpdateFunctionCodeMock(mockUpdateLambdaCode);
cfnMockProvider.setUpdateStateMachineMock(mockUpdateMachineDefinition);
cfnMockProvider.stubGetEndpointSuffix(mockGetEndpointSuffix);
});

test('returns a deployStackResult with noOp=true when it receives an empty set of changes', async () => {
Expand Down Expand Up @@ -240,3 +243,69 @@ test('can correctly reference AWS::Partition in hotswappable changes', async ()
S3Key: 'new-key',
});
});

test('can correctly reference AWS::URLSuffix in hotswappable changes', async () => {
// GIVEN
setup.setCurrentCfnStackTemplate({
Resources: {
Func: {
Type: 'AWS::Lambda::Function',
Properties: {
Code: {
S3Bucket: 'current-bucket',
S3Key: 'current-key',
},
FunctionName: {
'Fn::Join': ['', [
'my-function-',
{ Ref: 'AWS::URLSuffix' },
'-',
{ Ref: 'AWS::URLSuffix' },
]],
},
},
Metadata: {
'aws:asset:path': 'old-path',
},
},
},
});
const cdkStackArtifact = setup.cdkStackArtifactOf({
template: {
Resources: {
Func: {
Type: 'AWS::Lambda::Function',
Properties: {
Code: {
S3Bucket: 'current-bucket',
S3Key: 'new-key',
},
FunctionName: {
'Fn::Join': ['', [
'my-function-',
{ Ref: 'AWS::URLSuffix' },
'-',
{ Ref: 'AWS::URLSuffix' },
]],
},
},
Metadata: {
'aws:asset:path': 'new-path',
},
},
},
},
});

// WHEN
const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact);

// THEN
expect(deployStackResult).not.toBeUndefined();
expect(mockUpdateLambdaCode).toHaveBeenCalledWith({
FunctionName: 'my-function-amazonaws.com-amazonaws.com',
S3Bucket: 'current-bucket',
S3Key: 'new-key',
});
expect(mockGetEndpointSuffix).toHaveBeenCalledTimes(1);
});
4 changes: 4 additions & 0 deletions packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,8 @@ export class CfnMockProvider {
): Promise<DeployStackResult | undefined> {
return deployments.tryHotswapDeployment(this.mockSdkProvider, assetParams, currentCfnStack, stackArtifact);
}

public stubGetEndpointSuffix(stub: () => string) {
this.mockSdkProvider.stubGetEndpointSuffix(stub);
}
}
12 changes: 12 additions & 0 deletions packages/aws-cdk/test/util/mock-sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ export class MockSdkProvider extends SdkProvider {
public stubStepFunctions(stubs: SyncHandlerSubsetOf<AWS.StepFunctions>) {
(this.sdk as any).stepFunctions = jest.fn().mockReturnValue(partialAwsService<AWS.StepFunctions>(stubs));
}

public stubGetEndpointSuffix(stub: () => string) {
this.sdk.getEndpointSuffix = stub;
}
}

export class MockSdk implements ISDK {
Expand All @@ -125,6 +129,7 @@ export class MockSdk implements ISDK {
public readonly secretsManager = jest.fn();
public readonly kms = jest.fn();
public readonly stepFunctions = jest.fn();
public readonly getEndpointSuffix = jest.fn();

public currentAccount(): Promise<Account> {
return Promise.resolve({ accountId: '123456789012', partition: 'aws' });
Expand All @@ -150,6 +155,13 @@ export class MockSdk implements ISDK {
public stubSsm(stubs: SyncHandlerSubsetOf<AWS.SSM>) {
this.ssm.mockReturnValue(partialAwsService<AWS.SSM>(stubs));
}

/**
* Replace the getEndpointSuffix client with the given object
*/
public stubGetEndpointSuffix(stub: () => string) {
this.getEndpointSuffix.mockReturnValue(stub());
}
}

/**
Expand Down

0 comments on commit 06fb9f9

Please sign in to comment.