diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts index e09b5d714af69..9169e155bd696 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts @@ -257,8 +257,7 @@ export class Alarm extends AlarmBase { return dispatchMetric(metric, { withStat(stat, conf) { self.validateMetricStat(stat, metric); - const canRenderAsLegacyMetric = conf.renderingProperties?.label == undefined && - (stat.account == undefined || Stack.of(self).account == stat.account); + const canRenderAsLegacyMetric = conf.renderingProperties?.label == undefined && !self.requiresAccountId(stat); // Do this to disturb existing templates as little as possible if (canRenderAsLegacyMetric) { return dropUndefined({ @@ -286,7 +285,7 @@ export class Alarm extends AlarmBase { unit: stat.unitFilter, }, id: 'm1', - accountId: stat.account, + accountId: self.requiresAccountId(stat) ? stat.account : undefined, label: conf.renderingProperties?.label, returnData: true, } as CfnAlarm.MetricDataQueryProperty, @@ -321,7 +320,7 @@ export class Alarm extends AlarmBase { unit: stat.unitFilter, }, id: entry.id || uniqueMetricId(), - accountId: stat.account, + accountId: self.requiresAccountId(stat) ? stat.account : undefined, label: conf.renderingProperties?.label, returnData: entry.tag ? undefined : false, // entry.tag evaluates to true if the metric is the math expression the alarm is based on. }; @@ -370,6 +369,32 @@ export class Alarm extends AlarmBase { throw new Error('Cannot create an Alarm based on a MathExpression which specifies a searchAccount or searchRegion'); } } + + /** + * Determine if the accountId property should be included in the metric. + */ + private requiresAccountId(stat: MetricStatConfig): boolean { + const stackAccount = Stack.of(this).account; + + // if stat.account is undefined, it's by definition in the same account + if (stat.account === undefined) { + return false; + } + + // if this is a region-agnostic stack, we can't assume anything about stat.account + // and therefore we assume its a cross-account call + if (Token.isUnresolved(stackAccount)) { + return true; + } + + // ok, we can compare the two concrete values directly - if they are the same we + // can omit the account ID from the metric. + if (stackAccount === stat.account) { + return false; + } + + return true; + } } function definitelyDifferent(x: string | undefined, y: string) { diff --git a/packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts b/packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts index afb2224eb2f50..dcde88284aadd 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts @@ -9,9 +9,7 @@ const testMetric = new Metric({ }); describe('Alarm', () => { - test('alarm does not accept a math expression with more than 10 metrics', () => { - const stack = new Stack(); const usingMetrics: Record = {}; @@ -30,19 +28,15 @@ describe('Alarm', () => { }); expect(() => { - new Alarm(stack, 'Alarm', { metric: math, threshold: 1000, evaluationPeriods: 3, }); - }).toThrow(/Alarms on math expressions cannot contain more than 10 individual metrics/); - - }); - test('non ec2 instance related alarm does not accept EC2 action', () => { + test('non ec2 instance related alarm does not accept EC2 action', () => { const stack = new Stack(); const alarm = new Alarm(stack, 'Alarm', { metric: testMetric, @@ -53,8 +47,8 @@ describe('Alarm', () => { expect(() => { alarm.addAlarmAction(new Ec2TestAlarmAction('arn:aws:automate:us-east-1:ec2:reboot')); }).toThrow(/EC2 alarm actions requires an EC2 Per-Instance Metric. \(.+ does not have an 'InstanceId' dimension\)/); - }); + test('can make simple alarm', () => { // GIVEN const stack = new Stack(); @@ -76,8 +70,6 @@ describe('Alarm', () => { Statistic: 'Average', Threshold: 1000, }); - - }); test('override metric period in Alarm', () => { @@ -102,8 +94,6 @@ describe('Alarm', () => { Statistic: 'Average', Threshold: 1000, }); - - }); test('override statistic Alarm', () => { @@ -129,8 +119,6 @@ describe('Alarm', () => { ExtendedStatistic: Match.absent(), Threshold: 1000, }); - - }); test('can use percentile in Alarm', () => { @@ -156,8 +144,6 @@ describe('Alarm', () => { ExtendedStatistic: 'p99', Threshold: 1000, }); - - }); test('can set DatapointsToAlarm', () => { @@ -183,8 +169,6 @@ describe('Alarm', () => { Statistic: 'Average', Threshold: 1000, }); - - }); test('can add actions to alarms', () => { @@ -208,8 +192,6 @@ describe('Alarm', () => { InsufficientDataActions: ['B'], OKActions: ['C'], }); - - }); test('can make alarm directly from metric', () => { @@ -234,8 +216,6 @@ describe('Alarm', () => { Statistic: 'Minimum', Threshold: 1000, }); - - }); test('can use percentile string to make alarm', () => { @@ -253,8 +233,6 @@ describe('Alarm', () => { Template.fromStack(stack).hasResourceProperties('AWS::CloudWatch::Alarm', { ExtendedStatistic: 'p99.9', }); - - }); test('can use a generic string for extended statistic to make alarm', () => { @@ -273,9 +251,7 @@ describe('Alarm', () => { Statistic: Match.absent(), ExtendedStatistic: 'tm99.9999999999', }); - }); - }); class TestAlarmAction implements IAlarmAction { diff --git a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts index 668807c89bfef..61b006e9d8f89 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts @@ -1,4 +1,4 @@ -import { Template } from '@aws-cdk/assertions'; +import { Match, Template } from '@aws-cdk/assertions'; import { Duration, Stack } from '@aws-cdk/core'; import { Alarm, GraphWidget, IWidget, MathExpression, Metric } from '../lib'; @@ -7,11 +7,13 @@ const a = new Metric({ namespace: 'Test', metricName: 'ACount' }); let stack1: Stack; let stack2: Stack; let stack3: Stack; +let stack4: Stack; describe('cross environment', () => { beforeEach(() => { stack1 = new Stack(undefined, undefined, { env: { region: 'pluto', account: '1234' } }); stack2 = new Stack(undefined, undefined, { env: { region: 'mars', account: '5678' } }); stack3 = new Stack(undefined, undefined, { env: { region: 'pluto', account: '0000' } }); + stack4 = new Stack(undefined, undefined); }); describe('in graphs', () => { @@ -124,12 +126,10 @@ describe('cross environment', () => { Namespace: 'Test', Period: 300, }); - - }); test('metric attached to stack1 will throw in stack2', () => { - // Cross-region/cross-account metrics are supported in Dashboards but not in Alarms + // Cross-region metrics are supported in Dashboards but not in Alarms // GIVEN expect(() => { @@ -139,8 +139,6 @@ describe('cross environment', () => { metric: a.attachTo(stack1), }); }).toThrow(/Cannot create an Alarm in region 'mars' based on metric 'ACount' in 'pluto'/); - - }); test('metric attached to stack3 will render in stack1', () => { @@ -207,12 +205,49 @@ describe('cross environment', () => { }); }); + test('metric from same account as stack will not have accountId', () => { + // GIVEN + + // including label property will force Alarm configuration to "modern" config. + const b = new Metric({ + namespace: 'Test', + metricName: 'ACount', + label: 'my-label', + }); + + new Alarm(stack1, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric: b, + }); + + // THEN + Template.fromStack(stack1).hasResourceProperties('AWS::CloudWatch::Alarm', { + Metrics: [ + { + AccountId: Match.absent(), + Id: 'm1', + Label: 'my-label', + MetricStat: { + Metric: { + MetricName: 'ACount', + Namespace: 'Test', + }, + Period: 300, + Stat: 'Average', + }, + ReturnData: true, + }, + ], + }); + }); + test('math expression can render in a different account', () => { // GIVEN const b = new Metric({ namespace: 'Test', metricName: 'ACount', - account: '1234', + account: '5678', }); const c = new MathExpression({ @@ -248,7 +283,64 @@ describe('cross environment', () => { ReturnData: false, }, { - AccountId: '1234', + AccountId: '5678', + Id: 'b', + MetricStat: { + Metric: { + MetricName: 'ACount', + Namespace: 'Test', + }, + Period: 60, + Stat: 'Average', + }, + ReturnData: false, + }, + ], + }); + }); + + test('math expression from same account as stack will not have accountId', () => { + // GIVEN + const b = new Metric({ + namespace: 'Test', + metricName: 'ACount', + account: '1234', + }); + + const c = new MathExpression({ + expression: 'a + b', + usingMetrics: { a: a.attachTo(stack1), b }, + period: Duration.minutes(1), + }); + + new Alarm(stack1, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric: c, + }); + + // THEN + Template.fromStack(stack1).hasResourceProperties('AWS::CloudWatch::Alarm', { + Metrics: [ + { + Expression: 'a + b', + Id: 'expr_1', + }, + { + AccountId: Match.absent(), + Id: 'a', + MetricStat: { + Metric: { + MetricName: 'ACount', + Namespace: 'Test', + }, + Period: 60, + Stat: 'Average', + }, + ReturnData: false, + }, + { + AccountId: Match.absent(), Id: 'b', MetricStat: { Metric: { @@ -289,7 +381,7 @@ describe('cross environment', () => { }).toThrow(/Cannot create an Alarm based on a MathExpression which specifies a searchAccount or searchRegion/); }); - test('match expression with different searchRegion will throw', () => { + test('math expression with different searchRegion will throw', () => { // GIVEN const b = new Metric({ namespace: 'Test', @@ -313,6 +405,107 @@ describe('cross environment', () => { }); }).toThrow(/Cannot create an Alarm based on a MathExpression which specifies a searchAccount or searchRegion/); }); + + describe('accountId requirements', () => { + test('metric account is not defined', () => { + const metric = new Metric({ + namespace: 'Test', + metricName: 'ACount', + }); + + new Alarm(stack4, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric, + }); + + // Alarm will be defined as legacy alarm. + Template.fromStack(stack4).hasResourceProperties('AWS::CloudWatch::Alarm', { + Threshold: 1, + EvaluationPeriods: 1, + MetricName: 'ACount', + Namespace: 'Test', + }); + }); + + test('metric account is defined and stack account is token', () => { + const metric = new Metric({ + namespace: 'Test', + metricName: 'ACount', + account: '123456789', + }); + + new Alarm(stack4, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric, + }); + + // Alarm will be defined as modern alarm. + Template.fromStack(stack4).hasResourceProperties('AWS::CloudWatch::Alarm', { + Metrics: Match.anyValue(), + }); + }); + + test('metric account is attached to stack account', () => { + const metric = new Metric({ + namespace: 'Test', + metricName: 'ACount', + }); + + new Alarm(stack4, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric: metric.attachTo(stack4), + }); + + // Alarm will be defined as legacy alarm. + Template.fromStack(stack4).hasResourceProperties('AWS::CloudWatch::Alarm', { + Threshold: 1, + EvaluationPeriods: 1, + MetricName: 'ACount', + Namespace: 'Test', + }); + }); + + test('metric account === stack account, but both are tokens', () => { + const metric = new Metric({ + namespace: 'Test', + metricName: 'ACount', + account: stack4.account, + }); + + new Alarm(stack4, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric, + }); + + // Alarm will be defined as modern alarm, since there is no way of knowing that the two tokens are equal. + Template.fromStack(stack4).hasResourceProperties('AWS::CloudWatch::Alarm', { + Metrics: Match.anyValue(), + }); + }); + + test('metric account !== stack account', () => { + const metric = new Metric({ + namespace: 'Test', + metricName: 'ACount', + account: '123456789', + }); + + new Alarm(stack1, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric, + }); + + // Alarm will be defined as modern alarm. + Template.fromStack(stack1).hasResourceProperties('AWS::CloudWatch::Alarm', { + Metrics: Match.anyValue(), + }); + }); + }); }); }); diff --git a/packages/@aws-cdk/aws-s3/README.md b/packages/@aws-cdk/aws-s3/README.md index 60f97010b9a20..dab9af9a9d7f8 100644 --- a/packages/@aws-cdk/aws-s3/README.md +++ b/packages/@aws-cdk/aws-s3/README.md @@ -447,6 +447,6 @@ const bucket = new Bucket(this, 'MyTempFileBucket', { ``` **Warning** if you have deployed a bucket with `autoDeleteObjects: true`, -switching this to `false` in a CDK version *before* `1.126.0` will lead to all -objects in the bucket being deleted. Be sure to update to version `1.126.0` or -later before switching this value to `false`. +switching this to `false` in a CDK version *before* `1.126.0` will lead to +all objects in the bucket being deleted. Be sure to update your bucket resources +by deploying with CDK version `1.126.0` or later **before** switching this value to `false`. diff --git a/packages/@aws-cdk/aws-s3/lib/auto-delete-objects-handler/index.ts b/packages/@aws-cdk/aws-s3/lib/auto-delete-objects-handler/index.ts index fed602825c6a0..2459d44ab1d18 100644 --- a/packages/@aws-cdk/aws-s3/lib/auto-delete-objects-handler/index.ts +++ b/packages/@aws-cdk/aws-s3/lib/auto-delete-objects-handler/index.ts @@ -58,7 +58,14 @@ async function onDelete(bucketName?: string) { process.stdout.write(`Bucket does not have '${AUTO_DELETE_OBJECTS_TAG}' tag, skipping cleaning.\n`); return; } - await emptyBucket(bucketName); + try { + await emptyBucket(bucketName); + } catch (e) { + if (e.code !== 'NoSuchBucket') { + throw e; + } + // Bucket doesn't exist. Ignoring + } } /** diff --git a/packages/@aws-cdk/aws-s3/test/auto-delete-objects-handler.test.ts b/packages/@aws-cdk/aws-s3/test/auto-delete-objects-handler.test.ts index 51cc65b3d5466..b4b7f523faef2 100644 --- a/packages/@aws-cdk/aws-s3/test/auto-delete-objects-handler.test.ts +++ b/packages/@aws-cdk/aws-s3/test/auto-delete-objects-handler.test.ts @@ -287,6 +287,23 @@ test('delete event where bucket has many objects does recurse appropriately', as }); }); +test('does nothing when the bucket does not exist', async () => { + // GIVEN + mockS3Client.promise.mockRejectedValue({ code: 'NoSuchBucket' }); + + // WHEN + const event: Partial = { + RequestType: 'Delete', + ResourceProperties: { + ServiceToken: 'Foo', + BucketName: 'MyBucket', + }, + }; + await invokeHandler(event); + + expect(mockS3Client.deleteObjects).not.toHaveBeenCalled(); +}); + // helper function to get around TypeScript expecting a complete event object, // even though our tests only need some of the fields async function invokeHandler(event: Partial) { diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 3a1b2882b2506..9690c6f9d9258 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -362,6 +362,7 @@ Hotswapping is currently supported for the following changes (additional changes will be supported in the future): - Code asset changes of AWS Lambda functions. +- Definition changes of AWS Step Functions State Machines. **⚠ Note #1**: This command deliberately introduces drift in CloudFormation stacks in order to speed up deployments. For this reason, only use it for development purposes. diff --git a/packages/aws-cdk/lib/api/aws-auth/sdk.ts b/packages/aws-cdk/lib/api/aws-auth/sdk.ts index 0f1468b4d08b8..9090a59c8d792 100644 --- a/packages/aws-cdk/lib/api/aws-auth/sdk.ts +++ b/packages/aws-cdk/lib/api/aws-auth/sdk.ts @@ -32,6 +32,7 @@ export interface ISDK { elbv2(): AWS.ELBv2; secretsManager(): AWS.SecretsManager; kms(): AWS.KMS; + stepFunctions(): AWS.StepFunctions; } /** @@ -128,6 +129,10 @@ export class SDK implements ISDK { return this.wrapServiceErrorHandling(new AWS.KMS(this.config)); } + public stepFunctions(): AWS.StepFunctions { + return this.wrapServiceErrorHandling(new AWS.StepFunctions(this.config)); + } + public async currentAccount(): Promise { // Get/refresh if necessary before we can access `accessKeyId` await this.forceCredentialRetrieval(); diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index 5812e50a605ef..7f002b6c684cb 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -3,9 +3,10 @@ import * as cxapi from '@aws-cdk/cx-api'; import { CloudFormation } from 'aws-sdk'; import { ISDK, Mode, SdkProvider } from './aws-auth'; import { DeployStackResult } from './deploy-stack'; -import { ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation, ListStackResources } from './hotswap/common'; +import { ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation, ListStackResources, HotswappableChangeCandidate } from './hotswap/common'; import { EvaluateCloudFormationTemplate } from './hotswap/evaluate-cloudformation-template'; import { isHotswappableLambdaFunctionChange } from './hotswap/lambda-functions'; +import { isHotswappableStateMachineChange } from './hotswap/stepfunctions-state-machines'; import { CloudFormationStack } from './util/cloudformation'; /** @@ -57,24 +58,80 @@ export async function tryHotswapDeployment( async function findAllHotswappableChanges( stackChanges: cfn_diff.TemplateDiff, evaluateCfnTemplate: EvaluateCloudFormationTemplate, ): Promise { - const promises = new Array>(); - stackChanges.resources.forEachDifference(async (logicalId: string, change: cfn_diff.ResourceDifference) => { - promises.push(isHotswappableLambdaFunctionChange(logicalId, change, evaluateCfnTemplate)); + let foundNonHotswappableChange = false; + const promises: Array>> = []; + + // gather the results of the detector functions + stackChanges.resources.forEachDifference((logicalId: string, change: cfn_diff.ResourceDifference) => { + const resourceHotswapEvaluation = isCandidateForHotswapping(change); + + if (resourceHotswapEvaluation === ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT) { + foundNonHotswappableChange = true; + } else if (resourceHotswapEvaluation === ChangeHotswapImpact.IRRELEVANT) { + // empty 'if' just for flow-aware typing to kick in... + } else { + promises.push([ + isHotswappableLambdaFunctionChange(logicalId, resourceHotswapEvaluation, evaluateCfnTemplate), + isHotswappableStateMachineChange(logicalId, resourceHotswapEvaluation, evaluateCfnTemplate), + ]); + } }); - return Promise.all(promises).then(hotswapDetectionResults => { - const hotswappableResources = new Array(); - let foundNonHotswappableChange = false; - for (const lambdaFunctionShortCircuitChange of hotswapDetectionResults) { - if (lambdaFunctionShortCircuitChange === ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT) { + + const changesDetectionResults: Array> = []; + for (const detectorResultPromises of promises) { + const hotswapDetectionResults = await Promise.all(detectorResultPromises); + changesDetectionResults.push(hotswapDetectionResults); + } + + const hotswappableResources = new Array(); + + // resolve all detector results + for (const hotswapDetectionResults of changesDetectionResults) { + const perChangeHotswappableResources = new Array(); + + for (const result of hotswapDetectionResults) { + if (typeof result !== 'string') { + perChangeHotswappableResources.push(result); + } + } + + // if we found any hotswappable changes, return now + if (perChangeHotswappableResources.length > 0) { + hotswappableResources.push(...perChangeHotswappableResources); + continue; + } + + // no hotswappable changes found, so any REQUIRES_FULL_DEPLOYMENTs imply a non-hotswappable change + for (const result of hotswapDetectionResults) { + if (result === ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT) { foundNonHotswappableChange = true; - } else if (lambdaFunctionShortCircuitChange === ChangeHotswapImpact.IRRELEVANT) { - // empty 'if' just for flow-aware typing to kick in... - } else { - hotswappableResources.push(lambdaFunctionShortCircuitChange); } } - return foundNonHotswappableChange ? undefined : hotswappableResources; - }); + // no REQUIRES_FULL_DEPLOYMENT implies that all results are IRRELEVANT + } + + return foundNonHotswappableChange ? undefined : hotswappableResources; +} + +/** + * returns `ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT` if a resource was deleted, or a change that we cannot short-circuit occured. + * Returns `ChangeHotswapImpact.IRRELEVANT` if a change that does not impact shortcircuiting occured, such as a metadata change. + */ +export function isCandidateForHotswapping(change: cfn_diff.ResourceDifference): HotswappableChangeCandidate | ChangeHotswapImpact { + // a resource has been removed OR a resource has been added; we can't short-circuit that change + if (!change.newValue || !change.oldValue) { + return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; + } + + // Ignore Metadata changes + if (change.newValue.Type === 'AWS::CDK::Metadata') { + return ChangeHotswapImpact.IRRELEVANT; + } + + return { + newValue: change.newValue, + propertyUpdates: change.propertyUpdates, + }; } async function applyAllHotswappableChanges( diff --git a/packages/aws-cdk/lib/api/hotswap/common.ts b/packages/aws-cdk/lib/api/hotswap/common.ts index c11b29d1d7daa..1e482d112aef4 100644 --- a/packages/aws-cdk/lib/api/hotswap/common.ts +++ b/packages/aws-cdk/lib/api/hotswap/common.ts @@ -1,6 +1,7 @@ import * as cfn_diff from '@aws-cdk/cloudformation-diff'; import { CloudFormation } from 'aws-sdk'; import { ISDK } from '../aws-auth'; +import { CfnEvaluationException, EvaluateCloudFormationTemplate } from './evaluate-cloudformation-template'; export interface ListStackResources { listStackResources(): Promise; @@ -33,6 +34,43 @@ export enum ChangeHotswapImpact { export type ChangeHotswapResult = HotswapOperation | ChangeHotswapImpact; -export function assetMetadataChanged(change: cfn_diff.ResourceDifference): boolean { +/** + * Represents a change that can be hotswapped. + */ +export class HotswappableChangeCandidate { + /** + * The value the resource is being updated to. + */ + public readonly newValue: cfn_diff.Resource; + + /** + * The changes made to the resource properties. + */ + public readonly propertyUpdates: { [key: string]: cfn_diff.PropertyDifference }; + + public constructor(newValue: cfn_diff.Resource, propertyUpdates: { [key: string]: cfn_diff.PropertyDifference }) { + this.newValue = newValue; + this.propertyUpdates = propertyUpdates; + } +} + +export async function establishResourcePhysicalName( + logicalId: string, physicalNameInCfnTemplate: any, evaluateCfnTemplate: EvaluateCloudFormationTemplate, +): Promise { + if (physicalNameInCfnTemplate != null) { + try { + return await evaluateCfnTemplate.evaluateCfnExpression(physicalNameInCfnTemplate); + } catch (e) { + // If we can't evaluate the resource's name CloudFormation expression, + // just look it up in the currently deployed Stack + if (!(e instanceof CfnEvaluationException)) { + throw e; + } + } + } + return evaluateCfnTemplate.findPhysicalNameFor(logicalId); +} + +export function assetMetadataChanged(change: HotswappableChangeCandidate): boolean { return !!change.newValue?.Metadata['aws:asset:path']; } diff --git a/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts b/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts index c110a66b6c9db..6aae68738acca 100644 --- a/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts +++ b/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts @@ -1,7 +1,6 @@ -import * as cfn_diff from '@aws-cdk/cloudformation-diff'; import { ISDK } from '../aws-auth'; -import { assetMetadataChanged, ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation } from './common'; -import { CfnEvaluationException, EvaluateCloudFormationTemplate } from './evaluate-cloudformation-template'; +import { assetMetadataChanged, ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation, HotswappableChangeCandidate, establishResourcePhysicalName } from './common'; +import { EvaluateCloudFormationTemplate } from './evaluate-cloudformation-template'; /** * Returns `false` if the change cannot be short-circuited, @@ -10,7 +9,7 @@ import { CfnEvaluationException, EvaluateCloudFormationTemplate } from './evalua * or a LambdaFunctionResource if the change can be short-circuited. */ export async function isHotswappableLambdaFunctionChange( - logicalId: string, change: cfn_diff.ResourceDifference, evaluateCfnTemplate: EvaluateCloudFormationTemplate, + logicalId: string, change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate, ): Promise { const lambdaCodeChange = await isLambdaFunctionCodeOnlyChange(change, evaluateCfnTemplate); if (typeof lambdaCodeChange === 'string') { @@ -24,7 +23,7 @@ export async function isHotswappableLambdaFunctionChange( return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; } - const functionName = await establishFunctionPhysicalName(logicalId, change, evaluateCfnTemplate); + const functionName = await establishResourcePhysicalName(logicalId, change.newValue.Properties?.FunctionName, evaluateCfnTemplate); if (!functionName) { return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; } @@ -37,34 +36,21 @@ export async function isHotswappableLambdaFunctionChange( } /** - * Returns `true` if the change is not for a AWS::Lambda::Function, + * Returns `ChangeHotswapImpact.IRRELEVANT` if the change is not for a AWS::Lambda::Function, * but doesn't prevent short-circuiting * (like a change to CDKMetadata resource), - * `false` if the change is to a AWS::Lambda::Function, + * `ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT` if the change is to a AWS::Lambda::Function, * but not only to its Code property, * or a LambdaFunctionCode if the change is to a AWS::Lambda::Function, * and only affects its Code property. */ async function isLambdaFunctionCodeOnlyChange( - change: cfn_diff.ResourceDifference, evaluateCfnTemplate: EvaluateCloudFormationTemplate, + change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate, ): Promise { - if (!change.newValue) { - return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; - } const newResourceType = change.newValue.Type; - // Ignore Metadata changes - if (newResourceType === 'AWS::CDK::Metadata') { - return ChangeHotswapImpact.IRRELEVANT; - } - // The only other resource change we should see is a Lambda function if (newResourceType !== 'AWS::Lambda::Function') { return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; } - if (change.oldValue?.Type == null) { - // this means this is a brand-new Lambda function - - // obviously, we can't short-circuit that! - return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; - } /* * On first glance, we would want to initialize these using the "previous" values (change.oldValue), @@ -83,9 +69,6 @@ async function isLambdaFunctionCodeOnlyChange( const propertyUpdates = change.propertyUpdates; for (const updatedPropName in propertyUpdates) { const updatedProp = propertyUpdates[updatedPropName]; - if (updatedProp.newValue === undefined) { - return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; - } for (const newPropName in updatedProp.newValue) { switch (newPropName) { case 'S3Bucket': @@ -132,21 +115,3 @@ class LambdaFunctionHotswapOperation implements HotswapOperation { }).promise(); } } - -async function establishFunctionPhysicalName( - logicalId: string, change: cfn_diff.ResourceDifference, evaluateCfnTemplate: EvaluateCloudFormationTemplate, -): Promise { - const functionNameInCfnTemplate = change.newValue?.Properties?.FunctionName; - if (functionNameInCfnTemplate != null) { - try { - return await evaluateCfnTemplate.evaluateCfnExpression(functionNameInCfnTemplate); - } catch (e) { - // If we can't evaluate the function's name CloudFormation expression, - // just look it up in the currently deployed Stack - if (!(e instanceof CfnEvaluationException)) { - throw e; - } - } - } - return evaluateCfnTemplate.findPhysicalNameFor(logicalId); -} diff --git a/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts b/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts new file mode 100644 index 0000000000000..c4a7a4eae8750 --- /dev/null +++ b/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts @@ -0,0 +1,62 @@ +import { ISDK } from '../aws-auth'; +import { ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation, HotswappableChangeCandidate, establishResourcePhysicalName } from './common'; +import { EvaluateCloudFormationTemplate } from './evaluate-cloudformation-template'; + +export async function isHotswappableStateMachineChange( + logicalId: string, change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate, +): Promise { + const stateMachineDefinitionChange = await isStateMachineDefinitionOnlyChange(change, evaluateCfnTemplate); + if (stateMachineDefinitionChange === ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT || + stateMachineDefinitionChange === ChangeHotswapImpact.IRRELEVANT) { + return stateMachineDefinitionChange; + } + + const machineNameInCfnTemplate = change.newValue?.Properties?.StateMachineName; + const machineName = await establishResourcePhysicalName(logicalId, machineNameInCfnTemplate, evaluateCfnTemplate); + if (!machineName) { + return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; + } + + return new StateMachineHotswapOperation({ + definition: stateMachineDefinitionChange, + stateMachineName: machineName, + }); +} + +async function isStateMachineDefinitionOnlyChange( + change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate, +): Promise { + const newResourceType = change.newValue.Type; + if (newResourceType !== 'AWS::StepFunctions::StateMachine') { + return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; + } + + const propertyUpdates = change.propertyUpdates; + for (const updatedPropName in propertyUpdates) { + // ensure that only changes to the definition string result in a hotswap + if (updatedPropName !== 'DefinitionString') { + return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; + } + } + + return evaluateCfnTemplate.evaluateCfnExpression(propertyUpdates.DefinitionString.newValue); +} + +interface StateMachineResource { + readonly stateMachineName: string; + readonly definition: string; +} + +class StateMachineHotswapOperation implements HotswapOperation { + constructor(private readonly stepFunctionResource: StateMachineResource) { + } + + public async apply(sdk: ISDK): Promise { + // not passing the optional properties leaves them unchanged + return sdk.stepFunctions().updateStateMachine({ + // even though the name of the property is stateMachineArn, passing the name of the state machine is allowed here + stateMachineArn: this.stepFunctionResource.stateMachineName, + definition: this.stepFunctionResource.definition, + }).promise(); + } +} diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts new file mode 100644 index 0000000000000..26a8d08c27290 --- /dev/null +++ b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts @@ -0,0 +1,242 @@ +import { Lambda, StepFunctions } from 'aws-sdk'; +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; + +beforeEach(() => { + cfnMockProvider = setup.setupHotswapTests(); + mockUpdateLambdaCode = jest.fn(); + mockUpdateMachineDefinition = jest.fn(); + cfnMockProvider.setUpdateFunctionCodeMock(mockUpdateLambdaCode); + cfnMockProvider.setUpdateStateMachineMock(mockUpdateMachineDefinition); +}); + +test('returns a deployStackResult with noOp=true when it receives an empty set of changes', async () => { + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(setup.cdkStackArtifactOf()); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(deployStackResult?.noOp).toBeTruthy(); + expect(deployStackResult?.stackArn).toEqual(setup.STACK_ID); +}); + +test('A change to only a non-hotswappable resource results in a full deployment', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + SomethingElse: { + Type: 'AWS::CloudFormation::SomethingElse', + Properties: { + Prop: 'old-value', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + SomethingElse: { + Type: 'AWS::CloudFormation::SomethingElse', + Properties: { + Prop: 'new-value', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).toBeUndefined(); + expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); + expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); +}); + +test('A change to both a hotswappable resource and a non-hotswappable resource results in a full deployment', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + SomethingElse: { + Type: 'AWS::CloudFormation::SomethingElse', + Properties: { + Prop: 'old-value', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + SomethingElse: { + Type: 'AWS::CloudFormation::SomethingElse', + Properties: { + Prop: 'new-value', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).toBeUndefined(); + expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); + expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); +}); + +test('changes only to CDK::Metadata result in a noOp', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + MetaData: { + Type: 'AWS::CDK::MetaData', + Properties: { + Prop: 'old-value', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + MetaData: { + Type: 'AWS::CDK::Metadata', + Properties: { + Prop: 'new-value', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(deployStackResult?.noOp).toEqual(true); + expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); + expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); +}); + +test('resource deletions require full deployments', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf(); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).toBeUndefined(); + expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); + expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); +}); + +test('can correctly reference AWS::Partition in hotswappable changes', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + FunctionName: { + 'Fn::Join': [ + '', + [ + { Ref: 'AWS::Partition' }, + '-', + 'my-function', + ], + ], + }, + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + FunctionName: { + 'Fn::Join': [ + '', + [ + { Ref: 'AWS::Partition' }, + '-', + 'my-function', + ], + ], + }, + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateLambdaCode).toHaveBeenCalledWith({ + FunctionName: 'aws-my-function', + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }); +}); diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts b/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts new file mode 100644 index 0000000000000..00d1a706a66e7 --- /dev/null +++ b/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts @@ -0,0 +1,95 @@ +import * as cxapi from '@aws-cdk/cx-api'; +import { CloudFormation } from 'aws-sdk'; +import * as lambda from 'aws-sdk/clients/lambda'; +import * as stepfunctions from 'aws-sdk/clients/stepfunctions'; +import { DeployStackResult } from '../../../lib'; +import * as deployments from '../../../lib/api/hotswap-deployments'; +import { Template } from '../../../lib/api/util/cloudformation'; +import { testStack, TestStackArtifact } from '../../util'; +import { MockSdkProvider } from '../../util/mock-sdk'; +import { FakeCloudformationStack } from '../fake-cloudformation-stack'; + +const STACK_NAME = 'withouterrors'; +export const STACK_ID = 'stackId'; + +let cfnMockProvider: CfnMockProvider; +let currentCfnStack: FakeCloudformationStack; +const currentCfnStackResources: CloudFormation.StackResourceSummary[] = []; + +export function setupHotswapTests() { + jest.resetAllMocks(); + // clear the array + currentCfnStackResources.splice(0); + cfnMockProvider = new CfnMockProvider(); + currentCfnStack = new FakeCloudformationStack({ + stackName: STACK_NAME, + stackId: STACK_ID, + }); + + return cfnMockProvider; +} + +export function cdkStackArtifactOf(testStackArtifact: Partial = {}): cxapi.CloudFormationStackArtifact { + return testStack({ + stackName: STACK_NAME, + ...testStackArtifact, + }); +} + +export function pushStackResourceSummaries(...items: CloudFormation.StackResourceSummary[]) { + currentCfnStackResources.push(...items); +} + +export function setCurrentCfnStackTemplate(template: Template) { + currentCfnStack.setTemplate(template); +} + +export function stackSummaryOf(logicalId: string, resourceType: string, physicalResourceId: string): CloudFormation.StackResourceSummary { + return { + LogicalResourceId: logicalId, + PhysicalResourceId: physicalResourceId, + ResourceType: resourceType, + ResourceStatus: 'CREATE_COMPLETE', + LastUpdatedTimestamp: new Date(), + }; +} + +export class CfnMockProvider { + private mockSdkProvider: MockSdkProvider; + + constructor() { + this.mockSdkProvider = new MockSdkProvider({ realSdk: false }); + + this.mockSdkProvider.stubCloudFormation({ + listStackResources: ({ StackName: stackName }) => { + if (stackName !== STACK_NAME) { + throw new Error(`Expected Stack name in listStackResources() call to be: '${STACK_NAME}', but received: ${stackName}'`); + } + return { + StackResourceSummaries: currentCfnStackResources, + }; + }, + }); + } + + public setUpdateStateMachineMock(mockUpdateMachineDefinition: + (input: stepfunctions.UpdateStateMachineInput) => + stepfunctions.UpdateStateMachineOutput) { + this.mockSdkProvider.stubStepFunctions({ + updateStateMachine: mockUpdateMachineDefinition, + }); + } + + public setUpdateFunctionCodeMock(mockUpdateLambdaCode: (input: lambda.UpdateFunctionCodeRequest) => lambda.FunctionConfiguration) { + this.mockSdkProvider.stubLambda({ + updateFunctionCode: mockUpdateLambdaCode, + }); + } + + public tryHotswapDeployment( + stackArtifact: cxapi.CloudFormationStackArtifact, + assetParams: { [key: string]: string } = {}, + ): Promise { + return deployments.tryHotswapDeployment(this.mockSdkProvider, assetParams, currentCfnStack, stackArtifact); + } +} diff --git a/packages/aws-cdk/test/api/hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/lambda-hotswap-deployments.test.ts similarity index 56% rename from packages/aws-cdk/test/api/hotswap-deployments.test.ts rename to packages/aws-cdk/test/api/hotswap/lambda-hotswap-deployments.test.ts index 5bb6ff8b7b466..1a81a8dc21ca8 100644 --- a/packages/aws-cdk/test/api/hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/lambda-hotswap-deployments.test.ts @@ -1,56 +1,18 @@ -import * as cxapi from '@aws-cdk/cx-api'; -import { CloudFormation, Lambda } from 'aws-sdk'; -import { tryHotswapDeployment } from '../../lib/api/hotswap-deployments'; -import { testStack, TestStackArtifact } from '../util'; -import { MockSdkProvider } from '../util/mock-sdk'; -import { FakeCloudformationStack } from './fake-cloudformation-stack'; +import { Lambda } from 'aws-sdk'; +import * as setup from './hotswap-test-setup'; -const STACK_NAME = 'withouterrors'; -const STACK_ID = 'stackId'; - -let mockSdkProvider: MockSdkProvider; let mockUpdateLambdaCode: (params: Lambda.Types.UpdateFunctionCodeRequest) => Lambda.Types.FunctionConfiguration; -let currentCfnStack: FakeCloudformationStack; -const currentCfnStackResources: CloudFormation.StackResourceSummary[] = []; +let cfnMockProvider: setup.CfnMockProvider; beforeEach(() => { - jest.resetAllMocks(); - mockSdkProvider = new MockSdkProvider({ realSdk: false }); + cfnMockProvider = setup.setupHotswapTests(); mockUpdateLambdaCode = jest.fn(); - mockSdkProvider.stubLambda({ - updateFunctionCode: mockUpdateLambdaCode, - }); - // clear the array - currentCfnStackResources.splice(0); - mockSdkProvider.stubCloudFormation({ - listStackResources: ({ StackName: stackName }) => { - if (stackName !== STACK_NAME) { - throw new Error(`Expected Stack name in listStackResources() call to be: '${STACK_NAME}', but received: ${stackName}'`); - } - return { - StackResourceSummaries: currentCfnStackResources, - }; - }, - }); - currentCfnStack = new FakeCloudformationStack({ - stackName: STACK_NAME, - stackId: STACK_ID, - }); -}); - -test('returns a deployStackResult with noOp=true when it receives an empty set of changes', async () => { - // WHEN - const deployStackResult = await tryHotswapDeployment(mockSdkProvider, {}, currentCfnStack, cdkStackArtifactOf()); - - // THEN - expect(deployStackResult).not.toBeUndefined(); - expect(deployStackResult?.noOp).toBeTruthy(); - expect(deployStackResult?.stackArn).toEqual(STACK_ID); + cfnMockProvider.setUpdateFunctionCodeMock(mockUpdateLambdaCode); }); -test('returns undefined when it a new Lambda function is added to the Stack', async () => { +test('returns undefined when a new Lambda function is added to the Stack', async () => { // GIVEN - const cdkStackArtifact = cdkStackArtifactOf({ + const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Resources: { Func: { @@ -61,7 +23,7 @@ test('returns undefined when it a new Lambda function is added to the Stack', as }); // WHEN - const deployStackResult = await tryHotswapDeployment(mockSdkProvider, {}, currentCfnStack, cdkStackArtifact); + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); // THEN expect(deployStackResult).toBeUndefined(); @@ -69,7 +31,7 @@ test('returns undefined when it a new Lambda function is added to the Stack', as test('calls the updateLambdaCode() API when it receives only a code difference in a Lambda function', async () => { // GIVEN - currentCfnStack.setTemplate({ + setup.setCurrentCfnStackTemplate({ Resources: { Func: { Type: 'AWS::Lambda::Function', @@ -86,7 +48,7 @@ test('calls the updateLambdaCode() API when it receives only a code difference i }, }, }); - const cdkStackArtifact = cdkStackArtifactOf({ + const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Resources: { Func: { @@ -107,7 +69,7 @@ test('calls the updateLambdaCode() API when it receives only a code difference i }); // WHEN - const deployStackResult = await tryHotswapDeployment(mockSdkProvider, {}, currentCfnStack, cdkStackArtifact); + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); // THEN expect(deployStackResult).not.toBeUndefined(); @@ -120,7 +82,7 @@ test('calls the updateLambdaCode() API when it receives only a code difference i test("correctly evaluates the function's name when it references a different resource from the template", async () => { // GIVEN - currentCfnStack.setTemplate({ + setup.setCurrentCfnStackTemplate({ Resources: { Bucket: { Type: 'AWS::S3::Bucket', @@ -146,8 +108,8 @@ test("correctly evaluates the function's name when it references a different res }, }, }); - currentCfnStackResources.push(stackSummaryOf('Bucket', 'AWS::S3::Bucket', 'mybucket')); - const cdkStackArtifact = cdkStackArtifactOf({ + setup.pushStackResourceSummaries(setup.stackSummaryOf('Bucket', 'AWS::S3::Bucket', 'mybucket')); + const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Resources: { Bucket: { @@ -177,7 +139,7 @@ test("correctly evaluates the function's name when it references a different res }); // WHEN - const deployStackResult = await tryHotswapDeployment(mockSdkProvider, {}, currentCfnStack, cdkStackArtifact); + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); // THEN expect(deployStackResult).not.toBeUndefined(); @@ -190,7 +152,7 @@ test("correctly evaluates the function's name when it references a different res test("correctly falls back to taking the function's name from the current stack if it can't evaluate it in the template", async () => { // GIVEN - currentCfnStack.setTemplate({ + setup.setCurrentCfnStackTemplate({ Parameters: { Param1: { Type: 'String' }, AssetBucketParam: { Type: 'String' }, @@ -211,8 +173,8 @@ test("correctly falls back to taking the function's name from the current stack }, }, }); - currentCfnStackResources.push(stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-function')); - const cdkStackArtifact = cdkStackArtifactOf({ + setup.pushStackResourceSummaries(setup.stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-function')); + const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Parameters: { Param1: { Type: 'String' }, @@ -237,9 +199,7 @@ test("correctly falls back to taking the function's name from the current stack }); // WHEN - const deployStackResult = await tryHotswapDeployment(mockSdkProvider, { - AssetBucketParam: 'asset-bucket', - }, currentCfnStack, cdkStackArtifact); + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact, { AssetBucketParam: 'asset-bucket' }); // THEN expect(deployStackResult).not.toBeUndefined(); @@ -252,7 +212,7 @@ test("correctly falls back to taking the function's name from the current stack test("will not perform a hotswap deployment if it cannot find a Ref target (outside the function's name)", async () => { // GIVEN - currentCfnStack.setTemplate({ + setup.setCurrentCfnStackTemplate({ Parameters: { Param1: { Type: 'String' }, }, @@ -271,8 +231,8 @@ test("will not perform a hotswap deployment if it cannot find a Ref target (outs }, }, }); - currentCfnStackResources.push(stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-func')); - const cdkStackArtifact = cdkStackArtifactOf({ + setup.pushStackResourceSummaries(setup.stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-func')); + const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Parameters: { Param1: { Type: 'String' }, @@ -296,13 +256,13 @@ test("will not perform a hotswap deployment if it cannot find a Ref target (outs // THEN await expect(() => - tryHotswapDeployment(mockSdkProvider, {}, currentCfnStack, cdkStackArtifact), + cfnMockProvider.tryHotswapDeployment(cdkStackArtifact), ).rejects.toThrow(/Parameter or resource 'Param1' could not be found for evaluation/); }); test("will not perform a hotswap deployment if it doesn't know how to handle a specific attribute (outside the function's name)", async () => { // GIVEN - currentCfnStack.setTemplate({ + setup.setCurrentCfnStackTemplate({ Resources: { Bucket: { Type: 'AWS::S3::Bucket', @@ -321,11 +281,11 @@ test("will not perform a hotswap deployment if it doesn't know how to handle a s }, }, }); - currentCfnStackResources.push( - stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-func'), - stackSummaryOf('Bucket', 'AWS::S3::Bucket', 'my-bucket'), + setup.pushStackResourceSummaries( + setup.stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-func'), + setup.stackSummaryOf('Bucket', 'AWS::S3::Bucket', 'my-bucket'), ); - const cdkStackArtifact = cdkStackArtifactOf({ + const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Resources: { Bucket: { @@ -349,23 +309,142 @@ test("will not perform a hotswap deployment if it doesn't know how to handle a s // THEN await expect(() => - tryHotswapDeployment(mockSdkProvider, {}, currentCfnStack, cdkStackArtifact), + cfnMockProvider.tryHotswapDeployment(cdkStackArtifact), ).rejects.toThrow("We don't support the 'UnknownAttribute' attribute of the 'AWS::S3::Bucket' resource. This is a CDK limitation. Please report it at https://github.com/aws/aws-cdk/issues/new/choose"); }); -function cdkStackArtifactOf(testStackArtifact: Partial = {}): cxapi.CloudFormationStackArtifact { - return testStack({ - stackName: STACK_NAME, - ...testStackArtifact, +test('calls the updateLambdaCode() API when it receives a code difference in a Lambda function with no name', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + }, + Metadata: { + 'aws:asset:path': 'current-path', + }, + }, + }, }); -} + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + }, + Metadata: { + 'aws:asset:path': 'current-path', + }, + }, + }, + }, + }); + + // WHEN + setup.pushStackResourceSummaries(setup.stackSummaryOf('Func', 'AWS::Lambda::Function', 'mock-function-resource-id')); + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateLambdaCode).toHaveBeenCalledWith({ + FunctionName: 'mock-function-resource-id', + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }); +}); -function stackSummaryOf(logicalId: string, resourceType: string, physicalResourceId: string): CloudFormation.StackResourceSummary { - return { - LogicalResourceId: logicalId, - PhysicalResourceId: physicalResourceId, - ResourceType: resourceType, - ResourceStatus: 'CREATE_COMPLETE', - LastUpdatedTimestamp: new Date(), - }; -} +test('does not call the updateLambdaCode() API when it receives a change that is not a code difference in a Lambda function', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + PackageType: 'Zip', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + PackageType: 'Image', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).toBeUndefined(); + expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); +}); + +test('does not call the updateLambdaCode() API when a resource with type that is not AWS::Lambda::Function but has the same properties is changed', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::NotLambda::NotAFunction', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::NotLambda::NotAFunction', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).toBeUndefined(); + expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); +}); diff --git a/packages/aws-cdk/test/api/hotswap/state-machine-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/state-machine-hotswap-deployments.test.ts new file mode 100644 index 0000000000000..5ece4a3621c38 --- /dev/null +++ b/packages/aws-cdk/test/api/hotswap/state-machine-hotswap-deployments.test.ts @@ -0,0 +1,483 @@ +import { StepFunctions } from 'aws-sdk'; +import * as setup from './hotswap-test-setup'; + +let mockUpdateMachineDefinition: (params: StepFunctions.Types.UpdateStateMachineInput) => StepFunctions.Types.UpdateStateMachineOutput; +let cfnMockProvider: setup.CfnMockProvider; + +beforeEach(() => { + cfnMockProvider = setup.setupHotswapTests(); + mockUpdateMachineDefinition = jest.fn(); + cfnMockProvider.setUpdateStateMachineMock(mockUpdateMachineDefinition); +}); + +test('returns undefined when a new StateMachine is added to the Stack', async () => { + // GIVEN + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).toBeUndefined(); +}); + +test('calls the updateStateMachine() API when it receives only a definitionString change without Fn::Join in a state machine', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: '{ Prop: "old-value" }', + StateMachineName: 'my-machine', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: '{ Prop: "new-value" }', + StateMachineName: 'my-machine', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ + definition: '{ Prop: "new-value" }', + stateMachineArn: 'my-machine', + }); +}); + +test('calls the updateStateMachine() API when it receives only a definitionString change with Fn::Join in a state machine', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { + 'Fn::Join': [ + '\n', + [ + '{', + ' "StartAt" : "SuccessState"', + ' "States" : {', + ' "SuccessState": {', + ' "Type": "Pass"', + ' "Result": "Success"', + ' "End": true', + ' }', + ' }', + '}', + ], + ], + }, + StateMachineName: 'my-machine', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { + 'Fn::Join': [ + '\n', + [ + '{', + ' "StartAt": "SuccessState",', + ' "States": {', + ' "SuccessState": {', + ' "Type": "Succeed"', + ' }', + ' }', + '}', + ], + ], + }, + StateMachineName: 'my-machine', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ + definition: JSON.stringify({ + StartAt: 'SuccessState', + States: { + SuccessState: { + Type: 'Succeed', + }, + }, + }, null, 2), + stateMachineArn: 'my-machine', + }); +}); + +test('calls the updateStateMachine() API when it receives a change to the definitionString in a state machine that has no name', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: '{ "Prop" : "old-value" }', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: '{ "Prop" : "new-value" }', + }, + }, + }, + }, + }); + + // WHEN + setup.pushStackResourceSummaries(setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'mock-machine-resource-id')); + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ + definition: '{ "Prop" : "new-value" }', + stateMachineArn: 'mock-machine-resource-id', // the sdk will convert the ID to the arn in a production environment + }); +}); + +test('does not call the updateStateMachine() API when it receives a change to a property that is not the definitionString in a state machine', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: '{ "Prop" : "old-value" }', + LoggingConfiguration: { // non-definitionString property + IncludeExecutionData: true, + }, + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: '{ "Prop" : "new-value" }', + LoggingConfiguration: { + IncludeExecutionData: false, + }, + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).toBeUndefined(); + expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); +}); + +test('does not call the updateStateMachine() API when a resource has a DefinitionString property but is not an AWS::StepFunctions::StateMachine is changed', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Machine: { + Type: 'AWS::NotStepFunctions::NotStateMachine', + Properties: { + DefinitionString: '{ Prop: "old-value" }', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Machine: { + Type: 'AWS::NotStepFunctions::NotStateMachine', + Properties: { + DefinitionString: '{ Prop: "new-value" }', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).toBeUndefined(); + expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); +}); + +test('can correctly hotswap old style synth changes', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Parameters: { AssetParam1: { Type: 'String' } }, + Resources: { + SM: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { Ref: 'AssetParam1' }, + StateMachineName: 'machine-name', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Parameters: { AssetParam2: { Type: String } }, + Resources: { + SM: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { Ref: 'AssetParam2' }, + StateMachineName: 'machine-name', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact, { AssetParam2: 'asset-param-2' }); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ + definition: 'asset-param-2', + stateMachineArn: 'machine-name', + }); +}); + +test('calls the updateStateMachine() API when it receives a change to the definitionString that uses Attributes in a state machine', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + }, + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { + 'Fn::Join': [ + '\n', + [ + '{', + ' "StartAt" : "SuccessState"', + ' "States" : {', + ' "SuccessState": {', + ' "Type": "Succeed"', + ' }', + ' }', + '}', + ], + ], + }, + StateMachineName: 'my-machine', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + }, + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { + 'Fn::Join': [ + '', + [ + '"Resource": ', + { 'Fn::GetAtt': ['Func', 'Arn'] }, + ], + ], + }, + StateMachineName: 'my-machine', + }, + }, + }, + }, + }); + + // WHEN + setup.pushStackResourceSummaries( + setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'mock-machine-resource-id'), + setup.stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-func'), + ); + const deployStackResult = await cfnMockProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ + definition: '"Resource": arn:aws:lambda:here:123456789012:function:my-func', + stateMachineArn: 'my-machine', + }); +}); + +test("will not perform a hotswap deployment if it cannot find a Ref target (outside the state machine's name)", async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Parameters: { + Param1: { Type: 'String' }, + }, + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { + 'Fn::Join': [ + '', + [ + '{ Prop: "old-value" }, ', + '{ "Param" : ', + { 'Fn::Sub': '${Param1}' }, + ' }', + ], + ], + }, + }, + }, + }, + }); + setup.pushStackResourceSummaries(setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'my-machine')); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Parameters: { + Param1: { Type: 'String' }, + }, + Resources: { + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { + 'Fn::Join': [ + '', + [ + '{ Prop: "new-value" }, ', + '{ "Param" : ', + { 'Fn::Sub': '${Param1}' }, + ' }', + ], + ], + }, + }, + }, + }, + }, + }); + + // THEN + await expect(() => + cfnMockProvider.tryHotswapDeployment(cdkStackArtifact), + ).rejects.toThrow(/Parameter or resource 'Param1' could not be found for evaluation/); +}); + +test("will not perform a hotswap deployment if it doesn't know how to handle a specific attribute (outside the state machines's name)", async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + }, + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { + 'Fn::Join': [ + '', + [ + '{ Prop: "old-value" }, ', + '{ "S3Bucket" : ', + { 'Fn::GetAtt': ['Bucket', 'UnknownAttribute'] }, + ' }', + ], + ], + }, + StateMachineName: 'my-machine', + }, + }, + }, + }); + setup.pushStackResourceSummaries( + setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'my-machine'), + setup.stackSummaryOf('Bucket', 'AWS::S3::Bucket', 'my-bucket'), + ); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Bucket: { + Type: 'AWS::Lambda::Function', + }, + Machine: { + Type: 'AWS::StepFunctions::StateMachine', + Properties: { + DefinitionString: { + 'Fn::Join': [ + '', + [ + '{ Prop: "new-value" }, ', + '{ "S3Bucket" : ', + { 'Fn::GetAtt': ['Bucket', 'UnknownAttribute'] }, + ' }', + ], + ], + }, + StateMachineName: 'my-machine', + }, + }, + }, + }, + }); + + // THEN + await expect(() => + cfnMockProvider.tryHotswapDeployment(cdkStackArtifact), + ).rejects.toThrow("We don't support the 'UnknownAttribute' attribute of the 'AWS::S3::Bucket' resource. This is a CDK limitation. Please report it at https://github.com/aws/aws-cdk/issues/new/choose"); +}); diff --git a/packages/aws-cdk/test/util/mock-sdk.ts b/packages/aws-cdk/test/util/mock-sdk.ts index 19ab5ae3cc9f5..c6075853c78ba 100644 --- a/packages/aws-cdk/test/util/mock-sdk.ts +++ b/packages/aws-cdk/test/util/mock-sdk.ts @@ -101,6 +101,10 @@ export class MockSdkProvider extends SdkProvider { public stubLambda(stubs: SyncHandlerSubsetOf) { (this.sdk as any).lambda = jest.fn().mockReturnValue(partialAwsService(stubs)); } + + public stubStepFunctions(stubs: SyncHandlerSubsetOf) { + (this.sdk as any).stepFunctions = jest.fn().mockReturnValue(partialAwsService(stubs)); + } } export class MockSdk implements ISDK { @@ -115,6 +119,7 @@ export class MockSdk implements ISDK { public readonly elbv2 = jest.fn(); public readonly secretsManager = jest.fn(); public readonly kms = jest.fn(); + public readonly stepFunctions = jest.fn(); public currentAccount(): Promise { return Promise.resolve({ accountId: '123456789012', partition: 'aws' });