diff --git a/packages/@aws-cdk-testing/cli-integ/lib/shell.ts b/packages/@aws-cdk-testing/cli-integ/lib/shell.ts index 5ec61d5b8..436ee7633 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/shell.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/shell.ts @@ -68,9 +68,22 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom // now write the input with a slight delay to ensure // the child process has already started reading. - setTimeout(() => { + const sendInput = () => { child.writeStdin(interaction.input + (interaction.end ?? os.EOL)); - }, 500); + }; + + if (interaction.beforeInput) { + void interaction.beforeInput() + .catch((err) => { + writeToOutputs(`\n[Prompt: ${interaction.prompt.toString()}] beforeInput hook failed!\n`); + writeToOutputs(`${err}\n\n`); + }) + .finally(() => { + setTimeout(sendInput, 500); + }); + } else { + setTimeout(sendInput, 500); + } } } }); @@ -100,7 +113,7 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom const stdoutOutput = Buffer.concat(stdout).toString('utf-8'); const out = (options.onlyStderr ? stderrOutput : stdoutOutput + stderrOutput).trim(); - const logAndreject = (error: Error) => { + const logAndReject = (error: Error) => { if (show === 'error') { writeToOutputs(`${out}\n`); } @@ -109,15 +122,15 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom if (remainingInteractions.length !== 0) { // regardless of the exit code, if we didn't consume all expected interactions we probably - // did somethiing wrong. - logAndreject(new Error(`Expected more user interactions but subprocess exited with ${code}`)); + // did something wrong. + logAndReject(new Error(`Expected more user interactions but subprocess exited with ${code}`)); return; } if (code === 0 || options.allowErrExit) { resolve(out); } else { - logAndreject(new Error(`'${command.join(' ')}' exited with error code ${code}.`)); + logAndReject(new Error(`'${command.join(' ')}' exited with error code ${code}.`)); } }); }); @@ -168,6 +181,12 @@ export interface UserInteraction { * @default os.EOL */ readonly end?: string; + + /** + * An async callback to run after the prompt is matched but before the input is sent. + * Useful for verifying external state while the process is paused at a prompt. + */ + readonly beforeInput?: () => Promise; } export interface ShellOptions extends child_process.SpawnOptions { diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-with-any-change-approval-shows-diff.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-with-any-change-approval-shows-diff.integtest.ts index 7a22f5ea0..ea6c1507b 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-with-any-change-approval-shows-diff.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-with-any-change-approval-shows-diff.integtest.ts @@ -1,7 +1,5 @@ import { integTest, withDefaultFixture } from '../../../lib'; -jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime - integTest( 'deploy with any-change approval shows diff', withDefaultFixture(async (fixture) => { diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-with-change-set-approval-diff.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-with-change-set-approval-diff.integtest.ts new file mode 100644 index 000000000..db5b0a7e1 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-deploy-with-change-set-approval-diff.integtest.ts @@ -0,0 +1,53 @@ +import { DescribeStacksCommand, ListChangeSetsCommand } from '@aws-sdk/client-cloudformation'; +import { integTest, withDefaultFixture } from '../../../lib'; + +integTest( + 'deploy with change-set method uses change set for approval diff', + withDefaultFixture(async (fixture) => { + let changeSetVerified = false; + const stackName = fixture.fullStackName('test-2'); + const changeSetName = `${fixture.stackNamePrefix}-approval-diff-test`; + + // Deploy with --require-approval=any-change without --yes. + // The CLI will create a change set for the approval diff, pause for confirmation, + // and then execute the same change set after the user confirms. + const output = await fixture.cdkDeploy('test-2', { + options: ['--require-approval=any-change', '--method=change-set', `--change-set-name=${changeSetName}`], + neverRequireApproval: false, + interact: [ + { + prompt: /Do you wish to deploy these changes/, + input: 'y', + beforeInput: async () => { + // While the CLI is paused at the approval prompt, verify that + // the named change set has been created and is ready for execution. + const response = await fixture.aws.cloudFormation.send( + new ListChangeSetsCommand({ StackName: stackName }), + ); + const changeSets = response.Summaries ?? []; + const namedChangeSet = changeSets.find(cs => cs.ChangeSetName === changeSetName); + expect(namedChangeSet).toBeDefined(); + expect(namedChangeSet?.Status).toEqual('CREATE_COMPLETE'); + changeSetVerified = true; + }, + }, + ], + modEnv: { + FORCE_COLOR: '0', + }, + }); + + // The approval diff should contain resource information from the change set + expect(output).toContain('AWS::SNS::Topic'); + expect(output).toContain('Do you wish to deploy these changes'); + + // Verify the beforeInput callback actually ran + expect(changeSetVerified).toBe(true); + + // Verify the stack was actually deployed + const response = await fixture.aws.cloudFormation.send( + new DescribeStacksCommand({ StackName: fixture.fullStackName('test-2') }), + ); + expect(response.Stacks?.[0].StackStatus).toEqual('CREATE_COMPLETE'); + }), +); diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/private/deployment-method.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/private/deployment-method.ts new file mode 100644 index 000000000..a7141b796 --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/private/deployment-method.ts @@ -0,0 +1,53 @@ +import type { ChangeSetDeployment, DeploymentMethod } from '..'; + +export const DEFAULT_DEPLOY_CHANGE_SET_NAME = 'cdk-deploy-change-set'; + +/** + * Execute a previously created change set. + * This is an internal deployment method used by the two-phase deploy flow. + */ +export interface ExecuteChangeSetDeployment { + readonly method: 'execute-change-set'; + readonly changeSetName: string; +} + +/** + * A change set deployment that will execute. + */ +export type ExecutingChangeSetDeployment = ChangeSetDeployment & { execute: true }; + +/** + * A change set deployment that will not execute. + */ +export type NonExecutingChangeSetDeployment = ChangeSetDeployment & { execute: false }; + +/** + * Returns true if the deployment method is a change-set deployment. + */ +export function isChangeSetDeployment(method?: DeploymentMethod): method is ChangeSetDeployment { + return method?.method === 'change-set'; +} + +/** + * Returns true if the deployment method is a change-set deployment that will execute. + */ +export function isExecutingChangeSetDeployment(method?: DeploymentMethod): method is ExecutingChangeSetDeployment { + return isChangeSetDeployment(method) && (method.execute ?? true); +} + +/** + * Returns true if the deployment method is a change-set deployment that will not execute. + */ +export function isNonExecutingChangeSetDeployment(method?: DeploymentMethod): method is NonExecutingChangeSetDeployment { + return isChangeSetDeployment(method) && (method.execute === false); +} + +/** + * Create an ExecuteChangeSetDeployment from a ChangeSetDeployment. + */ +export function toExecuteChangeSetDeployment(method: ChangeSetDeployment): ExecuteChangeSetDeployment { + return { + method: 'execute-change-set', + changeSetName: method.changeSetName ?? DEFAULT_DEPLOY_CHANGE_SET_NAME, + }; +} diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/private/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/private/index.ts index 688292d45..e99fb9bfd 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/private/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/private/index.ts @@ -1,3 +1,4 @@ export * from './deploy-options'; +export * from './deployment-method'; export * from './helpers'; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack.ts index 7e75b30c1..e74018d1f 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack.ts @@ -27,6 +27,7 @@ import { import { determineAllowCrossAccountAssetPublishing } from './checks'; import type { DeployStackResult, SuccessfulDeployStackResult } from './deployment-result'; import type { ChangeSetDeployment, DeploymentMethod, DirectDeployment } from '../../actions/deploy'; +import { DEFAULT_DEPLOY_CHANGE_SET_NAME } from '../../actions/deploy/private/deployment-method'; import { DeploymentError, DeploymentErrorCodes, ToolkitError } from '../../toolkit/toolkit-error'; import { formatErrorMessage } from '../../util'; import type { SDK, SdkProvider, ICloudFormationClient } from '../aws-auth/private'; @@ -40,6 +41,7 @@ import type { IoHelper } from '../io/private'; import type { ResourcesToImport } from '../resource-import'; import { StackActivityMonitor } from '../stack-events'; import { EarlyValidationReporter } from './early-validation'; +import type { ExecuteChangeSetDeployment } from '../../actions/deploy/private/deployment-method'; export interface DeployStackOptions { /** @@ -125,7 +127,7 @@ export interface DeployStackOptions { * * @default - Change set with defaults */ - readonly deploymentMethod?: DeploymentMethod; + readonly deploymentMethod?: DeploymentMethod | ExecuteChangeSetDeployment; /** * The collection of extra parameters @@ -357,7 +359,7 @@ class FullCloudFormationDeployment { private readonly uuid: string; constructor( - private readonly deploymentMethod: DirectDeployment | ChangeSetDeployment, + private readonly deploymentMethod: DirectDeployment | ChangeSetDeployment | ExecuteChangeSetDeployment, private readonly options: DeployStackOptions, private readonly cloudFormationStack: CloudFormationStack, private readonly stackArtifact: cxapi.CloudFormationStackArtifact, @@ -384,13 +386,16 @@ class FullCloudFormationDeployment { case 'change-set': return this.changeSetDeployment(deploymentMethod); + case 'execute-change-set': + return this.executeExistingChangeSet(deploymentMethod); + case 'direct': return this.directDeployment(); } } private async changeSetDeployment(deploymentMethod: ChangeSetDeployment): Promise { - const changeSetName = deploymentMethod.changeSetName ?? 'cdk-deploy-change-set'; + const changeSetName = deploymentMethod.changeSetName ?? DEFAULT_DEPLOY_CHANGE_SET_NAME; const execute = deploymentMethod.execute ?? true; const importExistingResources = deploymentMethod.importExistingResources ?? false; const revertDrift = deploymentMethod.revertDrift ?? false; @@ -437,10 +442,40 @@ class FullCloudFormationDeployment { noOp: false, outputs: this.cloudFormationStack.outputs, stackArn: changeSetDescription.StackId!, + changeSet: changeSetDescription, }; } // If there are replacements in the changeset, check the rollback flag and stack status + return this.checkAndExecuteChangeSet(changeSetDescription); + } + + private async executeExistingChangeSet(deploymentMethod: ExecuteChangeSetDeployment): Promise { + await this.updateTerminationProtection(); + + // The change set was already created and validated during the prepare phase, + // just describe it to get the info needed for execution. + const changeSetDescription = await this.cfn.describeChangeSet({ + StackName: this.stackName, + ChangeSetName: deploymentMethod.changeSetName, + }); + + return this.checkAndExecuteChangeSet(changeSetDescription); + } + + /** + * Check rollback/replacement constraints and execute the change set if all checks pass. + */ + private async checkAndExecuteChangeSet(changeSetDescription: DescribeChangeSetCommandOutput): Promise { + if (changeSetDescription.Status !== 'CREATE_COMPLETE') { + const status = changeSetDescription.Status ?? 'UNKNOWN'; + const reason = changeSetDescription.StatusReason ? `: ${changeSetDescription.StatusReason}` : ''; + throw new ToolkitError( + 'ChangeSetNotReady', + `Change set '${changeSetDescription.ChangeSetName}' on stack '${this.stackName}' is not ready for execution (status: ${status}${reason})`, + ); + } + const replacement = hasReplacement(changeSetDescription); const isPausedFailState = this.cloudFormationStack.stackStatus.isRollbackable; const rollback = this.options.rollback ?? true; @@ -739,6 +774,12 @@ async function canSkipDeploy( return false; } + // Executing an existing change set, never skip + if (deployStackOptions.deploymentMethod?.method === 'execute-change-set') { + await ioHelper.defaults.debug(`${deployName}: executing existing change set, never skip`); + return false; + } + // Drift-aware if ( deployStackOptions.deploymentMethod?.method === 'change-set' && diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployment-result.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployment-result.ts index db21414b8..5fd0fd42b 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployment-result.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployment-result.ts @@ -1,3 +1,4 @@ +import type { DescribeChangeSetCommandOutput } from '@aws-sdk/client-cloudformation'; import { ToolkitError } from '../../toolkit/toolkit-error'; export type DeployStackResult = @@ -12,6 +13,12 @@ export interface SuccessfulDeployStackResult { readonly noOp: boolean; readonly outputs: { [name: string]: string }; readonly stackArn: string; + + /** + * The change set that was created during deployment, if any. + * Populated when using `change-set` deployment method with `execute: false`. + */ + readonly changeSet?: DescribeChangeSetCommandOutput; } /** The stack is currently in a failpaused state, and needs to be rolled back before the deployment */ diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts index 5666f9599..3b7a7422e 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts @@ -10,12 +10,15 @@ import { import { stabilizeStack, uploadStackTemplateAssets, + waitForStackDelete, } from './cfn-api'; import { determineAllowCrossAccountAssetPublishing } from './checks'; import { deployStack, destroyStack } from './deploy-stack'; -import type { DeployStackResult } from './deployment-result'; -import type { DeploymentMethod } from '../../actions/deploy'; +import type { DeployStackResult, SuccessfulDeployStackResult } from './deployment-result'; +import type { ChangeSetDeployment, DeploymentMethod } from '../../actions/deploy'; +import { DEFAULT_DEPLOY_CHANGE_SET_NAME } from '../../actions/deploy/private/deployment-method'; +import type { ExecuteChangeSetDeployment } from '../../actions/deploy/private/deployment-method'; import { DeploymentError, ToolkitError } from '../../toolkit/toolkit-error'; import { formatErrorMessage } from '../../util'; import type { SdkProvider } from '../aws-auth/private'; @@ -89,7 +92,7 @@ export interface DeployStackOptions { * * @default - Change set with default options */ - readonly deploymentMethod?: DeploymentMethod; + readonly deploymentMethod?: DeploymentMethod | ExecuteChangeSetDeployment; /** * Force deployment, even if the deployed template is identical to the one we are about to deploy. @@ -146,6 +149,24 @@ export interface DeployStackOptions { readonly assetParallelism?: boolean; } +export interface PrepareStackOptions extends Omit { + /** + * The change-set deployment method to use. + */ + readonly deploymentMethod: ChangeSetDeployment; + + /** + * Whether to clean up the change set if it has no changes. + * + * Set to true when the caller forced execute: false internally + * (two-phase deploy). Set to false when the user explicitly + * asked for --no-execute (prepare-change-set). + * + * @default false + */ + readonly cleanupOnNoOp?: boolean; +} + export interface RollbackStackOptions { /** * Stack to roll back @@ -389,6 +410,61 @@ export class Deployments { }, this.ioHelper); } + /** + * Create a change set for a stack without executing it. + * + * Returns the result if the change set was successfully created, or undefined + * if the prepare returned a non-success result (e.g. rollback needed). + */ + public async prepareStack( + options: PrepareStackOptions, + ): Promise { + const result = await this.deployStack({ + ...options, + deploymentMethod: { ...options.deploymentMethod, execute: false }, + }); + + // With execute: false, the only possible result type is did-deploy-stack + // (either noOp for empty change sets, or with a changeSet description). + // Rollback/replacement checks are only reached when executing. + if (result.type !== 'did-deploy-stack') { + return undefined; + } + + // Clean up empty change sets if requested (i.e. when the caller forced + // execute: false internally, not when the user explicitly asked for --no-execute). + if (result.noOp && options.cleanupOnNoOp) { + const changeSetName = options.deploymentMethod.changeSetName ?? DEFAULT_DEPLOY_CHANGE_SET_NAME; + await this.cleanupChangeSet(options.stack, changeSetName); + } + + return result; + } + + /** + * Clean up a change set that was created by prepareStack but never executed. + * If the stack was created in REVIEW_IN_PROGRESS state (new stack), delete the stack too. + */ + public async cleanupChangeSet(stack: cxapi.CloudFormationStackArtifact, changeSetName: string): Promise { + const env = await this.envs.accessStackForMutableStackOperations(stack); + const cfn = env.sdk.cloudFormation(); + const deployName = stack.stackName; + + const cloudFormationStack = await CloudFormationStack.lookup(cfn, deployName); + if (!cloudFormationStack.exists) { + return; + } + + await cfn.deleteChangeSet({ StackName: deployName, ChangeSetName: changeSetName }); + + // If the stack was newly created for this change set, it will be in REVIEW_IN_PROGRESS. + // Delete it and wait for the deletion to complete so we don't leave an empty stack behind. + if (cloudFormationStack.stackStatus.name === 'REVIEW_IN_PROGRESS') { + await cfn.deleteStack({ StackName: deployName }); + await waitForStackDelete(cfn, this.ioHelper, deployName); + } + } + public async rollbackStack(options: RollbackStackOptions): Promise { let resourcesToSkip: string[] = options.orphanLogicalIds ?? []; if (options.orphanFailedResources && resourcesToSkip.length > 0) { diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 02e99087f..821cf6516 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -38,8 +38,12 @@ import { BootstrapSource } from '../actions/bootstrap'; import { AssetBuildTime, type DeployOptions } from '../actions/deploy'; import { buildParameterMap, + isChangeSetDeployment, + isExecutingChangeSetDeployment, + isNonExecutingChangeSetDeployment, type PrivateDeployOptions, removePublishedAssetsFromWorkGraph, + toExecuteChangeSetDeployment, } from '../actions/deploy/private'; import { type DestroyOptions } from '../actions/destroy'; import type { DiffOptions } from '../actions/diff'; @@ -667,10 +671,60 @@ export class Toolkit extends CloudAssemblySourceBuilder { const currentTemplate = await deployments.readCurrentTemplate(stack); + // Following are the same semantics we apply with respect to Notification ARNs (dictated by the SDK) + // + // - undefined => cdk ignores it, as if it wasn't supported (allows external management). + // - []: => cdk manages it, and the user wants to wipe it out. + // - ['arn-1'] => cdk manages it, and the user wants to set it to ['arn-1']. + const notificationArns = (!!options.notificationArns || !!stack.notificationArns) + ? (options.notificationArns ?? []).concat(stack.notificationArns ?? []) + : undefined; + + for (const notificationArn of notificationArns ?? []) { + if (!validateSnsTopicArn(notificationArn)) { + throw new ToolkitError('InvalidSnsTopicArn', `Notification arn ${notificationArn} is not a valid arn for an SNS topic`); + } + } + + // Deploy options that are shared between change set creation and execution + const sharedDeployOptions = { + stack, + deployName: stack.stackName, + roleArn: options.roleArn, + toolkitStackName: this.toolkitStackName, + reuseAssets: options.reuseAssets, + tags: options.tags?.length ? options.tags : tagsForStack(stack), + forceDeployment: options.forceDeployment, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), + usePreviousParameters: options.parameters?.keepExistingParameters, + rollback: options.rollback, + notificationArns, + extraUserAgent: options.extraUserAgent, + assetParallelism: options.assetParallelism, + }; + + // When using change-set method, always create the change set upfront. + // This gives us an accurate diff for approval and avoids creating it twice. + // For non-executing deployments (prepare-change-set), this is the final result. + const prepareResult = isChangeSetDeployment(options.deploymentMethod) + ? await deployments.prepareStack({ + ...sharedDeployOptions, + deploymentMethod: options.deploymentMethod, + cleanupOnNoOp: isExecutingChangeSetDeployment(options.deploymentMethod), + }) + : undefined; + + // Empty change set — no changes to deploy + if (prepareResult?.noOp === true) { + await ioHelper.notify(IO.CDK_TOOLKIT_I5900.msg(chalk.green(`\n ✅ ${stack.displayName} (no changes)`), prepareResult)); + return; + } + const formatter = new DiffFormatter({ templateInfo: { oldTemplate: currentTemplate, newTemplate: stack, + changeSet: prepareResult?.changeSet, }, }); @@ -693,22 +747,10 @@ export class Toolkit extends CloudAssemblySourceBuilder { templateDiffs: formatter.diffs, })); if (!deployConfirmed) { - throw new ToolkitError('DeployAborted', 'Aborted by user'); - } - - // Following are the same semantics we apply with respect to Notification ARNs (dictated by the SDK) - // - // - undefined => cdk ignores it, as if it wasn't supported (allows external management). - // - []: => cdk manages it, and the user wants to wipe it out. - // - ['arn-1'] => cdk manages it, and the user wants to set it to ['arn-1']. - const notificationArns = (!!options.notificationArns || !!stack.notificationArns) - ? (options.notificationArns ?? []).concat(stack.notificationArns ?? []) - : undefined; - - for (const notificationArn of notificationArns ?? []) { - if (!validateSnsTopicArn(notificationArn)) { - throw new ToolkitError('InvalidSnsTopicArn', `Notification arn ${notificationArn} is not a valid arn for an SNS topic`); + if (prepareResult?.changeSet?.ChangeSetName) { + await deployments.cleanupChangeSet(stack, prepareResult.changeSet.ChangeSetName); } + throw new ToolkitError('DeployAborted', 'Aborted by user'); } const stackIndex = stacks.indexOf(stack) + 1; @@ -720,14 +762,10 @@ export class Toolkit extends CloudAssemblySourceBuilder { }); deploySpan.incCounter('resources', resourceCount); - let tags = options.tags; - if (!tags || tags.length === 0) { - tags = tagsForStack(stack); - } - let deployDuration; try { - let deployResult: SuccessfulDeployStackResult | undefined; + const prepareIsFinal = isNonExecutingChangeSetDeployment(options.deploymentMethod); + let deployResult: SuccessfulDeployStackResult | undefined = prepareIsFinal ? prepareResult : undefined; let rollback = options.rollback; let iteration = 0; @@ -737,20 +775,13 @@ export class Toolkit extends CloudAssemblySourceBuilder { } const r = await deployments.deployStack({ - stack, - deployName: stack.stackName, - roleArn: options.roleArn, - toolkitStackName: this.toolkitStackName, - reuseAssets: options.reuseAssets, - notificationArns, - tags, - deploymentMethod: options.deploymentMethod, - forceDeployment: options.forceDeployment, - parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), - usePreviousParameters: options.parameters?.keepExistingParameters, + ...sharedDeployOptions, + // On the first iteration, execute the prepared change set. + // On retries (after rollback), create a new change set since the old one is gone. + deploymentMethod: iteration === 1 && isExecutingChangeSetDeployment(options.deploymentMethod) + ? toExecuteChangeSetDeployment(options.deploymentMethod) + : options.deploymentMethod, rollback, - extraUserAgent: options.extraUserAgent, - assetParallelism: options.assetParallelism, }); switch (r.type) { diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/deploy-hotswap.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/deploy-hotswap.test.ts index 243b27a4e..03942be99 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/deploy-hotswap.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/deploy-hotswap.test.ts @@ -16,6 +16,7 @@ jest.mock('../../lib/api/deployments', () => { ...jest.requireActual('../../lib/api/deployments'), Deployments: jest.fn().mockImplementation(() => ({ deployStack: mockDeployStack, + prepareStack: jest.fn().mockResolvedValue(undefined), resolveEnvironment: jest.fn().mockResolvedValue({}), isSingleAssetPublished: jest.fn().mockResolvedValue(true), readCurrentTemplate: jest.fn().mockResolvedValue({ Resources: {} }), diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts index 537c11584..3cadde085 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts @@ -1,4 +1,5 @@ import { StackParameters } from '../../lib/actions/deploy'; +import { DEFAULT_DEPLOY_CHANGE_SET_NAME } from '../../lib/actions/deploy/private'; import type { DeployStackOptions, DeployStackResult } from '../../lib/api/deployments'; import * as deployments from '../../lib/api/deployments'; import { WorkGraphBuilder } from '../../lib/api/work-graph'; @@ -123,6 +124,97 @@ IAM Statement Changes })); }); + describe('two-phase deploy with change-set approval', () => { + test('calls deployStack with execute:false then execute-change-set', async () => { + // GIVEN + jest.spyOn(deployments.Deployments.prototype, 'prepareStack').mockResolvedValueOnce({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack', + changeSet: { Status: 'CREATE_COMPLETE', Changes: [{ Type: 'Resource' }], ChangeSetName: DEFAULT_DEPLOY_CHANGE_SET_NAME, $metadata: {} } as any, + }); + mockDeployStack + .mockReset() + .mockResolvedValueOnce({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack', + }); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.deploy(cx, { + deploymentMethod: { method: 'change-set' }, + }); + + // THEN + expect(mockDeployStack).toHaveBeenCalledTimes(1); + expect(mockDeployStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: expect.objectContaining({ method: 'execute-change-set' }), + }), + ); + }); + + test('uses template-only diff with method=direct', async () => { + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.deploy(cx, { + deploymentMethod: { method: 'direct' }, + }); + + // THEN — only one deployStack call with direct method + expect(mockDeployStack).toHaveBeenCalledTimes(1); + expect(mockDeployStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: { method: 'direct' }, + }), + ); + }); + + test('skips execute when prepare returns noOp', async () => { + // GIVEN + jest.spyOn(deployments.Deployments.prototype, 'prepareStack').mockResolvedValueOnce({ + type: 'did-deploy-stack', + noOp: true, + outputs: {}, + stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack', + }); + jest.spyOn(deployments.Deployments.prototype, 'cleanupChangeSet').mockResolvedValue(); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.deploy(cx, { + deploymentMethod: { method: 'change-set' }, + }); + + // THEN — no deployStack call, prepare was final + expect(mockDeployStack).toHaveBeenCalledTimes(0); + }); + + test('non-executing change-set skips deploy loop', async () => { + // GIVEN + jest.spyOn(deployments.Deployments.prototype, 'prepareStack').mockResolvedValueOnce({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack', + changeSet: { Status: 'CREATE_COMPLETE', Changes: [{ Type: 'Resource' }], ChangeSetName: 'cdk-deploy-change-set', $metadata: {} } as any, + }); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-bucket'); + await toolkit.deploy(cx, { + deploymentMethod: { method: 'change-set', execute: false }, + }); + + // THEN — no deployStack call, prepare was final + expect(mockDeployStack).toHaveBeenCalledTimes(0); + }); + }); + describe('deployment options', () => { test('parameters are passed in', async () => { // WHEN diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/deployment-method.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/deployment-method.test.ts new file mode 100644 index 000000000..cb6e4676c --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/test/actions/deployment-method.test.ts @@ -0,0 +1,68 @@ +import { + isChangeSetDeployment, + isExecutingChangeSetDeployment, + isNonExecutingChangeSetDeployment, + toExecuteChangeSetDeployment, +} from '../../lib/actions/deploy/private/deployment-method'; + +describe('isChangeSetDeployment', () => { + test('true for change-set method', () => { + expect(isChangeSetDeployment({ method: 'change-set' })).toBe(true); + }); + + test('false for direct method', () => { + expect(isChangeSetDeployment({ method: 'direct' })).toBe(false); + }); + + test('false for undefined', () => { + expect(isChangeSetDeployment(undefined)).toBe(false); + }); +}); + +describe('isExecutingChangeSetDeployment', () => { + test('true when execute is undefined (defaults to true)', () => { + expect(isExecutingChangeSetDeployment({ method: 'change-set' })).toBe(true); + }); + + test('true when execute is true', () => { + expect(isExecutingChangeSetDeployment({ method: 'change-set', execute: true })).toBe(true); + }); + + test('false when execute is false', () => { + expect(isExecutingChangeSetDeployment({ method: 'change-set', execute: false })).toBe(false); + }); + + test('false for direct method', () => { + expect(isExecutingChangeSetDeployment({ method: 'direct' })).toBe(false); + }); +}); + +describe('isNonExecutingChangeSetDeployment', () => { + test('true when execute is false', () => { + expect(isNonExecutingChangeSetDeployment({ method: 'change-set', execute: false })).toBe(true); + }); + + test('false when execute is undefined', () => { + expect(isNonExecutingChangeSetDeployment({ method: 'change-set' })).toBe(false); + }); + + test('false when execute is true', () => { + expect(isNonExecutingChangeSetDeployment({ method: 'change-set', execute: true })).toBe(false); + }); + + test('false for direct method', () => { + expect(isNonExecutingChangeSetDeployment({ method: 'direct' })).toBe(false); + }); +}); + +describe('toExecuteChangeSetDeployment', () => { + test('uses provided changeSetName', () => { + const result = toExecuteChangeSetDeployment({ method: 'change-set', changeSetName: 'my-cs' }); + expect(result).toEqual({ method: 'execute-change-set', changeSetName: 'my-cs' }); + }); + + test('defaults changeSetName to cdk-deploy-change-set', () => { + const result = toExecuteChangeSetDeployment({ method: 'change-set' }); + expect(result).toEqual({ method: 'execute-change-set', changeSetName: 'cdk-deploy-change-set' }); + }); +}); diff --git a/packages/@aws-cdk/toolkit-lib/test/api/deployments/cloudformation-deployments.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/deployments/cloudformation-deployments.test.ts index 7b3de279e..12cb2814f 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/deployments/cloudformation-deployments.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/deployments/cloudformation-deployments.test.ts @@ -93,6 +93,52 @@ test('passes through deploymentMethod with hotswap to deployStack()', async () = ); }); +test('prepareStack calls deployStack with execute: false and returns successful result', async () => { + // GIVEN + (deployStack as jest.Mock).mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'arn:stack', + changeSet: { Status: 'CREATE_COMPLETE' }, + }); + + // WHEN + const result = await deployments.prepareStack({ + stack: testStack({ stackName: 'boop' }), + deploymentMethod: { method: 'change-set', changeSetName: 'my-cs' }, + }); + + // THEN + expect(deployStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: { method: 'change-set', changeSetName: 'my-cs', execute: false }, + }), + expect.anything(), + ); + expect(result).toEqual(expect.objectContaining({ + type: 'did-deploy-stack', + noOp: false, + changeSet: { Status: 'CREATE_COMPLETE' }, + })); +}); + +test('prepareStack returns undefined for non-success results', async () => { + // GIVEN + (deployStack as jest.Mock).mockResolvedValue({ + type: 'replacement-requires-rollback', + }); + + // WHEN + const result = await deployments.prepareStack({ + stack: testStack({ stackName: 'boop' }), + deploymentMethod: { method: 'change-set' }, + }); + + // THEN + expect(result).toBeUndefined(); +}); + test('placeholders are substituted in CloudFormation execution role', async () => { await deployments.deployStack({ stack: testStack({ diff --git a/packages/@aws-cdk/toolkit-lib/test/api/deployments/deploy-stack.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/deployments/deploy-stack.test.ts index 4439381ca..41310a2b4 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/deployments/deploy-stack.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/deployments/deploy-stack.test.ts @@ -1258,6 +1258,159 @@ test.each([ test('assertIsSuccessfulDeployStackResult does what it says', () => { expect(() => assertIsSuccessfulDeployStackResult({ type: 'replacement-requires-rollback' })).toThrow(); }); + +describe('execute-change-set deployment method', () => { + test('executes an existing change set without creating a new one', async () => { + // GIVEN + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + Status: 'CREATE_COMPLETE', + ChangeSetName: 'my-change-set', + Changes: [{ Type: 'Resource' as const }], + }); + + // WHEN + const result = await testDeployStack({ + ...standardDeployStackArguments(), + deploymentMethod: { method: 'execute-change-set', changeSetName: 'my-change-set' }, + }); + + // THEN + expect(result.type).toEqual('did-deploy-stack'); + expect(mockCloudFormationClient).not.toHaveReceivedCommand(CreateChangeSetCommand); + expect(mockCloudFormationClient).toHaveReceivedCommand(ExecuteChangeSetCommand); + }); + + test('throws when change set is not in CREATE_COMPLETE status', async () => { + // GIVEN + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + Status: 'FAILED', + StatusReason: 'Something went wrong', + ChangeSetName: 'my-change-set', + }); + + // THEN + await expect(testDeployStack({ + ...standardDeployStackArguments(), + deploymentMethod: { method: 'execute-change-set', changeSetName: 'my-change-set' }, + })).rejects.toThrow(/not ready for execution.*FAILED.*Something went wrong/); + }); + + test('throws when change set is in CREATE_PENDING status', async () => { + // GIVEN + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + Status: 'CREATE_PENDING', + ChangeSetName: 'my-change-set', + }); + + // THEN + await expect(testDeployStack({ + ...standardDeployStackArguments(), + deploymentMethod: { method: 'execute-change-set', changeSetName: 'my-change-set' }, + })).rejects.toThrow(/not ready for execution.*CREATE_PENDING/); + }); + + test('throws without reason when status reason is absent', async () => { + // GIVEN + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + Status: 'DELETE_COMPLETE', + ChangeSetName: 'my-change-set', + }); + + // THEN + await expect(testDeployStack({ + ...standardDeployStackArguments(), + deploymentMethod: { method: 'execute-change-set', changeSetName: 'my-change-set' }, + })).rejects.toThrow(/not ready for execution.*DELETE_COMPLETE(?!.*:)/); + }); + + test('returns replacement-requires-rollback when change set has replacement and rollback is disabled', async () => { + // GIVEN + givenStackExists(); + givenChangeSetContainsReplacement(true); + + // WHEN + const result = await testDeployStack({ + ...standardDeployStackArguments(), + deploymentMethod: { method: 'execute-change-set', changeSetName: 'my-change-set' }, + rollback: false, + }); + + // THEN + expect(result.type).toEqual('replacement-requires-rollback'); + expect(mockCloudFormationClient).not.toHaveReceivedCommand(ExecuteChangeSetCommand); + }); + + test('is never skipped by canSkipDeploy', async () => { + // GIVEN - stack exists with identical template (would normally skip) + givenStackExists(); + givenTemplateIs(DEFAULT_FAKE_TEMPLATE); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + Status: 'CREATE_COMPLETE', + ChangeSetName: 'my-change-set', + Changes: [{ Type: 'Resource' as const }], + }); + + // WHEN + const result = await testDeployStack({ + ...standardDeployStackArguments(), + deploymentMethod: { method: 'execute-change-set', changeSetName: 'my-change-set' }, + }); + + // THEN - still executes despite identical template + expect(result.type).toEqual('did-deploy-stack'); + expect(mockCloudFormationClient).toHaveReceivedCommand(ExecuteChangeSetCommand); + }); +}); + +describe('change set returned with execute:false', () => { + test('returns changeSet description when execute is false', async () => { + // GIVEN + const changeSetResponse = { + Status: ChangeSetStatus.CREATE_COMPLETE, + ChangeSetName: 'cdk-deploy-change-set', + ChangeSetId: 'arn:aws:cloudformation:change-set/123', + StackId: 'arn:aws:cloudformation:stack/123', + Changes: [{ Type: 'Resource' as const }], + }; + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves(changeSetResponse); + + // WHEN + const result = await testDeployStack({ + ...standardDeployStackArguments(), + deploymentMethod: { method: 'change-set', execute: false }, + }); + + // THEN + expect(result.type).toEqual('did-deploy-stack'); + assertIsSuccessfulDeployStackResult(result); + expect(result.noOp).toBe(false); + expect(result.changeSet).toBeDefined(); + expect(result.changeSet?.Status).toEqual('CREATE_COMPLETE'); + }); + + test('does not return changeSet when change set is empty', async () => { + // GIVEN + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + Status: 'FAILED', + StatusReason: "The submitted information didn't contain changes.", + }); + + // WHEN + const result = await testDeployStack({ + ...standardDeployStackArguments(), + deploymentMethod: { method: 'change-set', execute: false }, + }); + + // THEN + assertIsSuccessfulDeployStackResult(result); + expect(result.noOp).toBe(true); + expect(result.changeSet).toBeUndefined(); + }); +}); /** * Set up the mocks so that it looks like the stack exists to start with * diff --git a/packages/aws-cdk/lib/api/deploy-private.ts b/packages/aws-cdk/lib/api/deploy-private.ts new file mode 100644 index 000000000..8dc7f93db --- /dev/null +++ b/packages/aws-cdk/lib/api/deploy-private.ts @@ -0,0 +1,2 @@ +/* eslint-disable import/no-relative-packages */ +export * from '../../../@aws-cdk/toolkit-lib/lib/actions/deploy/private/deployment-method'; diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index c76e7c1ec..f5b0204ac 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -32,10 +32,12 @@ import type { SdkProvider } from '../api/aws-auth'; import type { BootstrapEnvironmentOptions } from '../api/bootstrap'; import { Bootstrapper } from '../api/bootstrap'; import { ExtendedStackSelection, StackCollection } from '../api/cloud-assembly'; +import { isChangeSetDeployment, isExecutingChangeSetDeployment, isNonExecutingChangeSetDeployment, toExecuteChangeSetDeployment } from '../api/deploy-private'; import type { Deployments, SuccessfulDeployStackResult } from '../api/deployments'; import { mappingsByEnvironment, parseMappingGroups } from '../api/refactor'; import { type Tag } from '../api/tags'; import { StackActivityProgress } from '../commands/deploy'; +import { FlagOperations } from '../commands/flags/operations'; import { listStacks } from '../commands/list-stacks'; import type { FromScan, GenerateTemplateOutput } from '../commands/migrate'; import { @@ -69,7 +71,6 @@ import { canCollectTelemetry } from './telemetry/collect-telemetry'; import { cdkCliErrorName } from './telemetry/error'; import { CLI_PRIVATE_SPAN } from './telemetry/messages'; import type { ErrorDetails } from './telemetry/schema'; -import { FlagOperations } from '../commands/flags/operations'; // Must use a require() otherwise esbuild complains about calling a namespace // eslint-disable-next-line @typescript-eslint/no-require-imports,@typescript-eslint/consistent-type-imports @@ -371,7 +372,9 @@ export class CdkToolkit { quiet: boolean, ) { try { - await this.props.deployments.stackExists({ + // we don't actually need to know if the stack exists here + // we just use this to flush our any permissions issues and drop the result + void await this.props.deployments.stackExists({ stack, deployName: stack.stackName, tryLookupRole: true, @@ -509,12 +512,62 @@ export class CdkToolkit { return; } + // Following are the same semantics we apply with respect to Notification ARNs (dictated by the SDK) + // + // - undefined => cdk ignores it, as if it wasn't supported (allows external management). + // - []: => cdk manages it, and the user wants to wipe it out. + // - ['arn-1'] => cdk manages it, and the user wants to set it to ['arn-1']. + const notificationArns = (!!options.notificationArns || !!stack.notificationArns) + ? (options.notificationArns ?? []).concat(stack.notificationArns ?? []) + : undefined; + + for (const notificationArn of notificationArns ?? []) { + if (!validateSnsTopicArn(notificationArn)) { + throw new ToolkitError('InvalidSnsTopicArn', `Notification arn ${notificationArn} is not a valid arn for an SNS topic`); + } + } + + // Deploy options that are shared between change set creation and execution + const sharedDeployOptions = { + stack, + deployName: stack.stackName, + roleArn: options.roleArn, + toolkitStackName: options.toolkitStackName, + reuseAssets: options.reuseAssets, + tags: (options.tags?.length ? options.tags : tagsForStack(stack)), + forceDeployment: options.force, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), + usePreviousParameters: options.usePreviousParameters, + rollback: options.rollback, + notificationArns, + extraUserAgent: options.extraUserAgent, + assetParallelism: options.assetParallelism, + }; + + // When using change-set method, always create the change set upfront. + // This gives us an accurate diff for approval and avoids creating it twice. + // For non-executing deployments (prepare-change-set), this is the final result. + const prepareResult = isChangeSetDeployment(options.deploymentMethod) + ? await this.props.deployments.prepareStack({ + ...sharedDeployOptions, + deploymentMethod: options.deploymentMethod, + cleanupOnNoOp: isExecutingChangeSetDeployment(options.deploymentMethod), + }) + : undefined; + + // Empty change set — no changes to deploy + if (prepareResult?.noOp === true) { + await this.ioHost.asIoHelper().defaults.info(' ✅ %s (no changes)', chalk.bold(stack.displayName)); + return; + } + if (requireApproval !== RequireApproval.NEVER) { const currentTemplate = await this.props.deployments.readCurrentTemplate(stack); const formatter = new DiffFormatter({ templateInfo: { oldTemplate: currentTemplate, newTemplate: stack, + changeSet: prepareResult?.changeSet, }, }); const securityDiff = formatter.formatSecurityDiff(); @@ -526,40 +579,28 @@ export class CdkToolkit { const diffOutput = hasSecurityChanges ? securityDiff.formattedDiff : formatter.formatStackDiff().formattedDiff; await this.ioHost.asIoHelper().defaults.info(diffOutput); - await askUserConfirmation( - this.ioHost, - IO.CDK_TOOLKIT_I5060.req(`${motivation}: 'Do you wish to deploy these changes'`, { - motivation, - concurrency, - permissionChangeType: securityDiff.permissionChangeType, - templateDiffs: formatter.diffs, - }), - ); - } - } - - // Following are the same semantics we apply with respect to Notification ARNs (dictated by the SDK) - // - // - undefined => cdk ignores it, as if it wasn't supported (allows external management). - // - []: => cdk manages it, and the user wants to wipe it out. - // - ['arn-1'] => cdk manages it, and the user wants to set it to ['arn-1']. - const notificationArns = (!!options.notificationArns || !!stack.notificationArns) - ? (options.notificationArns ?? []).concat(stack.notificationArns ?? []) - : undefined; - - for (const notificationArn of notificationArns ?? []) { - if (!validateSnsTopicArn(notificationArn)) { - throw new ToolkitError('InvalidSnsTopicArn', `Notification arn ${notificationArn} is not a valid arn for an SNS topic`); + try { + await askUserConfirmation( + this.ioHost, + IO.CDK_TOOLKIT_I5060.req(`${motivation}: 'Do you wish to deploy these changes'`, { + motivation, + concurrency, + permissionChangeType: securityDiff.permissionChangeType, + templateDiffs: formatter.diffs, + }), + ); + } catch (e) { + if (prepareResult?.changeSet?.ChangeSetName) { + await this.props.deployments.cleanupChangeSet(stack, prepareResult.changeSet.ChangeSetName); + } + throw e; + } } } const stackIndex = stacks.indexOf(stack) + 1; await this.ioHost.asIoHelper().defaults.info(`${chalk.bold(stack.displayName)}: deploying... [${stackIndex}/${stackCollection.stackCount}]`); const startDeployTime = new Date().getTime(); - let tags = options.tags; - if (!tags || tags.length === 0) { - tags = tagsForStack(stack); - } // There is already a startDeployTime constant, but that does not work with telemetry. // We should integrate the two in the future @@ -568,9 +609,17 @@ export class CdkToolkit { let error: ErrorDetails | undefined; let elapsedDeployTime = 0; try { - let deployResult: SuccessfulDeployStackResult | undefined; + // The prepare result is final if the change set was empty (noOp) or + // the deployment method is non-executing (prepare-change-set). + const prepareIsFinal = prepareResult && isNonExecutingChangeSetDeployment(options.deploymentMethod); + let deployResult: SuccessfulDeployStackResult | undefined = prepareIsFinal ? prepareResult : undefined; + // Start with user config for rollback, + // but it might change if we encounter a failed state. let rollback = options.rollback; + + // We limit the loop to 2 iterations max as defensive programming. + // Should not be possible to happen. let iteration = 0; while (!deployResult) { if (++iteration > 2) { @@ -578,20 +627,13 @@ export class CdkToolkit { } const r = await this.props.deployments.deployStack({ - stack, - deployName: stack.stackName, - roleArn: options.roleArn, - toolkitStackName: options.toolkitStackName, - reuseAssets: options.reuseAssets, - notificationArns, - tags, - deploymentMethod: options.deploymentMethod, - forceDeployment: options.force, - parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), - usePreviousParameters: options.usePreviousParameters, + ...sharedDeployOptions, + // On the first iteration, execute the prepared change set. + // On retries (after rollback), create a new change set since the old one is gone. + deploymentMethod: iteration === 1 && isExecutingChangeSetDeployment(options.deploymentMethod) + ? toExecuteChangeSetDeployment(options.deploymentMethod) + : options.deploymentMethod, rollback, - extraUserAgent: options.extraUserAgent, - assetParallelism: options.assetParallelism, }); switch (r.type) { diff --git a/packages/aws-cdk/test/cli/cdk-toolkit.test.ts b/packages/aws-cdk/test/cli/cdk-toolkit.test.ts index 9b06deeb8..50a2ea684 100644 --- a/packages/aws-cdk/test/cli/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cli/cdk-toolkit.test.ts @@ -61,7 +61,7 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import { Manifest, RequireApproval } from '@aws-cdk/cloud-assembly-schema'; import type { DeploymentMethod } from '@aws-cdk/toolkit-lib'; import type { DestroyStackResult } from '@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack'; -import { DescribeStacksCommand, GetTemplateCommand, StackStatus } from '@aws-sdk/client-cloudformation'; +import { CreateChangeSetCommand, DescribeChangeSetCommand, DescribeStacksCommand, GetTemplateCommand, StackStatus } from '@aws-sdk/client-cloudformation'; import { GetParameterCommand } from '@aws-sdk/client-ssm'; import * as fs from 'fs-extra'; import { type Template, type SdkProvider, WorkGraphBuilder } from '../../lib/api'; @@ -225,6 +225,286 @@ describe('deploy', () => { })); }); + describe('two-phase deploy with change-set approval', () => { + test('calls prepareStack then execute-change-set when approval is required', async () => { + // GIVEN + const mockCfnDeployments = instanceMockFrom(Deployments); + mockCfnDeployments.readCurrentTemplate.mockResolvedValue({}); + mockCfnDeployments.prepareStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stackArn', + changeSet: { Status: 'CREATE_COMPLETE', Changes: [{ Type: 'Resource' }], ChangeSetName: 'cdk-deploy-change-set', $metadata: {} }, + }); + mockCfnDeployments.deployStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stackArn', + }); + + const cdkToolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockCfnDeployments, + }); + + // WHEN + await cdkToolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'change-set' }, + }); + + // THEN + expect(mockCfnDeployments.prepareStack).toHaveBeenCalledTimes(1); + expect(mockCfnDeployments.prepareStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: expect.objectContaining({ method: 'change-set' }), + }), + ); + expect(mockCfnDeployments.deployStack).toHaveBeenCalledTimes(1); + expect(mockCfnDeployments.deployStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: expect.objectContaining({ method: 'execute-change-set' }), + }), + ); + }); + + test('always creates change set upfront with method=change-set even when requireApproval is never', async () => { + // GIVEN + const mockCfnDeployments = instanceMockFrom(Deployments); + mockCfnDeployments.readCurrentTemplate.mockResolvedValue({}); + mockCfnDeployments.prepareStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stackArn', + changeSet: { Status: 'CREATE_COMPLETE', Changes: [], ChangeSetName: 'cdk-deploy-change-set', $metadata: {} }, + }); + mockCfnDeployments.deployStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stackArn', + }); + + const cdkToolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockCfnDeployments, + }); + + // WHEN + await cdkToolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.NEVER, + deploymentMethod: { method: 'change-set' }, + }); + + // THEN — prepare + execute + expect(mockCfnDeployments.prepareStack).toHaveBeenCalledTimes(1); + expect(mockCfnDeployments.prepareStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: expect.objectContaining({ method: 'change-set' }), + }), + ); + expect(mockCfnDeployments.deployStack).toHaveBeenCalledTimes(1); + expect(mockCfnDeployments.deployStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: expect.objectContaining({ method: 'execute-change-set' }), + }), + ); + }); + + test('does not create change set for approval with method=direct', async () => { + // GIVEN + const mockCfnDeployments = instanceMockFrom(Deployments); + mockCfnDeployments.readCurrentTemplate.mockResolvedValue({}); + mockCfnDeployments.deployStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stackArn', + }); + + const cdkToolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockCfnDeployments, + }); + + // WHEN + await cdkToolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'direct' }, + }); + + // THEN — only one call with direct method + expect(mockCfnDeployments.deployStack).toHaveBeenCalledTimes(1); + expect(mockCfnDeployments.deployStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: { method: 'direct' }, + }), + ); + }); + + test('skips deploy when prepare returns noOp', async () => { + // GIVEN + const mockCfnDeployments = instanceMockFrom(Deployments); + mockCfnDeployments.readCurrentTemplate.mockResolvedValue({}); + mockCfnDeployments.deployStack.mockResolvedValueOnce({ + type: 'did-deploy-stack', + noOp: true, + outputs: {}, + stackArn: 'stackArn', + }); + + const cdkToolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockCfnDeployments, + }); + + // WHEN + await cdkToolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'change-set' }, + }); + + // THEN — only the prepare call, no execute + expect(mockCfnDeployments.deployStack).toHaveBeenCalledTimes(1); + }); + + test('cleans up change set when approval is rejected', async () => { + // GIVEN + const mockCfnDeployments = instanceMockFrom(Deployments); + mockCfnDeployments.readCurrentTemplate.mockResolvedValue({}); + mockCfnDeployments.prepareStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stackArn', + changeSet: { Status: 'CREATE_COMPLETE', Changes: [{ Type: 'Resource' }], ChangeSetName: 'my-change-set', $metadata: {} }, + }); + + // Reject approval + requestSpy.mockRejectedValue(new Error('Aborted by user')); + + const cdkToolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockCfnDeployments, + }); + + // WHEN + await expect(cdkToolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'change-set' }, + })).rejects.toThrow(/Aborted/); + + // THEN + expect(mockCfnDeployments.cleanupChangeSet).toHaveBeenCalledWith( + expect.anything(), + 'my-change-set', + ); + }); + + test('prepare-change-set skips deploy loop and returns prepare result', async () => { + // GIVEN + const mockCfnDeployments = instanceMockFrom(Deployments); + mockCfnDeployments.prepareStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stackArn', + changeSet: { Status: 'CREATE_COMPLETE', Changes: [{ Type: 'Resource' }], ChangeSetName: 'cdk-deploy-change-set', $metadata: {} }, + }); + + const cdkToolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockCfnDeployments, + }); + + // WHEN + await cdkToolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.NEVER, + deploymentMethod: { method: 'change-set', execute: false }, + }); + + // THEN — prepare only, no deployStack call + expect(mockCfnDeployments.prepareStack).toHaveBeenCalledTimes(1); + expect(mockCfnDeployments.deployStack).not.toHaveBeenCalled(); + }); + + test('falls back to original method on retry after rollback', async () => { + // GIVEN + const mockCfnDeployments = instanceMockFrom(Deployments); + mockCfnDeployments.readCurrentTemplate.mockResolvedValue({}); + mockCfnDeployments.prepareStack.mockResolvedValue({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stackArn', + changeSet: { Status: 'CREATE_COMPLETE', Changes: [{ Type: 'Resource' }], ChangeSetName: 'cdk-deploy-change-set', $metadata: {} }, + }); + // First deploy: needs rollback. Second deploy: succeeds. + mockCfnDeployments.deployStack + .mockResolvedValueOnce({ type: 'failpaused-need-rollback-first', status: 'UPDATE_ROLLBACK_FAILED', reason: 'not-norollback' }) + .mockResolvedValueOnce({ type: 'did-deploy-stack', noOp: false, outputs: {}, stackArn: 'stackArn' }); + mockCfnDeployments.rollbackStack.mockResolvedValue({ success: true, stackArn: 'stackArn' }); + + // Auto-confirm rollback prompt + requestSpy.mockResolvedValue(true); + + const cdkToolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockCfnDeployments, + }); + + // WHEN + await cdkToolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.NEVER, + deploymentMethod: { method: 'change-set' }, + }); + + // THEN — first call uses execute-change-set, second uses original change-set method + expect(mockCfnDeployments.deployStack).toHaveBeenCalledTimes(2); + expect(mockCfnDeployments.deployStack).toHaveBeenNthCalledWith(1, + expect.objectContaining({ + deploymentMethod: expect.objectContaining({ method: 'execute-change-set' }), + }), + ); + expect(mockCfnDeployments.deployStack).toHaveBeenNthCalledWith(2, + expect.objectContaining({ + deploymentMethod: expect.objectContaining({ method: 'change-set' }), + }), + ); + }); + }); + test('fails when no valid stack names are given', async () => { // GIVEN const toolkit = defaultToolkitSetup(); @@ -876,6 +1156,14 @@ describe('deploy', () => { CreationTime: new Date(), }, ], + }) + .on(CreateChangeSetCommand) + .resolves({ Id: 'changeset-id', StackId: 'stack-id' }) + .on(DescribeChangeSetCommand) + .resolves({ + Status: 'CREATE_COMPLETE', + ChangeSetName: 'cdk-deploy-change-set', + Changes: [{ Type: 'Resource' }], }); }); @@ -906,7 +1194,7 @@ describe('deploy', () => { }); expect(mockForEnvironment).toHaveBeenCalledTimes(2); expect(mockForEnvironment).toHaveBeenNthCalledWith( - 1, + 2, { account: '123456789012', name: 'aws://123456789012/here', @@ -954,7 +1242,7 @@ describe('deploy', () => { }); expect(mockForEnvironment).toHaveBeenCalledTimes(3); expect(mockForEnvironment).toHaveBeenNthCalledWith( - 1, + 2, { account: '123456789012', name: 'aws://123456789012/here', @@ -967,7 +1255,7 @@ describe('deploy', () => { }, ); expect(mockForEnvironment).toHaveBeenNthCalledWith( - 2, + 3, { account: '123456789012', name: 'aws://123456789012/here', @@ -1012,7 +1300,7 @@ describe('deploy', () => { ); expect(mockForEnvironment).toHaveBeenCalledTimes(3); expect(mockForEnvironment).toHaveBeenNthCalledWith( - 1, + 2, { account: '123456789012', name: 'aws://123456789012/here', @@ -1025,7 +1313,7 @@ describe('deploy', () => { }, ); expect(mockForEnvironment).toHaveBeenNthCalledWith( - 2, + 3, { account: '123456789012', name: 'aws://123456789012/here', @@ -1041,10 +1329,12 @@ describe('deploy', () => { test('fallback to deploy role if forEnvironment throws', async () => { // GIVEN - // throw error first for the 'prepareSdkWithLookupRoleFor' call and succeed for the rest - mockForEnvironment = jest.spyOn(sdkProvider, 'forEnvironment').mockImplementationOnce(() => { - throw new Error('TheErrorThatGetsThrown'); - }); + // throw error for the 'prepareSdkWithLookupRoleFor' call (second call, after the prepare phase) and succeed for the rest + mockForEnvironment = jest.spyOn(sdkProvider, 'forEnvironment') + .mockResolvedValueOnce({ sdk: mockSdk, didAssumeRole: true }) + .mockImplementationOnce(() => { + throw new Error('TheErrorThatGetsThrown'); + }); const cdkToolkit = new CdkToolkit({ ioHost, @@ -1070,7 +1360,7 @@ describe('deploy', () => { ); expect(mockForEnvironment).toHaveBeenCalledTimes(3); expect(mockForEnvironment).toHaveBeenNthCalledWith( - 1, + 2, { account: '123456789012', name: 'aws://123456789012/here', @@ -1083,7 +1373,7 @@ describe('deploy', () => { }, ); expect(mockForEnvironment).toHaveBeenNthCalledWith( - 2, + 3, { account: '123456789012', name: 'aws://123456789012/here', @@ -1134,11 +1424,10 @@ describe('deploy', () => { name: 'aws://123456789012/here', region: 'here', }, - Mode.ForReading, - { - assumeRoleArn: 'bloop-lookup:here:123456789012', - assumeRoleExternalId: undefined, - }, + Mode.ForWriting, + expect.objectContaining({ + assumeRoleArn: 'bloop:here:123456789012', + }), ); expect(mockForEnvironment).toHaveBeenNthCalledWith( 2, @@ -1147,9 +1436,9 @@ describe('deploy', () => { name: 'aws://123456789012/here', region: 'here', }, - Mode.ForWriting, + Mode.ForReading, { - assumeRoleArn: 'bloop:here:123456789012', + assumeRoleArn: 'bloop-lookup:here:123456789012', assumeRoleExternalId: undefined, }, ); @@ -1190,11 +1479,10 @@ describe('deploy', () => { name: 'aws://123456789012/here', region: 'here', }, - 0, - { - assumeRoleArn: undefined, - assumeRoleExternalId: undefined, - }, + Mode.ForWriting, + expect.objectContaining({ + assumeRoleArn: 'bloop:here:123456789012', + }), ); expect(mockForEnvironment).toHaveBeenNthCalledWith( 2, @@ -1203,9 +1491,9 @@ describe('deploy', () => { name: 'aws://123456789012/here', region: 'here', }, - 1, + Mode.ForReading, { - assumeRoleArn: 'bloop:here:123456789012', + assumeRoleArn: undefined, assumeRoleExternalId: undefined, }, ); @@ -1938,7 +2226,17 @@ describe('rollback', () => { const mockedDeployStack = jest .spyOn(deployments, 'deployStack') + // First call: prepare phase (execute: false) + .mockResolvedValueOnce({ + type: 'did-deploy-stack', + noOp: false, + outputs: {}, + stackArn: 'stack:arn', + changeSet: { Status: 'CREATE_COMPLETE', Changes: [], ChangeSetName: 'cdk-deploy-change-set', $metadata: {} }, + }) + // Second call: execute-change-set returns the test's expected result .mockResolvedValueOnce(firstResult) + // Third call: retry after rollback .mockResolvedValueOnce({ type: 'did-deploy-stack', noOp: false, @@ -1978,8 +2276,9 @@ describe('rollback', () => { } } - expect(mockedDeployStack).toHaveBeenNthCalledWith(1, expect.objectContaining({ rollback: false })); - expect(mockedDeployStack).toHaveBeenNthCalledWith(2, expect.objectContaining({ rollback: true })); + expect(mockedDeployStack).toHaveBeenNthCalledWith(1, expect.objectContaining({ deploymentMethod: expect.objectContaining({ execute: false }) })); + expect(mockedDeployStack).toHaveBeenNthCalledWith(2, expect.objectContaining({ rollback: false, deploymentMethod: expect.objectContaining({ method: 'execute-change-set' }) })); + expect(mockedDeployStack).toHaveBeenNthCalledWith(3, expect.objectContaining({ rollback: true })); }); }); @@ -2113,12 +2412,6 @@ class FakeCloudFormation extends Deployments { expect(options.tags).toEqual(this.expectedTags[options.stack.stackName]); } - // In these tests, we don't make a distinction here between `undefined` and `[]`. - // - // In tests `deployStack` itself we do treat `undefined` and `[]` differently, - // and in `aws-cdk-lib` we emit them under different conditions. But this test - // without normalization depends on a version of `aws-cdk-lib` that hasn't been - // released yet. expect(options.notificationArns ?? []).toEqual(this.expectedNotificationArns ?? []); return Promise.resolve({ type: 'did-deploy-stack',