diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/rollback-test-app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/rollback-test-app/app.js index 419e30898c9bf..dd117b62a9dd9 100644 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/rollback-test-app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/rollback-test-app/app.js @@ -1,15 +1,17 @@ const cdk = require('aws-cdk-lib'); const lambda = require('aws-cdk-lib/aws-lambda'); +const sqs = require('aws-cdk-lib/aws-sqs'); const cr = require('aws-cdk-lib/custom-resources'); /** * This stack will be deployed in multiple phases, to achieve a very specific effect * - * It contains resources r1 and r2, where r1 gets deployed first. + * It contains resources r1 and r2, and a queue q, where r1 gets deployed first. * * - PHASE = 1: both resources deploy regularly. * - PHASE = 2a: r1 gets updated, r2 will fail to update * - PHASE = 2b: r1 gets updated, r2 will fail to update, and r1 will fail its rollback. + * - PHASE = 3: q gets replaced w.r.t. phases 1 and 2 * * To exercise this app: * @@ -22,7 +24,7 @@ const cr = require('aws-cdk-lib/custom-resources'); * # This will start a rollback that will fail because r1 fails its rollabck * * env PHASE=2b npx cdk rollback --force - * # This will retry the rollabck and skip r1 + * # This will retry the rollback and skip r1 * ``` */ class RollbacktestStack extends cdk.Stack { @@ -31,6 +33,7 @@ class RollbacktestStack extends cdk.Stack { let r1props = {}; let r2props = {}; + let fifo = false; const phase = process.env.PHASE; switch (phase) { @@ -46,6 +49,9 @@ class RollbacktestStack extends cdk.Stack { r1props.FailRollback = true; r2props.FailUpdate = true; break; + case '3': + fifo = true; + break; } const fn = new lambda.Function(this, 'Fun', { @@ -76,6 +82,10 @@ class RollbacktestStack extends cdk.Stack { properties: r2props, }); r2.node.addDependency(r1); + + new sqs.Queue(this, 'Queue', { + fifo, + }); } } diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index 25d669f8bedb6..ec6e0307777d4 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -2450,6 +2450,103 @@ integTest( }), ); +integTest( + 'automatic rollback if paused and change contains a replacement', + withSpecificFixture('rollback-test-app', async (fixture) => { + let phase = '1'; + + // Should succeed + await fixture.cdkDeploy('test-rollback', { + options: ['--no-rollback'], + modEnv: { PHASE: phase }, + verbose: false, + }); + try { + phase = '2a'; + + // Should fail + const deployOutput = await fixture.cdkDeploy('test-rollback', { + options: ['--no-rollback'], + modEnv: { PHASE: phase }, + verbose: false, + allowErrExit: true, + }); + expect(deployOutput).toContain('UPDATE_FAILED'); + + // Do a deployment with a replacement and --force: this will roll back first and then deploy normally + phase = '3'; + await fixture.cdkDeploy('test-rollback', { + options: ['--no-rollback', '--force'], + modEnv: { PHASE: phase }, + verbose: false, + }); + } finally { + await fixture.cdkDestroy('test-rollback'); + } + }), +); + +integTest( + 'automatic rollback if paused and --no-rollback is removed from flags', + withSpecificFixture('rollback-test-app', async (fixture) => { + let phase = '1'; + + // Should succeed + await fixture.cdkDeploy('test-rollback', { + options: ['--no-rollback'], + modEnv: { PHASE: phase }, + verbose: false, + }); + try { + phase = '2a'; + + // Should fail + const deployOutput = await fixture.cdkDeploy('test-rollback', { + options: ['--no-rollback'], + modEnv: { PHASE: phase }, + verbose: false, + allowErrExit: true, + }); + expect(deployOutput).toContain('UPDATE_FAILED'); + + // Do a deployment removing --no-rollback: this will roll back first and then deploy normally + phase = '1'; + await fixture.cdkDeploy('test-rollback', { + options: ['--force'], + modEnv: { PHASE: phase }, + verbose: false, + }); + } finally { + await fixture.cdkDestroy('test-rollback'); + } + }), +); + +integTest( + 'automatic rollback if replacement and --no-rollback is removed from flags', + withSpecificFixture('rollback-test-app', async (fixture) => { + let phase = '1'; + + // Should succeed + await fixture.cdkDeploy('test-rollback', { + options: ['--no-rollback'], + modEnv: { PHASE: phase }, + verbose: false, + }); + try { + // Do a deployment with a replacement and removing --no-rollback: this will do a regular rollback deploy + phase = '3'; + await fixture.cdkDeploy('test-rollback', { + options: ['--force'], + modEnv: { PHASE: phase }, + verbose: false, + }); + } finally { + await fixture.cdkDestroy('test-rollback'); + } + }), +); + integTest( 'test cdk rollback --force', withSpecificFixture('rollback-test-app', async (fixture) => { diff --git a/packages/@aws-cdk/cloudformation-diff/lib/format.ts b/packages/@aws-cdk/cloudformation-diff/lib/format.ts index 61f44fc15e5fa..a29155f87399a 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/format.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/format.ts @@ -160,7 +160,7 @@ export class Formatter { const resourceType = diff.isRemoval ? diff.oldResourceType : diff.newResourceType; // eslint-disable-next-line max-len - this.print(`${this.formatResourcePrefix(diff)} ${this.formatValue(resourceType, chalk.cyan)} ${this.formatLogicalId(logicalId)} ${this.formatImpact(diff.changeImpact)}`); + this.print(`${this.formatResourcePrefix(diff)} ${this.formatValue(resourceType, chalk.cyan)} ${this.formatLogicalId(logicalId)} ${this.formatImpact(diff.changeImpact)}`.trimEnd()); if (diff.isUpdate) { const differenceCount = diff.differenceCount; diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 05948cd497295..cc012275c6274 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -205,11 +205,14 @@ $ cdk deploy -R ``` If a deployment fails you can update your code and immediately retry the -deployment from the point of failure. If you would like to explicitly roll back a failed, paused deployment, -use `cdk rollback`. +deployment from the point of failure. If you would like to explicitly roll back +a failed, paused deployment, use `cdk rollback`. -NOTE: you cannot use `--no-rollback` for any updates that would cause a resource replacement, only for updates -and creations of new resources. +`--no-rollback` deployments cannot contain resource replacements. If the CLI +detects that a resource is being replaced, it will prompt you to perform +a regular replacement instead. If the stack rollback is currently paused +and you are trying to perform an deployment that contains a replacement, you +will be prompted to roll back first. #### Deploying multiple stacks @@ -801,7 +804,7 @@ In practice this means for any resource in the provided template, for example, } ``` -There must not exist a resource of that type with the same identifier in the desired region. In this example that identfier +There must not exist a resource of that type with the same identifier in the desired region. In this example that identfier would be "amzn-s3-demo-bucket" ##### **The provided template is not deployed to CloudFormation in the account/region, and there *is* overlap with existing resources in the account/region** @@ -900,7 +903,7 @@ CDK Garbage Collection. > API of feature might still change. Otherwise the feature is generally production > ready and fully supported. -`cdk gc` garbage collects unused assets from your bootstrap bucket via the following mechanism: +`cdk gc` garbage collects unused assets from your bootstrap bucket via the following mechanism: - for each object in the bootstrap S3 Bucket, check to see if it is referenced in any existing CloudFormation templates - if not, it is treated as unused and gc will either tag it or delete it, depending on your configuration. @@ -938,7 +941,7 @@ Found X objects to delete based off of the following criteria: Delete this batch (yes/no/delete-all)? ``` -Since it's quite possible that the bootstrap bucket has many objects, we work in batches of 1000 objects or 100 images. +Since it's quite possible that the bootstrap bucket has many objects, we work in batches of 1000 objects or 100 images. To skip the prompt either reply with `delete-all`, or use the `--confirm=false` option. ```console @@ -948,7 +951,7 @@ cdk gc --unstable=gc --confirm=false If you are concerned about deleting assets too aggressively, there are multiple levers you can configure: - rollback-buffer-days: this is the amount of days an asset has to be marked as isolated before it is elligible for deletion. -- created-buffer-days: this is the amount of days an asset must live before it is elligible for deletion. +- created-buffer-days: this is the amount of days an asset must live before it is elligible for deletion. When using `rollback-buffer-days`, instead of deleting unused objects, `cdk gc` will tag them with today's date instead. It will also check if any objects have been tagged by previous runs of `cdk gc` diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts index f3041fd3864ec..46b5fd5bd909a 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts @@ -8,7 +8,7 @@ import { warning } from '../../logging'; import { loadStructuredFile, serializeStructure } from '../../serialize'; import { rootDir } from '../../util/directories'; import { ISDK, Mode, SdkProvider } from '../aws-auth'; -import { DeployStackResult } from '../deploy-stack'; +import { SuccessfulDeployStackResult } from '../deploy-stack'; /* eslint-disable max-len */ @@ -21,7 +21,7 @@ export class Bootstrapper { constructor(private readonly source: BootstrapSource) { } - public bootstrapEnvironment(environment: cxapi.Environment, sdkProvider: SdkProvider, options: BootstrapEnvironmentOptions = {}): Promise { + public bootstrapEnvironment(environment: cxapi.Environment, sdkProvider: SdkProvider, options: BootstrapEnvironmentOptions = {}): Promise { switch (this.source.source) { case 'legacy': return this.legacyBootstrap(environment, sdkProvider, options); @@ -41,7 +41,7 @@ export class Bootstrapper { * Deploy legacy bootstrap stack * */ - private async legacyBootstrap(environment: cxapi.Environment, sdkProvider: SdkProvider, options: BootstrapEnvironmentOptions = {}): Promise { + private async legacyBootstrap(environment: cxapi.Environment, sdkProvider: SdkProvider, options: BootstrapEnvironmentOptions = {}): Promise { const params = options.parameters ?? {}; if (params.trustedAccounts?.length) { @@ -71,7 +71,7 @@ export class Bootstrapper { private async modernBootstrap( environment: cxapi.Environment, sdkProvider: SdkProvider, - options: BootstrapEnvironmentOptions = {}): Promise { + options: BootstrapEnvironmentOptions = {}): Promise { const params = options.parameters ?? {}; @@ -291,7 +291,7 @@ export class Bootstrapper { private async customBootstrap( environment: cxapi.Environment, sdkProvider: SdkProvider, - options: BootstrapEnvironmentOptions = {}): Promise { + options: BootstrapEnvironmentOptions = {}): Promise { // Look at the template, decide whether it's most likely a legacy or modern bootstrap // template, and use the right bootstrapper for that. diff --git a/packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts b/packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts index 501122697eab5..b1f0cb506837f 100644 --- a/packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts +++ b/packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts @@ -6,7 +6,7 @@ import * as fs from 'fs-extra'; import { BOOTSTRAP_VERSION_OUTPUT, BootstrapEnvironmentOptions, BOOTSTRAP_VERSION_RESOURCE, BOOTSTRAP_VARIANT_PARAMETER, DEFAULT_BOOTSTRAP_VARIANT } from './bootstrap-props'; import * as logging from '../../logging'; import { Mode, SdkProvider, ISDK } from '../aws-auth'; -import { deployStack, DeployStackResult } from '../deploy-stack'; +import { assertIsSuccessfulDeployStackResult, deployStack, SuccessfulDeployStackResult } from '../deploy-stack'; import { NoBootstrapStackEnvironmentResources } from '../environment-resources'; import { DEFAULT_TOOLKIT_STACK_NAME, ToolkitInfo } from '../toolkit-info'; @@ -63,14 +63,15 @@ export class BootstrapStack { template: any, parameters: Record, options: Omit, - ): Promise { + ): Promise { if (this.currentToolkitInfo.found && !options.force) { // Safety checks const abortResponse = { + type: 'did-deploy-stack', noOp: true, outputs: {}, stackArn: this.currentToolkitInfo.bootstrapStack.stackId, - }; + } satisfies SuccessfulDeployStackResult; // Validate that the bootstrap stack we're trying to replace is from the same variant as the one we're trying to deploy const currentVariant = this.currentToolkitInfo.variant; @@ -110,7 +111,7 @@ export class BootstrapStack { const assembly = builder.buildAssembly(); - return deployStack({ + const ret = await deployStack({ stack: assembly.getStackByName(this.toolkitStackName), resolvedEnvironment: this.resolvedEnvironment, sdk: this.sdk, @@ -124,6 +125,10 @@ export class BootstrapStack { // Obviously we can't need a bootstrap stack to deploy a bootstrap stack envResources: new NoBootstrapStackEnvironmentResources(this.resolvedEnvironment, this.sdk), }); + + assertIsSuccessfulDeployStackResult(ret); + + return ret; } } diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index fff70866b617c..5fe7b39bd7c89 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -21,12 +21,37 @@ import { determineAllowCrossAccountAssetPublishing } from './util/checks'; import { publishAssets } from '../util/asset-publishing'; import { StringWithoutPlaceholders } from './util/placeholders'; -export interface DeployStackResult { +export type DeployStackResult = + | SuccessfulDeployStackResult + | NeedRollbackFirstDeployStackResult + | ReplacementRequiresNoRollbackStackResult + ; + +/** Successfully deployed a stack */ +export interface SuccessfulDeployStackResult { + readonly type: 'did-deploy-stack'; readonly noOp: boolean; readonly outputs: { [name: string]: string }; readonly stackArn: string; } +/** The stack is currently in a failpaused state, and needs to be rolled back before the deployment */ +export interface NeedRollbackFirstDeployStackResult { + readonly type: 'failpaused-need-rollback-first'; + readonly reason: 'not-norollback' | 'replacement'; +} + +/** The upcoming change has a replacement, which requires deploying without --no-rollback */ +export interface ReplacementRequiresNoRollbackStackResult { + readonly type: 'replacement-requires-norollback'; +} + +export function assertIsSuccessfulDeployStackResult(x: DeployStackResult): asserts x is SuccessfulDeployStackResult { + if (x.type !== 'did-deploy-stack') { + throw new Error(`Unexpected deployStack result. This should not happen: ${JSON.stringify(x)}. If you are seeing this error, please report it at https://github.com/aws/aws-cdk/issues/new/choose.`); + } +} + export interface DeployStackOptions { /** * The stack to be deployed @@ -283,6 +308,7 @@ export async function deployStack(options: DeployStackOptions): Promise { + private async executeChangeSet(changeSet: CloudFormation.DescribeChangeSetOutput): Promise { debug('Initiating execution of changeset %s on stack %s', changeSet.ChangeSetId, this.stackName); await this.cfn.executeChangeSet({ @@ -482,7 +522,7 @@ class FullCloudFormationDeployment { } } - private async directDeployment(): Promise { + private async directDeployment(): Promise { print('%s: %s stack...', chalk.bold(this.stackName), this.update ? 'updating' : 'creating'); const startTime = new Date(); @@ -500,7 +540,7 @@ class FullCloudFormationDeployment { } catch (err: any) { if (err.message === 'No updates are to be performed.') { debug('No updates are to be performed for stack %s', this.stackName); - return { noOp: true, outputs: this.cloudFormationStack.outputs, stackArn: this.cloudFormationStack.stackId }; + return { type: 'did-deploy-stack', noOp: true, outputs: this.cloudFormationStack.outputs, stackArn: this.cloudFormationStack.stackId }; } throw err; } @@ -522,7 +562,7 @@ class FullCloudFormationDeployment { } } - private async monitorDeployment(startTime: Date, expectedChanges: number | undefined): Promise { + private async monitorDeployment(startTime: Date, expectedChanges: number | undefined): Promise { const monitor = this.options.quiet ? undefined : StackActivityMonitor.withDefaultPrinter(this.cfn, this.stackName, this.stackArtifact, { resourcesTotal: expectedChanges, progress: this.options.progress, @@ -543,7 +583,7 @@ class FullCloudFormationDeployment { await monitor?.stop(); } debug('Stack %s has completed updating', this.stackName); - return { noOp: false, outputs: finalState.outputs, stackArn: finalState.stackId }; + return { type: 'did-deploy-stack', noOp: false, outputs: finalState.outputs, stackArn: finalState.stackId }; } /** @@ -722,3 +762,10 @@ function suffixWithErrors(msg: string, errors?: string[]) { function arrayEquals(a: any[], b: any[]): boolean { return a.every(item => b.includes(item)) && b.every(item => a.includes(item)); } + +function hasReplacement(cs: AWS.CloudFormation.DescribeChangeSetOutput) { + return (cs.Changes ?? []).some(c => { + const a = c.ResourceChange?.PolicyAction; + return a === 'ReplaceAndDelete' || a === 'ReplaceAndRetain' || a === 'ReplaceAndSnapshot'; + }); +} diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index 427561fce67a6..7944d499af889 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -2,7 +2,7 @@ import * as cfn_diff from '@aws-cdk/cloudformation-diff'; import * as cxapi from '@aws-cdk/cx-api'; import * as chalk from 'chalk'; import { ISDK, Mode, SdkProvider } from './aws-auth'; -import { DeployStackResult } from './deploy-stack'; +import { SuccessfulDeployStackResult } from './deploy-stack'; import { EvaluateCloudFormationTemplate } from './evaluate-cloudformation-template'; import { print } from '../logging'; import { isHotswappableAppSyncChange } from './hotswap/appsync-mapping-templates'; @@ -66,7 +66,7 @@ export async function tryHotswapDeployment( sdkProvider: SdkProvider, assetParams: { [key: string]: string }, cloudFormationStack: CloudFormationStack, stackArtifact: cxapi.CloudFormationStackArtifact, hotswapMode: HotswapMode, hotswapPropertyOverrides: HotswapPropertyOverrides, -): Promise { +): Promise { // resolve the environment, so we can substitute things like AWS::Region in CFN expressions const resolvedEnv = await sdkProvider.resolveEnvironment(stackArtifact.environment); // create a new SDK using the CLI credentials, because the default one will not work for new-style synthesis - @@ -104,7 +104,7 @@ export async function tryHotswapDeployment( // apply the short-circuitable changes await applyAllHotswappableChanges(sdk, hotswappableChanges); - return { noOp: hotswappableChanges.length === 0, stackArn: cloudFormationStack.stackId, outputs: cloudFormationStack.outputs }; + return { type: 'did-deploy-stack', noOp: hotswappableChanges.length === 0, stackArn: cloudFormationStack.stackId, outputs: cloudFormationStack.outputs }; } /** diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-status.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-status.ts index 4dd113aaa30db..e4555aef93dcb 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation/stack-status.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation/stack-status.ts @@ -67,6 +67,10 @@ export class StackStatus { } } + get isRollbackable(): boolean { + return [RollbackChoice.START_ROLLBACK, RollbackChoice.CONTINUE_UPDATE_ROLLBACK].includes(this.rollbackChoice); + } + public toString(): string { return this.name + (this.reason ? ` (${this.reason})` : ''); } diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 57dabce74a5dd..f6955b6e14449 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -6,7 +6,7 @@ import * as chokidar from 'chokidar'; import * as fs from 'fs-extra'; import * as promptly from 'promptly'; import * as uuid from 'uuid'; -import { DeploymentMethod } from './api'; +import { DeploymentMethod, SuccessfulDeployStackResult } from './api'; import { SdkProvider } from './api/aws-auth'; import { Bootstrapper, BootstrapEnvironmentOptions } from './api/bootstrap'; import { CloudAssembly, DefaultSelection, ExtendedStackSelection, StackCollection, StackSelector } from './api/cxapp/cloud-assembly'; @@ -36,6 +36,12 @@ import { environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob // eslint-disable-next-line @typescript-eslint/no-require-imports const pLimit: typeof import('p-limit') = require('p-limit'); +let TESTING = false; + +export function markTesting() { + TESTING = true; +} + export interface CdkToolkitProps { /** @@ -266,8 +272,8 @@ export class CdkToolkit { }); }; - const deployStack = async (assetNode: StackNode) => { - const stack = assetNode.stack; + const deployStack = async (stackNode: StackNode) => { + const stack = stackNode.stack; if (stackCollection.stackCount !== 1) { highlight(stack.displayName); } if (!stack.environment) { @@ -295,24 +301,11 @@ export class CdkToolkit { if (requireApproval !== RequireApproval.Never) { const currentTemplate = await this.props.deployments.readCurrentTemplate(stack); if (printSecurityDiff(currentTemplate, stack, requireApproval)) { - await withCorkedLogging(async () => { - // only talk to user if STDIN is a terminal (otherwise, fail) - if (!process.stdin.isTTY) { - throw new Error( - '"--require-approval" is enabled and stack includes security-sensitive updates, ' + - 'but terminal (TTY) is not attached so we are unable to get a confirmation from the user'); - } - - // only talk to user if concurrency is 1 (otherwise, fail) - if (concurrency > 1) { - throw new Error( - '"--require-approval" is enabled and stack includes security-sensitive updates, ' + - 'but concurrency is greater than 1 so we are unable to get a confirmation from the user'); - } - - const confirmed = await promptly.confirm('Do you wish to deploy these changes (y/n)?'); - if (!confirmed) { throw new Error('Aborted by user'); } - }); + await askUserConfirmation( + concurrency, + '"--require-approval" is enabled and stack includes security-sensitive updates', + 'Do you wish to deploy these changes', + ); } } @@ -337,31 +330,95 @@ export class CdkToolkit { let elapsedDeployTime = 0; try { - const result = await this.props.deployments.deployStack({ - stack, - deployName: stack.stackName, - roleArn: options.roleArn, - toolkitStackName: options.toolkitStackName, - reuseAssets: options.reuseAssets, - notificationArns, - tags, - execute: options.execute, - changeSetName: options.changeSetName, - deploymentMethod: options.deploymentMethod, - force: options.force, - parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), - usePreviousParameters: options.usePreviousParameters, - progress, - ci: options.ci, - rollback: options.rollback, - hotswap: options.hotswap, - hotswapPropertyOverrides: hotswapPropertyOverrides, - extraUserAgent: options.extraUserAgent, - assetParallelism: options.assetParallelism, - ignoreNoStacks: options.ignoreNoStacks, - }); + let deployResult: SuccessfulDeployStackResult | undefined; - const message = result.noOp + let rollback = options.rollback; + let iteration = 0; + while (!deployResult) { + if (++iteration > 2) { + throw new Error('This loop should have stabilized in 2 iterations, but didn\'t. If you are seeing this error, please report it at https://github.com/aws/aws-cdk/issues/new/choose'); + } + + const r = await this.props.deployments.deployStack({ + stack, + deployName: stack.stackName, + roleArn: options.roleArn, + toolkitStackName: options.toolkitStackName, + reuseAssets: options.reuseAssets, + notificationArns, + tags, + execute: options.execute, + changeSetName: options.changeSetName, + deploymentMethod: options.deploymentMethod, + force: options.force, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), + usePreviousParameters: options.usePreviousParameters, + progress, + ci: options.ci, + rollback, + hotswap: options.hotswap, + hotswapPropertyOverrides: hotswapPropertyOverrides, + extraUserAgent: options.extraUserAgent, + assetParallelism: options.assetParallelism, + ignoreNoStacks: options.ignoreNoStacks, + }); + + switch (r.type) { + case 'did-deploy-stack': + deployResult = r; + break; + + case 'failpaused-need-rollback-first': { + const motivation = r.reason === 'replacement' + ? 'Stack is in a paused fail state and change includes a replacement which cannot be deployed with "--no-rollback"' + : 'Stack is in a paused fail state and command line arguments do not include "--no-rollback"'; + + if (options.force) { + warning(`${motivation}. Rolling back first (--force).`); + } else { + await askUserConfirmation( + concurrency, + motivation, + `${motivation}. Roll back first and then proceed with deployment`, + ); + } + + // Perform a rollback + await this.rollback({ + selector: { patterns: [stack.hierarchicalId] }, + toolkitStackName: options.toolkitStackName, + force: options.force, + }); + + // Go around through the 'while' loop again but switch rollback to true. + rollback = true; + break; + } + + case 'replacement-requires-norollback': { + const motivation = 'Change includes a replacement which cannot be deployed with "--no-rollback"'; + + if (options.force) { + warning(`${motivation}. Proceeding with regular deployment (--force).`); + } else { + await askUserConfirmation( + concurrency, + motivation, + `${motivation}. Perform a regular deployment`, + ); + } + + // Go around through the 'while' loop again but switch rollback to false. + rollback = true; + break; + } + + default: + throw new Error(`Unexpected result type from deployStack: ${JSON.stringify(r)}. If you are seeing this error, please report it at https://github.com/aws/aws-cdk/issues/new/choose`); + } + } + + const message = deployResult.noOp ? ' ✅ %s (no changes)' : ' ✅ %s'; @@ -369,20 +426,20 @@ export class CdkToolkit { elapsedDeployTime = new Date().getTime() - startDeployTime; print('\n✨ Deployment time: %ss\n', formatTime(elapsedDeployTime)); - if (Object.keys(result.outputs).length > 0) { + if (Object.keys(deployResult.outputs).length > 0) { print('Outputs:'); - stackOutputs[stack.stackName] = result.outputs; + stackOutputs[stack.stackName] = deployResult.outputs; } - for (const name of Object.keys(result.outputs).sort()) { - const value = result.outputs[name]; + for (const name of Object.keys(deployResult.outputs).sort()) { + const value = deployResult.outputs[name]; print('%s.%s = %s', chalk.cyan(stack.id), chalk.cyan(name), chalk.underline(chalk.cyan(value))); } print('Stack ARN:'); - data(result.stackArn); + data(deployResult.stackArn); } catch (e: any) { // It has to be exactly this string because an integration test tests for // "bold(stackname) failed: ResourceNotReady: " @@ -1729,3 +1786,30 @@ function obscureTemplate(template: any = {}) { return template; } + +/** + * Ask the user for a yes/no confirmation + * + * Automatically fail the confirmation in case we're in a situation where the confirmation + * cannot be interactively obtained from a human at the keyboard. + */ +async function askUserConfirmation( + concurrency: number, + motivation: string, + question: string, +) { + await withCorkedLogging(async () => { + // only talk to user if STDIN is a terminal (otherwise, fail) + if (!TESTING && !process.stdin.isTTY) { + throw new Error(`${motivation}, but terminal (TTY) is not attached so we are unable to get a confirmation from the user`); + } + + // only talk to user if concurrency is 1 (otherwise, fail) + if (concurrency > 1) { + throw new Error(`${motivation}, but concurrency is greater than 1 so we are unable to get a confirmation from the user`); + } + + const confirmed = await promptly.confirm(`${chalk.cyan(question)} (y/n)?`); + if (!confirmed) { throw new Error('Aborted by user'); } + }); +} diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts index c940efb46fca7..487201108d293 100644 --- a/packages/aws-cdk/lib/diff.ts +++ b/packages/aws-cdk/lib/diff.ts @@ -119,19 +119,16 @@ export function printSecurityDiff( oldTemplate: any, newTemplate: cxapi.CloudFormationStackArtifact, requireApproval: RequireApproval, - quiet?: boolean, + _quiet?: boolean, stackName?: string, changeSet?: DescribeChangeSetOutput, stream: FormatStream = process.stderr, ): boolean { const diff = fullDiff(oldTemplate, newTemplate.template, changeSet); - // must output the stack name if there are differences, even if quiet - if (!quiet || !diff.isEmpty) { + if (diffRequiresApproval(diff, requireApproval)) { stream.write(format('Stack %s\n', chalk.bold(stackName))); - } - if (difRequiresApproval(diff, requireApproval)) { // eslint-disable-next-line max-len warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${requireApproval}).`); warning('Please confirm you intend to make the following modifications:\n'); @@ -148,7 +145,7 @@ export function printSecurityDiff( * TODO: Filter the security impact determination based off of an enum that allows * us to pick minimum "severities" to alert on. */ -function difRequiresApproval(diff: TemplateDiff, requireApproval: RequireApproval) { +function diffRequiresApproval(diff: TemplateDiff, requireApproval: RequireApproval) { switch (requireApproval) { case RequireApproval.Never: return false; case RequireApproval.AnyChange: return diff.permissionsAnyChanges; diff --git a/packages/aws-cdk/lib/import.ts b/packages/aws-cdk/lib/import.ts index cd6f70cebb03f..b0a94b54ded92 100644 --- a/packages/aws-cdk/lib/import.ts +++ b/packages/aws-cdk/lib/import.ts @@ -6,6 +6,7 @@ import * as chalk from 'chalk'; import * as fs from 'fs-extra'; import * as promptly from 'promptly'; import { DeploymentMethod } from './api'; +import { assertIsSuccessfulDeployStackResult } from './api/deploy-stack'; import { Deployments } from './api/deployments'; import { ResourceIdentifierProperties, ResourcesToImport } from './api/util/cloudformation'; import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; @@ -153,6 +154,8 @@ export class ResourceImporter { resourcesToImport, }); + assertIsSuccessfulDeployStackResult(result); + const message = result.noOp ? ' ✅ %s (no changes)' : ' ✅ %s'; diff --git a/packages/aws-cdk/test/api/bootstrap2.test.ts b/packages/aws-cdk/test/api/bootstrap2.test.ts index d9ec9d563768a..4be0ec22ea2f6 100644 --- a/packages/aws-cdk/test/api/bootstrap2.test.ts +++ b/packages/aws-cdk/test/api/bootstrap2.test.ts @@ -1,10 +1,7 @@ /* eslint-disable import/order */ -const mockDeployStack = jest.fn(); - -jest.mock('../../lib/api/deploy-stack', () => ({ - deployStack: mockDeployStack, -})); +import * as deployStack from '../../lib/api/deploy-stack'; +const mockDeployStack = jest.spyOn(deployStack, 'deployStack'); import { IAM } from 'aws-sdk'; import { Bootstrapper, DeployStackOptions, ToolkitInfo } from '../../lib/api'; @@ -53,6 +50,12 @@ describe('Bootstrapping v2', () => { createPolicy: mockCreatePolicyIamCode, getPolicy: mockGetPolicyIamCode, }); + mockDeployStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'arn:stack', + }); }); afterEach(() => { @@ -341,6 +344,12 @@ describe('Bootstrapping v2', () => { let template: any; mockDeployStack.mockImplementation((args: DeployStackOptions) => { template = args.stack.template; + return Promise.resolve({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'arn:stack', + }); }); await bootstrapper.bootstrapEnvironment(env, sdk, { diff --git a/packages/aws-cdk/test/api/deploy-stack.test.ts b/packages/aws-cdk/test/api/deploy-stack.test.ts index 7afcea68f4c60..cfc6f92a709df 100644 --- a/packages/aws-cdk/test/api/deploy-stack.test.ts +++ b/packages/aws-cdk/test/api/deploy-stack.test.ts @@ -1,5 +1,5 @@ /* eslint-disable import/order */ -import { deployStack, DeployStackOptions } from '../../lib/api'; +import { assertIsSuccessfulDeployStackResult, deployStack, DeployStackOptions } from '../../lib/api'; import { HotswapMode } from '../../lib/api/hotswap/common'; import { tryHotswapDeployment } from '../../lib/api/hotswap-deployments'; import { setCI } from '../../lib/logging'; @@ -129,7 +129,7 @@ test("calls tryHotswapDeployment() if 'hotswap' is `HotswapMode.HOTSWAP_ONLY`", }); // THEN - expect(deployStackResult.noOp).toEqual(true); + expect(deployStackResult.type === 'did-deploy-stack' && deployStackResult.noOp).toEqual(true); expect(tryHotswapDeployment).toHaveBeenCalled(); // check that the extra User-Agent is honored expect(sdk.appendCustomUserAgent).toHaveBeenCalledWith('extra-user-agent'); @@ -275,7 +275,7 @@ test('do deploy executable change set with 0 changes', async () => { }); // THEN - expect(ret.noOp).toBeFalsy(); + expect(ret.type === 'did-deploy-stack' && ret.noOp).toBeFalsy(); expect(cfnMocks.executeChangeSet).toHaveBeenCalled(); }); @@ -625,7 +625,7 @@ test('deployStack reports no change if describeChangeSet returns specific error' }); // THEN - expect(deployResult.noOp).toEqual(true); + expect(deployResult.type === 'did-deploy-stack' && deployResult.noOp).toEqual(true); }); test('deploy not skipped if template did not change but one tag removed', async () => { @@ -916,7 +916,33 @@ describe('disable rollback', () => { DisableRollback: true, })); }); +}); + +test.each([ + ['UPDATE_FAILED', 'failpaused-need-rollback-first'], + ['CREATE_COMPLETE', 'replacement-requires-norollback'], +])('no-rollback and replacement is disadvised: %p -> %p', async (stackStatus, expectedType) => { + // GIVEN + givenTemplateIs(FAKE_STACK.template); + givenStackExists({ + NotificationARNs: ['arn:aws:sns:bermuda-triangle-1337:123456789012:TestTopic'], + StackStatus: stackStatus, + }); + givenChangeSetContainsReplacement(); + + // WHEN + const result = await deployStack({ + ...standardDeployStackArguments(), + stack: FAKE_STACK, + rollback: false, + }); + + // THEN + expect(result.type).toEqual(expectedType); +}); +test('assertIsSuccessfulDeployStackResult does what it says', () => { + expect(() => assertIsSuccessfulDeployStackResult({ type: 'replacement-requires-norollback' })).toThrow(); }); /** @@ -955,3 +981,34 @@ function givenTemplateIs(template: any) { TemplateBody: JSON.stringify(template), }); } + +function givenChangeSetContainsReplacement() { + cfnMocks.describeChangeSet?.mockReturnValue({ + Status: 'CREATE_COMPLETE', + Changes: [ + { + Type: 'Resource', + ResourceChange: { + PolicyAction: 'ReplaceAndDelete', + Action: 'Modify', + LogicalResourceId: 'Queue4A7E3555', + PhysicalResourceId: 'https://sqs.eu-west-1.amazonaws.com/111111111111/Queue4A7E3555-P9C8nK3uv8v6.fifo', + ResourceType: 'AWS::SQS::Queue', + Replacement: 'True', + Scope: ['Properties'], + Details: [ + { + Target: { + Attribute: 'Properties', + Name: 'FifoQueue', + RequiresRecreation: 'Always', + }, + Evaluation: 'Static', + ChangeSource: 'DirectModification', + }, + ], + }, + }, + ], + }); +} diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts b/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts index 1288c827f2300..3150d20b5e806 100644 --- a/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts +++ b/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts @@ -3,7 +3,7 @@ import * as AWS from 'aws-sdk'; import * as codebuild from 'aws-sdk/clients/codebuild'; import * as lambda from 'aws-sdk/clients/lambda'; import * as stepfunctions from 'aws-sdk/clients/stepfunctions'; -import { DeployStackResult } from '../../../lib/api'; +import { SuccessfulDeployStackResult } from '../../../lib/api'; import { HotswapMode, HotswapPropertyOverrides } from '../../../lib/api/hotswap/common'; import * as deployments from '../../../lib/api/hotswap-deployments'; import { CloudFormationStack, Template } from '../../../lib/api/util/cloudformation'; @@ -180,7 +180,7 @@ export class HotswapMockSdkProvider { stackArtifact: cxapi.CloudFormationStackArtifact, assetParams: { [key: string]: string } = {}, hotswapPropertyOverrides?: HotswapPropertyOverrides, - ): Promise { + ): Promise { let hotswapProps = hotswapPropertyOverrides || new HotswapPropertyOverrides(); return deployments.tryHotswapDeployment(this.mockSdkProvider, assetParams, currentCfnStack, stackArtifact, hotswapMode, hotswapProps); } diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index 4c108ba418474..a36bf5efd224c 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -60,18 +60,21 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import { Manifest } from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import * as fs from 'fs-extra'; +import * as promptly from 'promptly'; import { instanceMockFrom, MockCloudExecutable, TestStackArtifact } from './util'; import { MockSdkProvider } from './util/mock-sdk'; import { Bootstrapper } from '../lib/api/bootstrap'; -import { DeployStackResult } from '../lib/api/deploy-stack'; +import { DeployStackResult, SuccessfulDeployStackResult } from '../lib/api/deploy-stack'; import { Deployments, DeployStackOptions, DestroyStackOptions, RollbackStackOptions, RollbackStackResult } from '../lib/api/deployments'; import { HotswapMode } from '../lib/api/hotswap/common'; import { Template } from '../lib/api/util/cloudformation'; -import { CdkToolkit, Tag } from '../lib/cdk-toolkit'; +import { CdkToolkit, markTesting, Tag } from '../lib/cdk-toolkit'; import { RequireApproval } from '../lib/diff'; import { Configuration } from '../lib/settings'; import { flatten } from '../lib/util'; +markTesting(); + process.env.CXAPI_DISABLE_SELECT_BY_ID = '1'; let cloudExecutable: MockCloudExecutable; @@ -436,6 +439,7 @@ describe('deploy', () => { // GIVEN const mockCfnDeployments = instanceMockFrom(Deployments); mockCfnDeployments.deployStack.mockReturnValue(Promise.resolve({ + type: 'did-deploy-stack', noOp: false, outputs: {}, stackArn: 'stackArn', @@ -1251,6 +1255,68 @@ describe('synth', () => { expect(mockedRollback).toHaveBeenCalled(); }); + + test.each([ + [{ type: 'failpaused-need-rollback-first', reason: 'replacement' }, false], + [{ type: 'failpaused-need-rollback-first', reason: 'replacement' }, true], + [{ type: 'failpaused-need-rollback-first', reason: 'not-norollback' }, false], + [{ type: 'replacement-requires-norollback' }, false], + [{ type: 'replacement-requires-norollback' }, true], + ] satisfies Array<[DeployStackResult, boolean]>)('no-rollback deployment that cant proceed will be called with rollback on retry: %p (using force: %p)', async (firstResult, useForce) => { + cloudExecutable = new MockCloudExecutable({ + stacks: [ + MockStack.MOCK_STACK_C, + ], + }); + + const deployments = new Deployments({ sdkProvider: new MockSdkProvider() }); + + // Rollback might be called -- just don't do nothing. + const mockRollbackStack = jest.spyOn(deployments, 'rollbackStack').mockResolvedValue({}); + + const mockedDeployStack = jest + .spyOn(deployments, 'deployStack') + .mockResolvedValueOnce(firstResult) + .mockResolvedValueOnce({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stack:arn', + }); + + const mockedConfirm = jest.spyOn(promptly, 'confirm').mockResolvedValue(true); + + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments, + }); + + await toolkit.deploy({ + selector: { patterns: [] }, + hotswap: HotswapMode.FULL_DEPLOYMENT, + rollback: false, + requireApproval: RequireApproval.Never, + force: useForce, + }); + + if (firstResult.type === 'failpaused-need-rollback-first') { + expect(mockRollbackStack).toHaveBeenCalled(); + } + + if (!useForce) { + // Questions will have been asked only if --force is not specified + if (firstResult.type === 'failpaused-need-rollback-first') { + expect(mockedConfirm).toHaveBeenCalledWith(expect.stringContaining('Roll back first and then proceed with deployment')); + } else { + expect(mockedConfirm).toHaveBeenCalledWith(expect.stringContaining('Perform a regular deployment')); + } + } + + expect(mockedDeployStack).toHaveBeenNthCalledWith(1, expect.objectContaining({ rollback: false })); + expect(mockedDeployStack).toHaveBeenNthCalledWith(2, expect.objectContaining({ rollback: true })); + }); }); class MockStack { @@ -1402,7 +1468,7 @@ class FakeCloudFormation extends Deployments { this.expectedNotificationArns = expectedNotificationArns ?? []; } - public deployStack(options: DeployStackOptions): Promise { + public deployStack(options: DeployStackOptions): Promise { expect([ MockStack.MOCK_STACK_A.stackName, MockStack.MOCK_STACK_B.stackName, @@ -1420,6 +1486,7 @@ class FakeCloudFormation extends Deployments { expect(options.notificationArns).toEqual(this.expectedNotificationArns); return Promise.resolve({ + type: 'did-deploy-stack', stackArn: `arn:aws:cloudformation:::stack/${options.stack.stackName}/MockedOut`, noOp: false, outputs: { StackName: options.stack.stackName }, diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index 7267f746b1af9..635e0157fbd88 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -72,7 +72,7 @@ describe('fixed template', () => { const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ''); expect(exitCode).toBe(0); expect(plainTextOutput.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '')).toContain(`Resources -[~] AWS::SomeService::SomeResource SomeResource +[~] AWS::SomeService::SomeResource SomeResource └─ [~] Something ├─ [-] old-value └─ [+] new-value @@ -152,6 +152,7 @@ describe('imports', () => { }); }); cloudFormation.deployStack.mockImplementation((options) => Promise.resolve({ + type: 'did-deploy-stack', noOp: true, outputs: {}, stackArn: '', @@ -272,6 +273,7 @@ describe('non-nested stacks', () => { }); }); cloudFormation.deployStack.mockImplementation((options) => Promise.resolve({ + type: 'did-deploy-stack', noOp: true, outputs: {}, stackArn: '', @@ -485,6 +487,7 @@ describe('stack exists checks', () => { }); }); cloudFormation.deployStack.mockImplementation((options) => Promise.resolve({ + type: 'did-deploy-stack', noOp: true, outputs: {}, stackArn: '', @@ -1044,6 +1047,14 @@ describe('--strict', () => { beforeEach(() => { const oldTemplate = {}; + cloudFormation = instanceMockFrom(Deployments); + cloudFormation.readCurrentTemplateWithNestedStacks.mockImplementation((_stackArtifact: CloudFormationStackArtifact) => { + return Promise.resolve({ + deployedRootTemplate: {}, + nestedStacks: {}, + }); + }); + cloudExecutable = new MockCloudExecutable({ stacks: [{ stackName: 'A', @@ -1095,8 +1106,8 @@ describe('--strict', () => { const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ''); expect(plainTextOutput.trim()).toEqual(`Stack A Resources -[+] AWS::CDK::Metadata MetadataResource -[+] AWS::Something::Amazing SomeOtherResource +[+] AWS::CDK::Metadata MetadataResource +[+] AWS::Something::Amazing SomeOtherResource Other Changes [+] Unknown Rules: {\"CheckBootstrapVersion\":{\"newCheck\":\"newBootstrapVersion\"}} @@ -1120,7 +1131,7 @@ Other Changes const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ''); expect(plainTextOutput.trim()).toEqual(`Stack A Resources -[+] AWS::Something::Amazing SomeOtherResource +[+] AWS::Something::Amazing SomeOtherResource ✨ Number of stacks with differences: 1`);