Skip to content
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

feat(cli): support Fn::ImportValue intrinsic function for hotswap deployments #27292

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6c25649
test: basic units for EvaluateCloudFormationTemplate
tomwwright Sep 25, 2023
1958df1
test: add basic literal expression evaluation tests
tomwwright Sep 26, 2023
599ba98
feat: LazyLookupExport class
tomwwright Sep 26, 2023
8f9443e
fix: typo in filename
tomwwright Sep 26, 2023
a48f8c7
test: tdd for importvalue resolver
tomwwright Sep 26, 2023
b1c4cda
feat: resolve Fn::ImportValue
tomwwright Sep 26, 2023
14681d1
refactor: undefined instead of throw when export not found
tomwwright Sep 26, 2023
01f278a
style: remove unused import
tomwwright Sep 26, 2023
4ac2b6e
fix: pass new lookup in other usages
tomwwright Sep 26, 2023
3e3bb12
test: add Fn::ImportValue resolution test
tomwwright Sep 27, 2023
a4828ac
docs: add clarifying note on intrinsic function support for hotswaps
tomwwright Sep 27, 2023
e570c15
refactor: unroll internal to generator
tomwwright Sep 27, 2023
1498d17
refactor: more descriptive variable name
tomwwright Sep 27, 2023
d9b5c4c
refactor: tighter variable scoping
tomwwright Sep 27, 2023
d93f78b
docs: qualify partal support for fn::getatt
tomwwright Oct 1, 2023
97e7563
refactor: rely on abstraction not implementation
tomwwright Oct 1, 2023
6fcb17c
refactor: lookupExport into an internal concern
tomwwright Oct 1, 2023
72e594d
fix: remove unused import
tomwwright Oct 1, 2023
96e2602
docs: remove code link to be more suitable for user-facing doco
tomwwright Oct 2, 2023
7eb4d78
test: add integration test for hotswap support
tomwwright Oct 16, 2023
9d9c4af
fix: silently skip listed exports without name
tomwwright Oct 16, 2023
12e9c6a
docs: add link to qualify fn::getatt limitations
tomwwright Oct 17, 2023
d5b5361
Merge branch 'main' into tomwwright/support-fn-importvalue-in-evaluate
vinayak-kukreja Oct 18, 2023
f5c30dc
Merge branch 'main' into tomwwright/support-fn-importvalue-in-evaluate
vinayak-kukreja Oct 19, 2023
79cb2c7
Merge branch 'main' into tomwwright/support-fn-importvalue-in-evaluate
mergify[bot] Oct 19, 2023
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
26 changes: 24 additions & 2 deletions packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,12 @@ class LambdaHotswapStack extends cdk.Stack {
handler: 'index.handler',
description: process.env.DYNAMIC_LAMBDA_PROPERTY_VALUE ?? "description",
environment: {
SomeVariable: process.env.DYNAMIC_LAMBDA_PROPERTY_VALUE ?? "environment",
}
SomeVariable:
process.env.DYNAMIC_LAMBDA_PROPERTY_VALUE ?? "environment",
ImportValueVariable: process.env.USE_IMPORT_VALUE_LAMBDA_PROPERTY
? cdk.Fn.importValue(TEST_EXPORT_OUTPUT_NAME)
: "no-import",
},
});

new cdk.CfnOutput(this, 'FunctionName', { value: fn.functionName });
Expand Down Expand Up @@ -343,6 +347,22 @@ class ConditionalResourceStack extends cdk.Stack {
}
}

const TEST_EXPORT_OUTPUT_NAME = 'test-export-output';

class ExportValueStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);

// just need any resource to exist within the stack
const topic = new sns.Topic(this, 'Topic');

new cdk.CfnOutput(this, 'ExportValueOutput', {
exportName: TEST_EXPORT_OUTPUT_NAME,
value: topic.topicArn,
});
}
}

class BundlingStage extends cdk.Stage {
constructor(parent, id, props) {
super(parent, id, props);
Expand Down Expand Up @@ -450,6 +470,8 @@ switch (stackSet) {

new ImportableStack(app, `${stackPrefix}-importable-stack`);

new ExportValueStack(app, `${stackPrefix}-export-value-stack`);

new BundlingStage(app, `${stackPrefix}-bundling-stage`);
break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,48 @@ integTest('hotswap deployment supports Lambda function\'s description and enviro
expect(deployOutput).toContain(`Lambda Function '${functionName}' hotswapped!`);
}));

integTest('hotswap deployment supports Fn::ImportValue intrinsic', withDefaultFixture(async (fixture) => {
// GIVEN
try {
await fixture.cdkDeploy('export-value-stack');
const stackArn = await fixture.cdkDeploy('lambda-hotswap', {
captureStderr: false,
modEnv: {
DYNAMIC_LAMBDA_PROPERTY_VALUE: 'original value',
USE_IMPORT_VALUE_LAMBDA_PROPERTY: 'true',
},
});

// WHEN
const deployOutput = await fixture.cdkDeploy('lambda-hotswap', {
options: ['--hotswap'],
captureStderr: true,
onlyStderr: true,
modEnv: {
DYNAMIC_LAMBDA_PROPERTY_VALUE: 'new value',
USE_IMPORT_VALUE_LAMBDA_PROPERTY: 'true',
},
});

const response = await fixture.aws.cloudFormation('describeStacks', {
StackName: stackArn,
});
const functionName = response.Stacks?.[0].Outputs?.[0].OutputValue;

// THEN

// The deployment should not trigger a full deployment, thus the stack's status must remains
// "CREATE_COMPLETE"
expect(response.Stacks?.[0].StackStatus).toEqual('CREATE_COMPLETE');
expect(deployOutput).toContain(`Lambda Function '${functionName}' hotswapped!`);

} finally {
// Ensure cleanup in reverse order due to use of import/export
await fixture.cdkDestroy('lambda-hotswap');
await fixture.cdkDestroy('export-value-stack');
}
}));

async function listChildren(parent: string, pred: (x: string) => Promise<boolean>) {
const ret = new Array<string>();
for (const child of await fs.readdir(parent, { encoding: 'utf-8' })) {
Expand Down
12 changes: 12 additions & 0 deletions packages/aws-cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,18 @@ and might have breaking changes in the future.

**⚠ Note #3**: Expected defaults for certain parameters may be different with the hotswap parameter. For example, an ECS service's minimum healthy percentage will currently be set to 0. Please review the source accordingly if this occurs.

**⚠ Note #4**: Only usage of certain [CloudFormation intrinsic functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html) are supported as part of a hotswapped deployment. At time of writing, these are:
vinayak-kukreja marked this conversation as resolved.
Show resolved Hide resolved

- `Ref`
- `Fn::GetAtt` *
- `Fn::ImportValue`
- `Fn::Join`
- `Fn::Select`
- `Fn::Split`
- `Fn::Sub`

> *: `Fn::GetAtt` is only partially supported. Refer to [this implementation](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts#L477-L492) for supported resources and attributes.

### `cdk watch`

The `watch` command is similar to `deploy`,
Expand Down
78 changes: 76 additions & 2 deletions packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as AWS from 'aws-sdk';
import { PromiseResult } from 'aws-sdk/lib/request';
import { ISDK } from './aws-auth';
import { NestedStackNames } from './nested-stack-helpers';

Expand Down Expand Up @@ -34,7 +35,57 @@ export class LazyListStackResources implements ListStackResources {
}
}

export class CfnEvaluationException extends Error {}
export interface LookupExport {
lookupExport(name: string): Promise<AWS.CloudFormation.Export | undefined>;
}

export class LookupExportError extends Error { }

export class LazyLookupExport implements LookupExport {
private cachedExports: { [name: string]: AWS.CloudFormation.Export } = {}

constructor(private readonly sdk: ISDK) { }

async lookupExport(name: string): Promise<AWS.CloudFormation.Export | undefined> {
if (this.cachedExports[name]) {
return this.cachedExports[name];
}

for await (const cfnExport of this.listExports()) {
if (!cfnExport.Name) {
continue; // ignore any result that omits a name
}
this.cachedExports[cfnExport.Name] = cfnExport;

if (cfnExport.Name === name) {
return cfnExport;
}
vinayak-kukreja marked this conversation as resolved.
Show resolved Hide resolved

}

return undefined; // export not found
}

private async * listExports() {
let nextToken: string | undefined = undefined;
while (true) {
const response: PromiseResult<AWS.CloudFormation.ListExportsOutput, AWS.AWSError> = await this.sdk.cloudFormation().listExports({
NextToken: nextToken,
}).promise();

for (const cfnExport of response.Exports ?? []) {
yield cfnExport;
}

if (!response.NextToken) {
return;
}
nextToken = response.NextToken;
}
}
}

export class CfnEvaluationException extends Error { }

export interface ResourceDefinition {
readonly LogicalId: string;
Expand Down Expand Up @@ -64,7 +115,8 @@ export class EvaluateCloudFormationTemplate {
private readonly urlSuffix: (region: string) => string;
private readonly sdk: ISDK;
private readonly nestedStackNames: { [nestedStackLogicalId: string]: NestedStackNames };
private readonly stackResources: LazyListStackResources;
private readonly stackResources: ListStackResources;
private readonly lookupExport: LookupExport;

private cachedUrlSuffix: string | undefined;

Expand All @@ -90,6 +142,9 @@ export class EvaluateCloudFormationTemplate {
// We need them to figure out the physical name of a resource in case it wasn't specified by the user.
// We fetch it lazily, to save a service call, in case all hotswapped resources have their physical names set.
this.stackResources = new LazyListStackResources(this.sdk, this.stackName);

// CloudFormation Exports lookup to be able to resolve Fn::ImportValue intrinsics in template
this.lookupExport = new LazyLookupExport(this.sdk);
}

// clones current EvaluateCloudFormationTemplate object, but updates the stack name
Expand Down Expand Up @@ -152,6 +207,14 @@ export class EvaluateCloudFormationTemplate {

public async evaluateCfnExpression(cfnExpression: any): Promise<any> {
const self = this;
/**
* Evaluates CloudFormation intrinsic functions
*
* Note that supported intrinsic functions are documented in README.md -- please update
* list of supported functions when adding new evaluations
*
* See: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html
*/
class CfnIntrinsics {
public evaluateIntrinsic(intrinsic: Intrinsic): any {
const intrinsicFunc = (this as any)[intrinsic.name];
Expand Down Expand Up @@ -214,6 +277,17 @@ export class EvaluateCloudFormationTemplate {
}
});
}

async 'Fn::ImportValue'(name: string): Promise<string> {
const exported = await self.lookupExport.lookupExport(name);
if (!exported) {
throw new CfnEvaluationException(`Export '${name}' could not be found for evaluation`);
}
if (!exported.Value) {
throw new CfnEvaluationException(`Export '${name}' exists without a value`);
}
return exported.Value;
}
}

if (cfnExpression == null) {
Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk/lib/api/hotswap-deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type HotswapDetector = (
logicalId: string, change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate
) => Promise<ChangeHotswapResult>;

const RESOURCE_DETECTORS: { [key:string]: HotswapDetector } = {
const RESOURCE_DETECTORS: { [key: string]: HotswapDetector } = {
// Lambda
'AWS::Lambda::Function': isHotswappableLambdaFunctionChange,
'AWS::Lambda::Version': isHotswappableLambdaFunctionChange,
Expand Down Expand Up @@ -247,8 +247,8 @@ async function findNestedHotswappableChanges(
/** Returns 'true' if a pair of changes is for the same resource. */
function changesAreForSameResource(oldChange: cfn_diff.ResourceDifference, newChange: cfn_diff.ResourceDifference): boolean {
return oldChange.oldResourceType === newChange.newResourceType &&
// this isn't great, but I don't want to bring in something like underscore just for this comparison
JSON.stringify(oldChange.oldProperties) === JSON.stringify(newChange.newProperties);
// this isn't great, but I don't want to bring in something like underscore just for this comparison
JSON.stringify(oldChange.oldProperties) === JSON.stringify(newChange.newProperties);
}

function makeRenameDifference(
Expand Down Expand Up @@ -371,7 +371,7 @@ function logNonHotswappableChanges(nonHotswappableChanges: NonHotswappableChange

for (const change of nonHotswappableChanges) {
change.rejectedChanges.length > 0 ?
print(' logicalID: %s, type: %s, rejected changes: %s, reason: %s', chalk.bold(change.logicalId), chalk.bold(change.resourceType), chalk.bold(change.rejectedChanges), chalk.red(change.reason)):
print(' logicalID: %s, type: %s, rejected changes: %s, reason: %s', chalk.bold(change.logicalId), chalk.bold(change.resourceType), chalk.bold(change.rejectedChanges), chalk.red(change.reason)) :
print(' logicalID: %s, type: %s, reason: %s', chalk.bold(change.logicalId), chalk.bold(change.resourceType), chalk.red(change.reason));
}

Expand Down
110 changes: 110 additions & 0 deletions packages/aws-cdk/test/api/evaluate-cloudformation-template.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import {
CfnEvaluationException,
EvaluateCloudFormationTemplate,
Template,
} from '../../lib/api/evaluate-cloudformation-template';
import { MockSdk } from '../util/mock-sdk';

const listStackResources = jest.fn();
const listExports: jest.Mock<AWS.CloudFormation.ListExportsOutput, AWS.CloudFormation.ListExportsInput[]> = jest.fn();
const sdk = new MockSdk();
sdk.stubCloudFormation({
listExports,
listStackResources,
});

const createEvaluateCloudFormationTemplate = (template: Template) => new EvaluateCloudFormationTemplate({
template,
parameters: {},
account: '0123456789',
region: 'ap-south-east-2',
partition: 'aws',
urlSuffix: (region) => sdk.getEndpointSuffix(region),
sdk,
stackName: 'test-stack',
});

describe('evaluateCfnExpression', () => {
describe('simple literal expressions', () => {
const template: Template = {};
const evaluateCfnTemplate = createEvaluateCloudFormationTemplate(template);

test('resolves Fn::Join correctly', async () => {
// WHEN
const result = await evaluateCfnTemplate.evaluateCfnExpression({
'Fn::Join': [':', ['a', 'b', 'c']],
});

// THEN
expect(result).toEqual('a:b:c');
});

test('resolves Fn::Split correctly', async () => {
// WHEN
const result = await evaluateCfnTemplate.evaluateCfnExpression({ 'Fn::Split': ['|', 'a|b|c'] });

// THEN
expect(result).toEqual(['a', 'b', 'c']);
});

test('resolves Fn::Select correctly', async () => {
// WHEN
const result = await evaluateCfnTemplate.evaluateCfnExpression({ 'Fn::Select': ['1', ['apples', 'grapes', 'oranges', 'mangoes']] });

// THEN
expect(result).toEqual('grapes');
});

test('resolves Fn::Sub correctly', async () => {
// WHEN
const result = await evaluateCfnTemplate.evaluateCfnExpression({ 'Fn::Sub': ['Testing Fn::Sub Foo=${Foo} Bar=${Bar}', { Foo: 'testing', Bar: 1 }] });

// THEN
expect(result).toEqual('Testing Fn::Sub Foo=testing Bar=1');
});
});

describe('resolving Fn::ImportValue', () => {
const template: Template = {};
const evaluateCfnTemplate = createEvaluateCloudFormationTemplate(template);

const createMockExport = (num: number) => ({
ExportingStackId: `test-exporting-stack-id-${num}`,
Name: `test-name-${num}`,
Value: `test-value-${num}`,
});

beforeEach(async () => {
listExports.mockReset();
listExports
.mockReturnValueOnce({
Exports: [
createMockExport(1),
createMockExport(2),
createMockExport(3),
],
NextToken: 'next-token-1',
})
.mockReturnValueOnce({
Exports: [
createMockExport(4),
createMockExport(5),
createMockExport(6),
],
NextToken: undefined,
});
});

test('resolves Fn::ImportValue using lookup', async () => {
const result = await evaluateCfnTemplate.evaluateCfnExpression({ 'Fn::ImportValue': 'test-name-5' });
expect(result).toEqual('test-value-5');
});

test('throws error when Fn::ImportValue cannot be resolved', async () => {
const evaluate = () => evaluateCfnTemplate.evaluateCfnExpression({
'Fn::ImportValue': 'blah',
});
await expect(evaluate).rejects.toBeInstanceOf(CfnEvaluationException);
});
});
});
Loading
Loading