From 274be5eb6e97ffe2354103bcda971c4019f34116 Mon Sep 17 00:00:00 2001 From: Orien Madgwick <497874+orien@users.noreply.github.com> Date: Wed, 24 Sep 2025 18:22:28 +1000 Subject: [PATCH] feat(cli): change set review on deploy --- .../toolkit-lib/docs/message-registry.md | 3 +- .../toolkit-lib/lib/actions/deploy/index.ts | 35 ++ .../lib/api/deployments/deploy-stack.ts | 35 +- .../lib/api/deployments/deployments.ts | 29 + .../lib/api/diff/diff-formatter.ts | 7 + .../lib/api/io/private/messages.ts | 6 +- .../toolkit-lib/lib/toolkit/toolkit.ts | 94 ++-- .../test/actions/deploy-hotswap.test.ts | 6 + .../toolkit-lib/test/actions/deploy.test.ts | 386 ++++++++++++- .../cloudformation-deployments.test.ts | 106 ++++ .../test/api/deployments/deploy-stack.test.ts | 208 +++++++ .../toolkit-lib/test/api/diff/diff.test.ts | 218 ++++++++ packages/aws-cdk/lib/cli/cdk-toolkit.ts | 138 ++++- packages/aws-cdk/test/cli/cdk-toolkit.test.ts | 511 +++++++++++++++++- 14 files changed, 1714 insertions(+), 68 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/docs/message-registry.md b/packages/@aws-cdk/toolkit-lib/docs/message-registry.md index dbe43118c..0dd01682e 100644 --- a/packages/@aws-cdk/toolkit-lib/docs/message-registry.md +++ b/packages/@aws-cdk/toolkit-lib/docs/message-registry.md @@ -82,13 +82,14 @@ Please let us know by [opening an issue](https://github.com/aws/aws-cdk-cli/issu | `CDK_TOOLKIT_I5002` | Provides time for resource migration | `info` | {@link Duration} | | `CDK_TOOLKIT_W5021` | Empty non-existent stack, deployment is skipped | `warn` | n/a | | `CDK_TOOLKIT_W5022` | Empty existing stack, stack will be destroyed | `warn` | n/a | +| `CDK_TOOLKIT_W5023` | No changes to existing stack, deployment is skipped | `warn` | n/a | | `CDK_TOOLKIT_I5031` | Informs about any log groups that are traced as part of the deployment | `info` | n/a | | `CDK_TOOLKIT_I5032` | Start monitoring log groups | `debug` | {@link CloudWatchLogMonitorControlEvent} | | `CDK_TOOLKIT_I5033` | A log event received from Cloud Watch | `info` | {@link CloudWatchLogEvent} | | `CDK_TOOLKIT_I5034` | Stop monitoring log groups | `debug` | {@link CloudWatchLogMonitorControlEvent} | | `CDK_TOOLKIT_E5035` | A log monitoring error | `error` | {@link ErrorPayload} | | `CDK_TOOLKIT_I5050` | Confirm rollback during deployment | `info` | {@link ConfirmationRequest} | -| `CDK_TOOLKIT_I5060` | Confirm deploy security sensitive changes | `info` | {@link DeployConfirmationRequest} | +| `CDK_TOOLKIT_I5060` | Confirm deploy changes | `info` | {@link DeployConfirmationRequest} | | `CDK_TOOLKIT_I5100` | Stack deploy progress | `info` | {@link StackDeployProgress} | | `CDK_TOOLKIT_I5210` | Started building a specific asset | `trace` | {@link BuildAsset} | | `CDK_TOOLKIT_I5211` | Building the asset has completed | `trace` | {@link Duration} | diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts index 60a795cc0..f8bac1931 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts @@ -35,6 +35,41 @@ export interface ChangeSetDeployment { * @default false */ readonly importExistingResources?: boolean; + + /** + * Whether to execute an existing change set instead of creating a new one. + * When true, the specified changeSetName must exist and will be executed directly. + * When false or undefined, a new change set will be created. + * + * This is useful for secure change set review workflows where: + * 1. A change set is created with `execute: false` + * 2. The change set is reviewed by authorized personnel + * 3. The same change set is executed using this option to ensure + * the exact changes that were reviewed are deployed + * + * @example + * // Step 1: Create change set for review + * deployStack(\{ + * deploymentMethod: \{ + * method: 'change-set', + * changeSetName: 'my-review-changeset', + * execute: false + * \} + * \}); + * + * // Step 2: Execute the reviewed change set + * deployStack(\{ + * deploymentMethod: \{ + * method: 'change-set', + * changeSetName: 'my-review-changeset', + * executeExistingChangeSet: true, + * execute: true + * \} + * \}); + * + * @default false + */ + readonly executeExistingChangeSet?: boolean; } /** 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 189a9f965..d94af2f18 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 @@ -430,10 +430,34 @@ class FullCloudFormationDeployment { const changeSetName = deploymentMethod.changeSetName ?? 'cdk-deploy-change-set'; const execute = deploymentMethod.execute ?? true; const importExistingResources = deploymentMethod.importExistingResources ?? false; - const changeSetDescription = await this.createChangeSet(changeSetName, execute, importExistingResources); + const executeExistingChangeSet = deploymentMethod.executeExistingChangeSet ?? false; + + let changeSetDescription: DescribeChangeSetCommandOutput; + + if (executeExistingChangeSet) { + // Execute an existing change set instead of creating a new one + await this.ioHelper.defaults.info(format('Executing existing change set %s on stack %s', changeSetName, this.stackName)); + changeSetDescription = await this.cfn.describeChangeSet({ + StackName: this.stackName, + ChangeSetName: changeSetName, + }); + + // Verify the change set exists and is in a valid state + if (!changeSetDescription.ChangeSetId) { + throw new ToolkitError(format('Change set %s not found on stack %s', changeSetName, this.stackName)); + } + if (changeSetDescription.Status !== 'CREATE_COMPLETE') { + throw new ToolkitError(format('Change set %s is in status %s and cannot be executed', changeSetName, changeSetDescription.Status)); + } + } else { + // Create a new change set (existing behavior) + changeSetDescription = await this.createChangeSet(changeSetName, execute, importExistingResources); + } + await this.updateTerminationProtection(); - if (changeSetHasNoChanges(changeSetDescription)) { + // Only check for empty changes when creating a new change set, not when executing an existing one + if (!executeExistingChangeSet && changeSetHasNoChanges(changeSetDescription)) { await this.ioHelper.defaults.debug(format('No changes are to be performed on %s.', this.stackName)); if (execute) { await this.ioHelper.defaults.debug(format('Deleting empty change set %s', changeSetDescription.ChangeSetId)); @@ -768,6 +792,13 @@ async function canSkipDeploy( return false; } + // Executing existing change set, never skip + if (deployStackOptions.deploymentMethod?.method === 'change-set' && + deployStackOptions.deploymentMethod.executeExistingChangeSet === true) { + await ioHelper.defaults.debug(`${deployName}: executing existing change set, never skip`); + return false; + } + // No existing stack if (!cloudFormationStack.exists) { await ioHelper.defaults.debug(`${deployName}: no existing stack`); 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 540e296dd..1ac1e374d 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts @@ -1,6 +1,7 @@ import { randomUUID } from 'crypto'; import * as cdk_assets from '@aws-cdk/cdk-assets-lib'; import type * as cxapi from '@aws-cdk/cx-api'; +import type { DescribeChangeSetCommandOutput } from '@aws-sdk/client-cloudformation'; import * as chalk from 'chalk'; import { AssetManifestBuilder } from './asset-manifest-builder'; import { @@ -674,6 +675,34 @@ export class Deployments { return publisher.isEntryPublished(asset); } + /** + * Read change set details for a stack + */ + public async describeChangeSet( + stackArtifact: cxapi.CloudFormationStackArtifact, + changeSetName: string, + ): Promise { + const env = await this.envs.accessStackForReadOnlyStackOperations(stackArtifact); + return env.sdk.cloudFormation().describeChangeSet({ + StackName: stackArtifact.stackName, + ChangeSetName: changeSetName, + }); + } + + /** + * Delete a change set for a stack + */ + public async deleteChangeSet( + stackArtifact: cxapi.CloudFormationStackArtifact, + changeSetName: string, + ): Promise { + const env = await this.envs.accessStackForMutableStackOperations(stackArtifact); + await env.sdk.cloudFormation().deleteChangeSet({ + StackName: stackArtifact.stackName, + ChangeSetName: changeSetName, + }); + } + /** * Validate that the bootstrap stack has the right version for this stack * diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diff/diff-formatter.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diff/diff-formatter.ts index 6adc19273..09e75d81c 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diff/diff-formatter.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diff/diff-formatter.ts @@ -43,6 +43,12 @@ interface FormatStackDiffOutput { * Complete formatted diff */ readonly formattedDiff: string; + + /** + * The type of permission changes in the stack diff. + * The IoHost will use this to decide whether or not to print. + */ + readonly permissionChangeType: PermissionChangeType; } /** @@ -323,6 +329,7 @@ export class DiffFormatter { return { numStacksWithChanges, formattedDiff, + permissionChangeType: this.permissionType(), }; } diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts b/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts index 82435c09a..9a0aae130 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts @@ -151,6 +151,10 @@ export const IO = { code: 'CDK_TOOLKIT_W5022', description: 'Empty existing stack, stack will be destroyed', }), + CDK_TOOLKIT_W5023: make.warn({ + code: 'CDK_TOOLKIT_W5023', + description: 'No changes to existing stack, deployment is skipped', + }), CDK_TOOLKIT_I5031: make.info({ code: 'CDK_TOOLKIT_I5031', description: 'Informs about any log groups that are traced as part of the deployment', @@ -182,7 +186,7 @@ export const IO = { }), CDK_TOOLKIT_I5060: make.confirm({ code: 'CDK_TOOLKIT_I5060', - description: 'Confirm deploy security sensitive changes', + description: 'Confirm deploy changes', interface: 'DeployConfirmationRequest', }), CDK_TOOLKIT_I5100: make.info({ diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 8aa68cd80..3e1e7980e 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -4,6 +4,7 @@ import type { FeatureFlagReportProperties } from '@aws-cdk/cloud-assembly-schema import { ArtifactType } from '@aws-cdk/cloud-assembly-schema'; import type { TemplateDiff } from '@aws-cdk/cloudformation-diff'; import * as cxapi from '@aws-cdk/cx-api'; +import type { DescribeChangeSetCommandOutput } from '@aws-sdk/client-cloudformation'; import * as chalk from 'chalk'; import * as chokidar from 'chokidar'; import * as fs from 'fs-extra'; @@ -19,7 +20,7 @@ import type { EnvironmentBootstrapResult, } from '../actions/bootstrap'; import { BootstrapSource } from '../actions/bootstrap'; -import { AssetBuildTime, type DeployOptions } from '../actions/deploy'; +import { AssetBuildTime, type DeploymentMethod, type DeployOptions } from '../actions/deploy'; import { buildParameterMap, type PrivateDeployOptions, @@ -606,32 +607,6 @@ export class Toolkit extends CloudAssemblySourceBuilder { return; } - const currentTemplate = await deployments.readCurrentTemplate(stack); - - const formatter = new DiffFormatter({ - templateInfo: { - oldTemplate: currentTemplate, - newTemplate: stack, - }, - }); - - const securityDiff = formatter.formatSecurityDiff(); - - // Send a request response with the formatted security diff as part of the message, - // and the template diff as data - // (IoHost decides whether to print depending on permissionChangeType) - const deployMotivation = '"--require-approval" is enabled and stack includes security-sensitive updates.'; - const deployQuestion = `${securityDiff.formattedDiff}\n\n${deployMotivation}\nDo you wish to deploy these changes`; - const deployConfirmed = await ioHelper.requestResponse(IO.CDK_TOOLKIT_I5060.req(deployQuestion, { - motivation: deployMotivation, - concurrency, - permissionChangeType: securityDiff.permissionChangeType, - templateDiffs: formatter.diffs, - })); - if (!deployConfirmed) { - throw new ToolkitError('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). @@ -647,6 +622,63 @@ export class Toolkit extends CloudAssemblySourceBuilder { } } + const tags = (options.tags && options.tags.length > 0) ? options.tags : tagsForStack(stack); + + let deploymentMethod: DeploymentMethod | undefined; + let changeSet: DescribeChangeSetCommandOutput | undefined; + if (options.deploymentMethod?.method === 'change-set') { + // Create a CloudFormation change set + const changeSetName = options.deploymentMethod?.changeSetName || `cdk-deploy-change-set-${Date.now()}`; + await deployments.deployStack({ + stack, + deployName: stack.stackName, + roleArn: options.roleArn, + toolkitStackName: this.toolkitStackName, + reuseAssets: options.reuseAssets, + notificationArns, + tags, + deploymentMethod: { method: 'change-set' as const, changeSetName, execute: false }, + forceDeployment: options.forceDeployment, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), + usePreviousParameters: options.parameters?.keepExistingParameters, + extraUserAgent: options.extraUserAgent, + assetParallelism: options.assetParallelism, + }); + + // Describe the change set to be presented to the user + changeSet = await deployments.describeChangeSet(stack, changeSetName); + + // Don't continue deploying the stack if there are no changes (unless forced) + if (!options.forceDeployment && changeSet.ChangeSetName && (changeSet.Changes === undefined || changeSet.Changes.length === 0)) { + await deployments.deleteChangeSet(stack, changeSet.ChangeSetName); + return ioHelper.notify(IO.CDK_TOOLKIT_W5023.msg(`${chalk.bold(stack.displayName)}: stack has no changes, skipping deployment.`)); + } + + // Adjust the deployment method for the subsequent deployment to execute the existing change set + deploymentMethod = { ...options.deploymentMethod, changeSetName, executeExistingChangeSet: true }; + } + // Present the diff to the user + const oldTemplate = await deployments.readCurrentTemplate(stack); + const formatter = new DiffFormatter({ templateInfo: { oldTemplate, newTemplate: stack, changeSet } }); + const diff = formatter.formatStackDiff(); + + // Send a request response with the formatted diff as part of the message, and the template diff as data + // (IoHost decides whether to print depending on permissionChangeType) + const deployMotivation = 'Approval required for stack deployment.'; + const deployQuestion = `${diff.formattedDiff}\n\n${deployMotivation}\nDo you wish to deploy these changes`; + const deployConfirmed = await ioHelper.requestResponse(IO.CDK_TOOLKIT_I5060.req(deployQuestion, { + motivation: deployMotivation, + concurrency, + permissionChangeType: diff.permissionChangeType, + templateDiffs: formatter.diffs, + })); + if (!deployConfirmed) { + if (changeSet?.ChangeSetName) { + await deployments.deleteChangeSet(stack, changeSet.ChangeSetName); + } + throw new ToolkitError('Aborted by user'); + } + const stackIndex = stacks.indexOf(stack) + 1; const deploySpan = await ioHelper.span(SPAN.DEPLOY_STACK) .begin(`${chalk.bold(stack.displayName)}: deploying... [${stackIndex}/${stackCollection.stackCount}]`, { @@ -655,11 +687,6 @@ export class Toolkit extends CloudAssemblySourceBuilder { stack, }); - let tags = options.tags; - if (!tags || tags.length === 0) { - tags = tagsForStack(stack); - } - let deployDuration; try { let deployResult: SuccessfulDeployStackResult | undefined; @@ -679,7 +706,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { reuseAssets: options.reuseAssets, notificationArns, tags, - deploymentMethod: options.deploymentMethod, + deploymentMethod: deploymentMethod ?? options.deploymentMethod, forceDeployment: options.forceDeployment, parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), usePreviousParameters: options.parameters?.keepExistingParameters, @@ -1392,4 +1419,3 @@ export class Toolkit extends CloudAssemblySourceBuilder { } } } - 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..99caadfe4 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 @@ -19,6 +19,12 @@ jest.mock('../../lib/api/deployments', () => { resolveEnvironment: jest.fn().mockResolvedValue({}), isSingleAssetPublished: jest.fn().mockResolvedValue(true), readCurrentTemplate: jest.fn().mockResolvedValue({ Resources: {} }), + describeChangeSet: jest.fn().mockResolvedValue({ + ChangeSetName: 'test-changeset', + Changes: [], + Status: 'CREATE_COMPLETE', + }), + deleteChangeSet: jest.fn().mockResolvedValue({}), })), }; }); 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 ab2a6c76b..a03cf95fa 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts @@ -29,6 +29,22 @@ beforeEach(() => { jest.spyOn(deployments.Deployments.prototype, 'readCurrentTemplate').mockResolvedValue({ Resources: {} }); jest.spyOn(deployments.Deployments.prototype, 'buildSingleAsset').mockImplementation(); jest.spyOn(deployments.Deployments.prototype, 'publishSingleAsset').mockImplementation(); + jest.spyOn(deployments.Deployments.prototype, 'describeChangeSet').mockResolvedValue({ + ChangeSetName: 'test-changeset', + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Add', + LogicalResourceId: 'TestResource', + ResourceType: 'AWS::S3::Bucket', + }, + }, + ], + Status: 'CREATE_COMPLETE', + $metadata: {}, + }); + jest.spyOn(deployments.Deployments.prototype, 'deleteChangeSet').mockResolvedValue(); }); describe('deploy', () => { @@ -65,7 +81,7 @@ IAM Statement Changes code: 'CDK_TOOLKIT_I5060', message: expect.stringContaining('Do you wish to deploy these changes'), data: expect.objectContaining({ - motivation: expect.stringContaining('stack includes security-sensitive updates.'), + motivation: expect.stringContaining('Approval required for stack'), permissionChangeType: 'broadening', templateDiffs: expect.objectContaining({ Stack1: expect.objectContaining({ @@ -161,6 +177,186 @@ IAM Statement Changes forcePublish: true, })); }); + + test('change-set method creates and describes changeset before deployment', async () => { + const describeChangeSetSpy = jest.spyOn(deployments.Deployments.prototype, 'describeChangeSet'); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'my-test-changeset', + }, + }); + + // THEN + // First call should create changeset with execute: false + expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ + deploymentMethod: expect.objectContaining({ + method: 'change-set', + changeSetName: 'my-test-changeset', + execute: false, + }), + })); + + // Should describe the changeset + expect(describeChangeSetSpy).toHaveBeenCalledWith( + expect.anything(), + 'my-test-changeset', + ); + + // Second call should execute the existing changeset + expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ + deploymentMethod: expect.objectContaining({ + method: 'change-set', + changeSetName: 'my-test-changeset', + executeExistingChangeSet: true, + }), + })); + + expect(mockDeployStack).toHaveBeenCalledTimes(2); + }); + + test('change-set with auto-generated changeSetName when not provided', async () => { + const describeChangeSetSpy = jest.spyOn(deployments.Deployments.prototype, 'describeChangeSet'); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + }, + }); + + // THEN + // Should use auto-generated name + const expectedChangeSetPattern = /^cdk-deploy-change-set-\d+$/; + expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ + deploymentMethod: expect.objectContaining({ + method: 'change-set', + changeSetName: expect.stringMatching(expectedChangeSetPattern), + execute: false, + }), + })); + expect(describeChangeSetSpy).toHaveBeenCalledWith( + expect.anything(), + expect.stringMatching(expectedChangeSetPattern), + ); + }); + + test('change-set with no changes deletes changeset and skips deployment', async () => { + const deleteChangeSetSpy = jest.spyOn(deployments.Deployments.prototype, 'deleteChangeSet'); + jest.spyOn(deployments.Deployments.prototype, 'describeChangeSet').mockResolvedValue({ + ChangeSetName: 'empty-changeset', + Changes: [], + Status: 'CREATE_COMPLETE', + $metadata: {}, + }); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'empty-changeset', + }, + }); + + // THEN + expect(deleteChangeSetSpy).toHaveBeenCalledWith( + expect.anything(), + 'empty-changeset', + ); + + // Should only be called once (for changeset creation, not execution) + expect(mockDeployStack).toHaveBeenCalledTimes(1); + + // Should show skip message + expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + message: expect.stringContaining('stack has no changes, skipping deployment'), + })); + }); + + test('change-set with undefined changes deletes changeset and skips deployment', async () => { + const deleteChangeSetSpy = jest.spyOn(deployments.Deployments.prototype, 'deleteChangeSet'); + jest.spyOn(deployments.Deployments.prototype, 'describeChangeSet').mockResolvedValue({ + ChangeSetName: 'empty-changeset', + Changes: undefined, + Status: 'CREATE_COMPLETE', + $metadata: {}, + }); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'empty-changeset', + }, + }); + + // THEN + expect(deleteChangeSetSpy).toHaveBeenCalledWith( + expect.anything(), + 'empty-changeset', + ); + + // Should only be called once (for changeset creation, not execution) + expect(mockDeployStack).toHaveBeenCalledTimes(1); + }); + + test('change-set method preserves other deployment options', async () => { + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'test-changeset', + }, + roleArn: 'arn:aws:iam::123456789012:role/MyRole', + reuseAssets: ['asset1'], + notificationArns: ['arn:aws:sns:us-east-1:111111111111:topic'], + forceDeployment: true, + parameters: StackParameters.exactly({ + 'my-param': 'my-value', + }), + assetParallelism: false, + }); + + // THEN - Both calls should preserve all options + expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ + roleArn: 'arn:aws:iam::123456789012:role/MyRole', + reuseAssets: ['asset1'], + notificationArns: ['arn:aws:sns:us-east-1:111111111111:topic'], + forceDeployment: true, + parameters: { 'my-param': 'my-value' }, + assetParallelism: false, + })); + + expect(mockDeployStack).toHaveBeenCalledTimes(2); + }); + + test('non-change-set deployment uses original flow', async () => { + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + deploymentMethod: { + method: 'direct', + }, + }); + + // THEN + expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ + deploymentMethod: { method: 'direct' }, + })); + + // Should only be called once (no changeset creation) + expect(mockDeployStack).toHaveBeenCalledTimes(1); + + // describeChangeSet should not be called + expect(deployments.Deployments.prototype.describeChangeSet).not.toHaveBeenCalled(); + }); }); describe('deployment results', () => { @@ -228,6 +424,44 @@ IAM Statement Changes // THEN successfulDeployment(); }); + + test('change-set information included in diff formatter', async () => { + const changeSetData = { + ChangeSetName: 'test-changeset', + Changes: [ + { + Type: 'Resource' as const, + ResourceChange: { + Action: 'Add' as const, + LogicalResourceId: 'TestResource', + ResourceType: 'AWS::S3::Bucket', + }, + }, + ], + Status: 'CREATE_COMPLETE' as const, + }; + + jest.spyOn(deployments.Deployments.prototype, 'describeChangeSet').mockResolvedValue({ + ...changeSetData, + $metadata: {}, + }); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'test-changeset', + }, + }); + + // THEN + // The changeset data should be available for diff formatting + // This is verified through the successful execution and user approval flow + expect(ioHost.requestSpy).toHaveBeenCalledWith(expect.objectContaining({ + message: expect.stringContaining('Do you wish to deploy these changes'), + })); + }); }); test('deploy returns stack information', async () => { @@ -321,6 +555,156 @@ IAM Statement Changes expect(mockDispose).toHaveBeenCalled(); await realDispose(); }); + + test('user rejection of change-set deployment deletes changeset', async () => { + const deleteChangeSetSpy = jest.spyOn(deployments.Deployments.prototype, 'deleteChangeSet'); + + // Mock describeChangeSet to return the specific changeset name + jest.spyOn(deployments.Deployments.prototype, 'describeChangeSet').mockResolvedValue({ + ChangeSetName: 'rejected-changeset', + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Add', + LogicalResourceId: 'TestResource', + ResourceType: 'AWS::S3::Bucket', + }, + }, + ], + Status: 'CREATE_COMPLETE', + $metadata: {}, + }); + + // Mock user rejection + ioHost.requestSpy.mockResolvedValue(false); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + const result = toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'rejected-changeset', + }, + }); + + // THEN + await expect(result).rejects.toThrow('Aborted by user'); + + // Should delete the changeset before throwing + expect(deleteChangeSetSpy).toHaveBeenCalledWith( + expect.anything(), + 'rejected-changeset', + ); + + // Should only be called once (for changeset creation, not execution) + expect(mockDeployStack).toHaveBeenCalledTimes(1); + }); + + test('user rejection of non-change-set deployment does not call deleteChangeSet', async () => { + const deleteChangeSetSpy = jest.spyOn(deployments.Deployments.prototype, 'deleteChangeSet'); + + // Mock user rejection + ioHost.requestSpy.mockResolvedValue(false); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + const result = toolkit.deploy(cx, { + deploymentMethod: { + method: 'direct', + }, + }); + + // THEN + await expect(result).rejects.toThrow('Aborted by user'); + + // Should NOT delete changeset for non-changeset deployment + expect(deleteChangeSetSpy).not.toHaveBeenCalled(); + }); + + test('motivation message format contains stack display name', async () => { + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx); + + // THEN + expect(ioHost.requestSpy).toHaveBeenCalledWith(expect.objectContaining({ + data: expect.objectContaining({ + motivation: 'Approval required for stack deployment.', + }), + })); + }); + + test('describeChangeSet failure is propagated', async () => { + jest.spyOn(deployments.Deployments.prototype, 'describeChangeSet').mockRejectedValue( + new Error('Failed to describe changeset'), + ); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + const result = toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'failing-describe-changeset', + }, + }); + + // THEN + await expect(result).rejects.toThrow('Failed to describe changeset'); + }); + + test('deleteChangeSet failure during user rejection throws deleteChangeSet error', async () => { + const deleteChangeSetSpy = jest.spyOn(deployments.Deployments.prototype, 'deleteChangeSet') + .mockRejectedValue(new Error('Failed to delete changeset')); + + // Mock user rejection + ioHost.requestSpy.mockResolvedValue(false); + + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + const result = toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'delete-fail-changeset', + }, + }); + + // THEN + await expect(result).rejects.toThrow('Failed to delete changeset'); + + // deleteChangeSet should have been attempted + expect(deleteChangeSetSpy).toHaveBeenCalled(); + }); + + test('deploymentMethod variable overrides options.deploymentMethod for changeset execution', async () => { + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + deploymentMethod: { + method: 'change-set', + changeSetName: 'override-test', + }, + }); + + // THEN + // First call: changeset creation + expect(mockDeployStack).toHaveBeenNthCalledWith(1, expect.objectContaining({ + deploymentMethod: expect.objectContaining({ + method: 'change-set', + changeSetName: 'override-test', + execute: false, + }), + })); + + // Second call: changeset execution with modified deployment method + expect(mockDeployStack).toHaveBeenNthCalledWith(2, expect.objectContaining({ + deploymentMethod: expect.objectContaining({ + method: 'change-set', + changeSetName: 'override-test', + executeExistingChangeSet: true, + }), + })); + }); }); function successfulDeployment() { 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 5691c236b..a5389f95a 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 @@ -1,6 +1,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import { ContinueUpdateRollbackCommand, + DeleteChangeSetCommand, DescribeStackEventsCommand, DescribeStacksCommand, ListStackResourcesCommand, @@ -8,6 +9,7 @@ import { type StackResourceSummary, StackStatus, DescribeChangeSetCommand, + ChangeAction, ChangeSetStatus, CreateChangeSetCommand, } from '@aws-sdk/client-cloudformation'; @@ -1215,3 +1217,107 @@ function givenStacks(stacks: Record { + it('calls CloudFormation describeChangeSet with correct parameters', async () => { + // GIVEN + const changeSetName = 'test-changeset'; + const stackName = 'test-stack'; + const stack = testStack({ stackName }); + const mockChangeSetResponse = { + ChangeSetId: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/test-changeset/12345', + ChangeSetName: changeSetName, + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/test-stack/12345', + Status: ChangeSetStatus.CREATE_COMPLETE, + Changes: [{ + Type: 'Resource' as const, + ResourceChange: { + Action: ChangeAction.Modify, + LogicalResourceId: 'TestResource', + ResourceType: 'AWS::S3::Bucket', + }, + }], + }; + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves(mockChangeSetResponse); + + // WHEN + const result = await deployments.describeChangeSet(stack, changeSetName); + + // THEN + expect(result).toEqual(mockChangeSetResponse); + expect(mockCloudFormationClient).toHaveReceivedCommandWith(DescribeChangeSetCommand, { + StackName: stackName, + ChangeSetName: changeSetName, + }); + }); + + it('handles CloudFormation errors gracefully', async () => { + // GIVEN + const changeSetName = 'non-existent-changeset'; + const stackName = 'test-stack'; + const stack = testStack({ stackName }); + const error = new Error('Change set not found'); + mockCloudFormationClient.on(DescribeChangeSetCommand).rejects(error); + + // WHEN + const result = deployments.describeChangeSet(stack, changeSetName); + + // THEN + await expect(result).rejects.toThrow('Change set not found'); + }); + + it('returns the change set', async () => { + // GIVEN + const changeSetName = 'empty-changeset'; + const stackName = 'test-stack'; + const stack = testStack({ stackName }); + const mockChangeSetResponse = { + ChangeSetId: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/empty-changeset/12345', + ChangeSetName: changeSetName, + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/test-stack/12345', + Status: ChangeSetStatus.CREATE_COMPLETE, + }; + + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves(mockChangeSetResponse); + + // WHEN + const result = await deployments.describeChangeSet(stack, changeSetName); + + // THEN + expect(result).toEqual(mockChangeSetResponse); + }); +}); + +describe('deleteChangeSet', () => { + it('calls CloudFormation deleteChangeSet with correct parameters', async () => { + // GIVEN + const changeSetName = 'test-changeset'; + const stackName = 'test-stack'; + const stack = testStack({ stackName }); + mockCloudFormationClient.on(DeleteChangeSetCommand).resolves({}); + + // WHEN + await deployments.deleteChangeSet(stack, changeSetName); + + // THEN + expect(mockCloudFormationClient).toHaveReceivedCommandWith(DeleteChangeSetCommand, { + StackName: stackName, + ChangeSetName: changeSetName, + }); + }); + + it('handles CloudFormation errors gracefully', async () => { + // GIVEN + const changeSetName = 'non-existent-changeset'; + const stackName = 'test-stack'; + const stack = testStack({ stackName }); + const error = new Error('Change set not found'); + mockCloudFormationClient.on(DeleteChangeSetCommand).rejects(error); + + // WHEN + const result = deployments.deleteChangeSet(stack, changeSetName); + + // THEN + await expect(result).rejects.toThrow('Change set not found'); + }); +}); 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 ef9291b60..f977ed92a 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 @@ -1239,3 +1239,211 @@ function givenChangeSetContainsReplacement(replacement: boolean) { ] : [], }); } + +describe('executing existing change sets', () => { + test('can execute an existing change set', async () => { + // GIVEN + const existingChangeSetName = 'existing-change-set'; + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + ChangeSetId: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/existing-change-set/12345678-1234-1234-1234-123456789012', + ChangeSetName: existingChangeSetName, + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/withouterrors/12345678-1234-1234-1234-123456789012', + Status: 'CREATE_COMPLETE', + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Modify', + LogicalResourceId: 'TestResource', + ResourceType: 'AWS::S3::Bucket', + }, + }, + ], + }); + mockCloudFormationClient.on(ExecuteChangeSetCommand).resolves({}); + const resolvedEnvironment = mockResolvedEnvironment(); + + // WHEN + const result = await testDeployStack({ + stack: FAKE_STACK, + resolvedEnvironment, + sdk: new MockSdk(), + sdkProvider: new MockSdkProvider(), + envResources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk, ioHelper), + deploymentMethod: { + method: 'change-set', + executeExistingChangeSet: true, + changeSetName: existingChangeSetName, + execute: true, + }, + }); + + // THEN + expect(result).toEqual(expect.objectContaining({ type: 'did-deploy-stack', noOp: false })); + expect(mockCloudFormationClient).toHaveReceivedCommandWith(DescribeChangeSetCommand, { + StackName: 'withouterrors', + ChangeSetName: existingChangeSetName, + }); + expect(mockCloudFormationClient).toHaveReceivedCommandWith(ExecuteChangeSetCommand, { + StackName: 'withouterrors', + ChangeSetName: existingChangeSetName, + ClientRequestToken: expect.stringMatching(/^exec/), + }); + // Should not create a new change set + expect(mockCloudFormationClient).not.toHaveReceivedCommand(CreateChangeSetCommand); + }); + + test('throws error when existing change set is not found', async () => { + // GIVEN + const nonExistentChangeSetName = 'non-existent-change-set'; + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + // Empty response simulating change set not found + }); + const resolvedEnvironment = mockResolvedEnvironment(); + + // WHEN + const result = testDeployStack({ + stack: FAKE_STACK, + resolvedEnvironment, + sdk: new MockSdk(), + sdkProvider: new MockSdkProvider(), + envResources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk, ioHelper), + deploymentMethod: { + method: 'change-set', + executeExistingChangeSet: true, + changeSetName: nonExistentChangeSetName, + execute: true, + }, + }); + + // THEN + await expect(result).rejects.toThrow(`Change set ${nonExistentChangeSetName} not found on stack withouterrors`); + }); + + test('throws error when existing change set is not in valid state', async () => { + // GIVEN + const invalidChangeSetName = 'invalid-change-set'; + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + ChangeSetId: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/invalid-change-set/12345678-1234-1234-1234-123456789012', + ChangeSetName: invalidChangeSetName, + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/withouterrors/12345678-1234-1234-1234-123456789012', + Status: 'FAILED', + }); + const resolvedEnvironment = mockResolvedEnvironment(); + + // WHEN + const result = testDeployStack({ + stack: FAKE_STACK, + resolvedEnvironment, + sdk: new MockSdk(), + sdkProvider: new MockSdkProvider(), + envResources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk, ioHelper), + deploymentMethod: { + method: 'change-set', + executeExistingChangeSet: true, + changeSetName: invalidChangeSetName, + execute: true, + }, + }); + + // THEN + await expect(result).rejects.toThrow(`Change set ${invalidChangeSetName} is in status FAILED and cannot be executed`); + }); + + test('works with change set in different status', async () => { + // GIVEN + const existingChangeSetName = 'pending-changeset'; + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + ChangeSetId: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/pending-changeset/12345678-1234-1234-1234-123456789012', + ChangeSetName: existingChangeSetName, + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/withouterrors/12345678-1234-1234-1234-123456789012', + Status: 'CREATE_COMPLETE', // Valid status for execution + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Add', + LogicalResourceId: 'NewResource', + ResourceType: 'AWS::S3::Bucket', + }, + }, + ], + }); + mockCloudFormationClient.on(ExecuteChangeSetCommand).resolves({}); + const resolvedEnvironment = mockResolvedEnvironment(); + + // WHEN + const result = await testDeployStack({ + stack: FAKE_STACK, + resolvedEnvironment, + sdk: new MockSdk(), + sdkProvider: new MockSdkProvider(), + envResources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk, ioHelper), + deploymentMethod: { + method: 'change-set', + executeExistingChangeSet: true, + changeSetName: existingChangeSetName, + execute: true, + }, + }); + + // THEN + expect(result).toEqual(expect.objectContaining({ type: 'did-deploy-stack', noOp: false })); + expect(mockCloudFormationClient).toHaveReceivedCommandWith(ExecuteChangeSetCommand, { + StackName: 'withouterrors', + ChangeSetName: existingChangeSetName, + ClientRequestToken: expect.stringMatching(/^exec/), + }); + }); + + test('executeExistingChangeSet with execute false does not execute', async () => { + // GIVEN + const existingChangeSetName = 'review-changeset'; + givenStackExists(); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + ChangeSetId: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/review-changeset/12345678-1234-1234-1234-123456789012', + ChangeSetName: existingChangeSetName, + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/withouterrors/12345678-1234-1234-1234-123456789012', + Status: 'CREATE_COMPLETE', + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Modify', + LogicalResourceId: 'TestResource', + ResourceType: 'AWS::S3::Bucket', + }, + }, + ], + }); + const resolvedEnvironment = mockResolvedEnvironment(); + + // WHEN + const result = await testDeployStack({ + stack: FAKE_STACK, + resolvedEnvironment, + sdk: new MockSdk(), + sdkProvider: new MockSdkProvider(), + envResources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk, ioHelper), + deploymentMethod: { + method: 'change-set', + executeExistingChangeSet: true, + changeSetName: existingChangeSetName, + execute: false, // Don't execute, just describe + }, + }); + + // THEN + expect(result).toEqual(expect.objectContaining({ type: 'did-deploy-stack', noOp: false })); + expect(mockCloudFormationClient).toHaveReceivedCommandWith(DescribeChangeSetCommand, { + StackName: 'withouterrors', + ChangeSetName: existingChangeSetName, + }); + // Should NOT execute the change set + expect(mockCloudFormationClient).not.toHaveReceivedCommand(ExecuteChangeSetCommand); + }); +}); diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diff/diff.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diff/diff.test.ts index 8214e7493..0af7db501 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diff/diff.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diff/diff.test.ts @@ -41,6 +41,7 @@ describe('formatStackDiff', () => { // THEN expect(result.numStacksWithChanges).toBe(0); expect(result.formattedDiff).toBeDefined(); + expect(result.permissionChangeType).toBe('none'); const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim(); expect(sanitizedDiff).toBe( 'Stack test-stack\n' + @@ -61,6 +62,7 @@ describe('formatStackDiff', () => { // THEN expect(result.numStacksWithChanges).toBe(1); expect(result.formattedDiff).toBeDefined(); + expect(result.permissionChangeType).toBe('none'); const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim(); expect(sanitizedDiff).toBe( 'Stack test-stack\n' + @@ -83,6 +85,7 @@ describe('formatStackDiff', () => { // THEN expect(result.numStacksWithChanges).toBe(1); expect(result.formattedDiff).toBeDefined(); + expect(result.permissionChangeType).toBe('none'); const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim(); expect(sanitizedDiff).toBe( 'Stack test-stack\n' + @@ -107,6 +110,7 @@ describe('formatStackDiff', () => { // THEN expect(result.formattedDiff).toBeDefined(); + expect(result.permissionChangeType).toBe('none'); const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim(); expect(sanitizedDiff).toBe( 'Stack test-stack\n' + @@ -145,10 +149,224 @@ describe('formatStackDiff', () => { // THEN expect(result.numStacksWithChanges).toBe(3); + expect(result.permissionChangeType).toBe('none'); expect(result.formattedDiff).toContain(`Stack ${chalk.bold('test-stack')}`); expect(result.formattedDiff).toContain(`Stack ${chalk.bold('nested-stack-1')}`); expect(result.formattedDiff).toContain(`Stack ${chalk.bold('nested-stack-2')}`); }); + + test('returns broadening permission change type when IAM changes broaden permissions', () => { + // GIVEN + const templateWithIAM: cxapi.CloudFormationStackArtifact = { + template: { + Resources: { + Role: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [{ + Effect: 'Allow', + Principal: { + Service: 'lambda.amazonaws.com', + }, + Action: 'sts:AssumeRole', + }], + }, + }, + }, + }, + }, + templateFile: 'template.json', + stackName: 'test-stack', + findMetadataByType: () => [], + } as any; + + // WHEN + const formatter = new DiffFormatter({ + templateInfo: { + oldTemplate: {}, + newTemplate: templateWithIAM, + }, + }); + const result = formatter.formatStackDiff(); + + // THEN + expect(result.numStacksWithChanges).toBe(1); + expect(result.permissionChangeType).toBe('broadening'); + expect(result.formattedDiff).toBeDefined(); + const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim(); + expect(sanitizedDiff).toContain('Stack test-stack'); + expect(sanitizedDiff).toContain('[+] AWS::IAM::Role Role'); + }); + + test('returns non-broadening permission change type when IAM changes but no broadening', () => { + // GIVEN + const oldTemplate = { + Resources: { + Role: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [{ + Effect: 'Allow', + Principal: { + Service: 'lambda.amazonaws.com', + }, + Action: 'sts:AssumeRole', + }], + }, + ManagedPolicyArns: [ + 'arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole', + 'arn:aws:iam::aws:policy/AmazonS3FullAccess', + ], + }, + }, + }, + }; + + const newTemplate: cxapi.CloudFormationStackArtifact = { + template: { + Resources: { + Role: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [{ + Effect: 'Allow', + Principal: { + Service: 'lambda.amazonaws.com', + }, + Action: 'sts:AssumeRole', + }], + }, + ManagedPolicyArns: [ + 'arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole', + ], + }, + }, + }, + }, + templateFile: 'template.json', + stackName: 'test-stack', + findMetadataByType: () => [], + } as any; + + // WHEN - removing a managed policy (narrowing permissions) + const formatter = new DiffFormatter({ + templateInfo: { + oldTemplate, + newTemplate, + }, + }); + const result = formatter.formatStackDiff(); + + // THEN + expect(result.numStacksWithChanges).toBe(1); + expect(result.permissionChangeType).toBe('non-broadening'); + expect(result.formattedDiff).toBeDefined(); + }); + + test('uses changeSet parameter when provided', () => { + // GIVEN + const mockChangeSet = { + ChangeSetName: 'test-changeset', + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Add', + LogicalResourceId: 'Func', + ResourceType: 'AWS::Lambda::Function', + }, + }, + ], + Status: 'CREATE_COMPLETE', + $metadata: {}, + }; + + // WHEN + const formatter = new DiffFormatter({ + templateInfo: { + oldTemplate: {}, + newTemplate: mockNewTemplate, + changeSet: mockChangeSet, + }, + }); + const result = formatter.formatStackDiff(); + + // THEN + expect(result.numStacksWithChanges).toBe(1); + expect(result.permissionChangeType).toBe('none'); + expect(result.formattedDiff).toBeDefined(); + // The changeSet should be used internally by the fullDiff function + // We can't easily verify this directly, but the diff should still work correctly + const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim(); + expect(sanitizedDiff).toBe( + 'Stack test-stack\n' + + 'Resources\n' + + '[+] AWS::Lambda::Function Func', + ); + }); + + test('handles permission change type with both changeSet and IAM resources', () => { + // GIVEN + const templateWithIAM: cxapi.CloudFormationStackArtifact = { + template: { + Resources: { + Role: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [{ + Effect: 'Allow', + Principal: { Service: 'lambda.amazonaws.com' }, + Action: 'sts:AssumeRole', + }], + }, + }, + }, + }, + }, + templateFile: 'template.json', + stackName: 'test-stack', + findMetadataByType: () => [], + } as any; + + const mockChangeSet = { + ChangeSetName: 'test-changeset', + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Add', + LogicalResourceId: 'Role', + ResourceType: 'AWS::IAM::Role', + }, + }, + ], + Status: 'CREATE_COMPLETE', + $metadata: {}, + }; + + // WHEN + const formatter = new DiffFormatter({ + templateInfo: { + oldTemplate: {}, + newTemplate: templateWithIAM, + changeSet: mockChangeSet, + }, + }); + const result = formatter.formatStackDiff(); + + // THEN + expect(result.numStacksWithChanges).toBe(1); + expect(result.permissionChangeType).toBe('broadening'); + expect(result.formattedDiff).toBeDefined(); + }); }); describe('formatSecurityDiff', () => { diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index 2c10171aa..5e186fa01 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -4,6 +4,7 @@ import { RequireApproval } from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import type { ConfirmationRequest, DeploymentMethod, ToolkitAction, ToolkitOptions } from '@aws-cdk/toolkit-lib'; import { PermissionChangeType, Toolkit, ToolkitError } from '@aws-cdk/toolkit-lib'; +import type { DescribeChangeSetCommandOutput } from '@aws-sdk/client-cloudformation'; import * as chalk from 'chalk'; import * as chokidar from 'chokidar'; import * as fs from 'fs-extra'; @@ -478,30 +479,6 @@ export class CdkToolkit { return; } - if (requireApproval !== RequireApproval.NEVER) { - const currentTemplate = await this.props.deployments.readCurrentTemplate(stack); - const formatter = new DiffFormatter({ - templateInfo: { - oldTemplate: currentTemplate, - newTemplate: stack, - }, - }); - const securityDiff = formatter.formatSecurityDiff(); - if (requiresApproval(requireApproval, securityDiff.permissionChangeType)) { - const motivation = '"--require-approval" is enabled and stack includes security-sensitive updates'; - await this.ioHost.asIoHelper().defaults.info(securityDiff.formattedDiff); - 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). @@ -517,14 +494,107 @@ export class CdkToolkit { } } - 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); } + let deploymentMethod: DeploymentMethod | undefined; + + switch (requireApproval) { + case RequireApproval.BROADENING: { + const currentTemplate = await this.props.deployments.readCurrentTemplate(stack); + const formatter = new DiffFormatter({ + templateInfo: { + oldTemplate: currentTemplate, + newTemplate: stack, + }, + }); + const securityDiff = formatter.formatSecurityDiff(); + if (requiresApproval(requireApproval, securityDiff.permissionChangeType)) { + const motivation = '"--require-approval" is enabled and stack includes security-sensitive updates'; + await this.ioHost.asIoHelper().defaults.info(securityDiff.formattedDiff); + 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, + }), + ); + } + break; + } + + case RequireApproval.ANYCHANGE: { + let changeSet: DescribeChangeSetCommandOutput | undefined; + if (options.deploymentMethod?.method === 'change-set') { + // Create a CloudFormation change set + const changeSetName = options.deploymentMethod?.changeSetName || `cdk-deploy-change-set-${Date.now()}`; + await this.props.deployments.deployStack({ + stack, + deployName: stack.stackName, + roleArn: options.roleArn, + toolkitStackName: options.toolkitStackName, + reuseAssets: options.reuseAssets, + notificationArns, + tags, + deploymentMethod: { method: 'change-set' as const, changeSetName, execute: false }, + forceDeployment: options.force, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), + usePreviousParameters: options.usePreviousParameters, + extraUserAgent: options.extraUserAgent, + assetParallelism: options.assetParallelism, + ignoreNoStacks: options.ignoreNoStacks, + }); + + // Describe the change set to be presented to the user + changeSet = await this.props.deployments.describeChangeSet(stack, changeSetName); + + // Don't continue deploying the stack if there are no changes (unless forced) + if (!options.force && changeSet.ChangeSetName && (changeSet.Changes === undefined || changeSet.Changes.length === 0)) { + await this.ioHost.asIoHelper().defaults.info('No changes'); + await this.deleteChangeSet(stack, changeSet.ChangeSetName); + return; + } + + // Adjust the deployment method for the subsequent deployment to execute the existing change set + deploymentMethod = { ...options.deploymentMethod, changeSetName, executeExistingChangeSet: true }; + } + + // Present the diff to the user + const oldTemplate = await this.props.deployments.readCurrentTemplate(stack); + const formatter = new DiffFormatter({ templateInfo: { oldTemplate, newTemplate: stack, changeSet } }); + const diff = formatter.formatStackDiff(); + await this.ioHost.asIoHelper().defaults.info(diff.formattedDiff); + + // Prompt the user to confirm deployment + try { + const motivation = 'Approval required for stack deployment.'; + await askUserConfirmation( + this.ioHost, + IO.CDK_TOOLKIT_I5060.req(`${motivation}: 'Do you wish to deploy these changes'`, { + motivation, + concurrency, + permissionChangeType: diff.permissionChangeType, + templateDiffs: formatter.diffs, + }), + ); + } catch (e: unknown) { + if (changeSet?.ChangeSetName) { + await this.deleteChangeSet(stack, changeSet.ChangeSetName); + } + throw e; + } + break; + } + } + + 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(); + // There is already a startDeployTime constant, but that does not work with telemetry. // We should integrate the two in the future const deploySpan = await this.ioHost.asIoHelper().span(CLI_PRIVATE_SPAN.DEPLOY).begin({}); @@ -550,7 +620,7 @@ export class CdkToolkit { tags, execute: options.execute, changeSetName: options.changeSetName, - deploymentMethod: options.deploymentMethod, + deploymentMethod: deploymentMethod ?? options.deploymentMethod, forceDeployment: options.force, parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), usePreviousParameters: options.usePreviousParameters, @@ -1472,6 +1542,18 @@ export class CdkToolkit { stackName: assetNode.parentStack.stackName, })); } + + /** + * Delete a change set (with CLI-appropriate error handling) + */ + private async deleteChangeSet(stack: cxapi.CloudFormationStackArtifact, changeSetName: string): Promise { + try { + await this.props.deployments.deleteChangeSet(stack, changeSetName); + } catch (error) { + // Log but don't throw - change set cleanup failure shouldn't break deployment + await this.ioHost.asIoHelper().defaults.debug(`Failed to cleanup change set ${changeSetName} for stack ${stack.displayName}: ${error}`); + } + } } /** diff --git a/packages/aws-cdk/test/cli/cdk-toolkit.test.ts b/packages/aws-cdk/test/cli/cdk-toolkit.test.ts index 45c611918..3857ed6eb 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 { Manifest, RequireApproval } from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; 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 { ChangeSetStatus, DescribeStacksCommand, GetTemplateCommand, StackStatus } from '@aws-sdk/client-cloudformation'; import { GetParameterCommand } from '@aws-sdk/client-ssm'; import * as fs from 'fs-extra'; import type { Template, SdkProvider } from '../../lib/api'; @@ -645,6 +645,492 @@ describe('deploy', () => { expect(mockSynthesize).not.toHaveBeenCalled(); }); + describe('RequireApproval.ANYCHANGE', () => { + let toolkit: CdkToolkit; + let mockDeployments: Deployments; + + beforeEach(() => { + mockDeployments = new FakeCloudFormation({ + 'Test-Stack-A': { Foo: 'Bar' }, + 'Test-Stack-B': { Baz: 'Zinga!' }, + 'Test-Stack-C': { Baz: 'Zinga!' }, + }); + + toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockDeployments, + }); + }); + + test('creates change set, shows diff, prompts for approval, and deploys changes', async () => { + const mockDeployStack = jest.spyOn(mockDeployments, 'deployStack'); + + // WHEN + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'change-set', changeSetName: 'test-change-set' }, + }); + + // THEN + expect(mockDeployStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: { method: 'change-set', changeSetName: 'test-change-set', execute: false }, + }), + ); + expect(requestSpy).toHaveBeenCalled(); + expect(mockDeployStack).toHaveBeenCalledWith( + expect.objectContaining({ + deploymentMethod: { method: 'change-set', changeSetName: 'test-change-set', executeExistingChangeSet: true }, + }), + ); + expect(mockDeployStack).toHaveBeenCalledTimes(2); + }); + + test('deletes change set when there are no changes', async () => { + // GIVEN + const mockDeployStack = jest.spyOn(mockDeployments, 'deployStack'); + const mockDeleteChangeSet = jest.spyOn(mockDeployments, 'deleteChangeSet'); + jest.spyOn(mockDeployments, 'describeChangeSet').mockResolvedValue({ + ChangeSetId: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/cdk-change-set/12345', + ChangeSetName: 'cdk-change-set', + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/Test-Stack-A-Display-Name/12345', + Status: ChangeSetStatus.CREATE_COMPLETE, + Changes: [], + $metadata: {}, + }); + + // WHEN + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'change-set' } as DeploymentMethod, + }); + + // THEN + expect(mockDeleteChangeSet).toHaveBeenCalled(); + expect(mockDeployStack).toHaveBeenCalledTimes(1); + }); + + test('deletes change set when user rejects', async () => { + // GIVEN + const mockDeleteChangeSet = jest.spyOn(mockDeployments, 'deleteChangeSet'); + requestSpy.mockRejectedValue(new Error('Aborted by user')); + + // WHEN + const result = toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'change-set' } as DeploymentMethod, + }); + + // THEN + await expect(result).rejects.toThrow('Aborted by user'); + expect(mockDeleteChangeSet).toHaveBeenCalled(); + }); + + // Test that verifies the behavior when deleteChangeSet fails during a no-changes scenario. + // The deleteChangeSet method in cdk-toolkit.ts catches errors and logs them as debug messages + // rather than propagating them, ensuring that change set cleanup failures don't break deployments. + test('continues deployment when change set deletion fails (no changes scenario)', async () => { + // GIVEN + const mockDeleteChangeSet = jest.spyOn(mockDeployments, 'deleteChangeSet').mockRejectedValue(new Error('Failed to delete change set')); + jest.spyOn(mockDeployments, 'describeChangeSet').mockResolvedValue({ + ChangeSetId: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/cdk-change-set/12345', + ChangeSetName: 'cdk-change-set', + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/Test-Stack-A-Display-Name/12345', + Status: ChangeSetStatus.CREATE_COMPLETE, + Changes: [], + $metadata: {}, + }); + + // WHEN - deployment should complete successfully despite change set deletion failure + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'change-set' } as DeploymentMethod, + }); + + // THEN - verify deleteChangeSet was called but error was caught and didn't break deployment + expect(mockDeleteChangeSet).toHaveBeenCalled(); + expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + level: 'debug', + message: expect.stringContaining('Failed to cleanup change set'), + })); + expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + level: 'debug', + message: expect.stringContaining('cdk-change-set'), + })); + }); + + // Test that verifies the behavior when deleteChangeSet fails after user rejects the deployment. + // Even if the change set cleanup fails, the original user rejection error should be propagated, + // not the deletion failure. This ensures proper error handling and user feedback. + test('continues with user rejection when change set deletion fails', async () => { + // GIVEN + const mockDeleteChangeSet = jest.spyOn(mockDeployments, 'deleteChangeSet').mockRejectedValue(new Error('Failed to delete change set')); + requestSpy.mockRejectedValue(new Error('Aborted by user')); + + // WHEN + const result = toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.ANYCHANGE, + deploymentMethod: { method: 'change-set' } as DeploymentMethod, + }); + + // THEN - deployment should still be rejected by user (not by change set deletion failure) + await expect(result).rejects.toThrow('Aborted by user'); + expect(mockDeleteChangeSet).toHaveBeenCalled(); + expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + level: 'debug', + message: expect.stringContaining('Failed to cleanup change set'), + })); + }); + }); + + // Tests for RequireApproval.BROADENING mode, which prompts for approval only when + // security-sensitive changes broaden permissions. This is the default approval mode. + // It checks if IAM or security group changes expand permissions and requires user confirmation + // only for those broadening changes, while allowing non-broadening changes to proceed automatically. + describe('RequireApproval.BROADENING', () => { + let toolkit: CdkToolkit; + let mockDeployments: Deployments; + + beforeEach(() => { + mockDeployments = new FakeCloudFormation({ + 'Test-Stack-A': { Foo: 'Bar' }, + }); + + toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockDeployments, + }); + }); + + test('prompts for approval when security changes broaden permissions', async () => { + // GIVEN - mock readCurrentTemplate to return a template without IAM resources + jest.spyOn(mockDeployments, 'readCurrentTemplate').mockResolvedValue({ + Resources: {}, + }); + + // Mock the stack to have IAM resources (broadening change) + const stackWithIAM = { + ...MockStack.MOCK_STACK_A, + template: { + Resources: { + MyRole: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Principal: { Service: 'lambda.amazonaws.com' }, + Action: 'sts:AssumeRole', + }, + ], + }, + ManagedPolicyArns: ['arn:aws:iam::aws:policy/AdministratorAccess'], + }, + }, + }, + }, + }; + + cloudExecutable = await MockCloudExecutable.create({ + stacks: [stackWithIAM], + }); + + toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockDeployments, + }); + + // WHEN + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.BROADENING, + }); + + // THEN - verify that user was prompted for approval + expect(requestSpy).toHaveBeenCalledWith(expect.objectContaining({ + code: 'CDK_TOOLKIT_I5060', + data: expect.objectContaining({ + permissionChangeType: 'broadening', + }), + })); + }); + + // Verifies that when IAM permissions are reduced (narrowing change), no approval is required + test('does not prompt for approval when changes are non-broadening', async () => { + // GIVEN - mock readCurrentTemplate to return a template with existing IAM resources + jest.spyOn(mockDeployments, 'readCurrentTemplate').mockResolvedValue({ + Resources: { + MyRole: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Principal: { Service: 'lambda.amazonaws.com' }, + Action: 'sts:AssumeRole', + }, + ], + }, + ManagedPolicyArns: ['arn:aws:iam::aws:policy/AdministratorAccess'], + }, + }, + }, + }); + + // New template removes the managed policy (narrowing, not broadening) + const stackWithReducedPermissions = { + ...MockStack.MOCK_STACK_A, + template: { + Resources: { + MyRole: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Principal: { Service: 'lambda.amazonaws.com' }, + Action: 'sts:AssumeRole', + }, + ], + }, + }, + }, + }, + }, + }; + + cloudExecutable = await MockCloudExecutable.create({ + stacks: [stackWithReducedPermissions], + }); + + toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockDeployments, + }); + + // WHEN + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.BROADENING, + }); + + // THEN - verify that user was NOT prompted (no broadening changes) + expect(requestSpy).not.toHaveBeenCalled(); + }); + + // Verifies that when there are no IAM or security changes, no approval is required + test('does not prompt for approval when there are no IAM changes', async () => { + // GIVEN - mock readCurrentTemplate to return empty template + jest.spyOn(mockDeployments, 'readCurrentTemplate').mockResolvedValue({ + Resources: {}, + }); + + // WHEN + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.BROADENING, + }); + + // THEN - verify that user was NOT prompted (no security changes) + expect(requestSpy).not.toHaveBeenCalled(); + }); + + // Verifies the complete flow: prompt for approval → user accepts → deployment proceeds + test('deploys successfully when user approves broadening changes', async () => { + // GIVEN + const mockDeployStack = jest.spyOn(mockDeployments, 'deployStack'); + jest.spyOn(mockDeployments, 'readCurrentTemplate').mockResolvedValue({ + Resources: {}, + }); + + const stackWithIAM = { + ...MockStack.MOCK_STACK_A, + template: { + Resources: { + MyRole: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Principal: { Service: 's3.amazonaws.com' }, + Action: 'sts:AssumeRole', + }, + ], + }, + }, + }, + }, + }, + }; + + cloudExecutable = await MockCloudExecutable.create({ + stacks: [stackWithIAM], + }); + + toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockDeployments, + }); + + // WHEN - user approves the deployment + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.BROADENING, + }); + + // THEN - deployment should proceed + expect(requestSpy).toHaveBeenCalled(); + expect(mockDeployStack).toHaveBeenCalledTimes(1); + }); + + // Verifies the complete flow: prompt for approval → user rejects → deployment is cancelled + test('rejects deployment when user denies broadening changes', async () => { + // GIVEN + const mockDeployStack = jest.spyOn(mockDeployments, 'deployStack'); + jest.spyOn(mockDeployments, 'readCurrentTemplate').mockResolvedValue({ + Resources: {}, + }); + requestSpy.mockRejectedValue(new Error('User rejected')); + + const stackWithIAM = { + ...MockStack.MOCK_STACK_A, + template: { + Resources: { + MyRole: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Principal: { Service: 's3.amazonaws.com' }, + Action: 'sts:AssumeRole', + }, + ], + }, + }, + }, + }, + }, + }; + + cloudExecutable = await MockCloudExecutable.create({ + stacks: [stackWithIAM], + }); + + toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockDeployments, + }); + + // WHEN + const result = toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.BROADENING, + }); + + // THEN - deployment should be rejected + await expect(result).rejects.toThrow('User rejected'); + expect(requestSpy).toHaveBeenCalled(); + expect(mockDeployStack).not.toHaveBeenCalled(); + }); + + // Verifies that the security diff is displayed to the user before requesting approval, + // allowing them to review the specific IAM changes that are broadening permissions + test('displays security diff when prompting for broadening changes', async () => { + // GIVEN + jest.spyOn(mockDeployments, 'readCurrentTemplate').mockResolvedValue({ + Resources: {}, + }); + + const stackWithIAM = { + ...MockStack.MOCK_STACK_A, + template: { + Resources: { + MyRole: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Principal: { Service: 'lambda.amazonaws.com' }, + Action: 'sts:AssumeRole', + }, + ], + }, + }, + }, + }, + }, + }; + + cloudExecutable = await MockCloudExecutable.create({ + stacks: [stackWithIAM], + }); + + toolkit = new CdkToolkit({ + ioHost, + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: mockDeployments, + }); + + // WHEN + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + requireApproval: RequireApproval.BROADENING, + }); + + // THEN - verify security diff was displayed + expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + level: 'info', + message: expect.stringContaining('MyRole'), + })); + expect(requestSpy).toHaveBeenCalledWith(expect.objectContaining({ + data: expect.objectContaining({ + motivation: expect.stringContaining('--require-approval'), + permissionChangeType: 'broadening', + }), + })); + }); + }); + describe('readCurrentTemplate', () => { let template: any; let mockCloudExecutable: MockCloudExecutable; @@ -1941,6 +2427,29 @@ class FakeCloudFormation extends Deployments { throw new Error(`not an expected mock stack: ${stack.stackName}`); } } + + public describeChangeSet(stack: cxapi.CloudFormationStackArtifact, changeSetName: string): Promise { + return Promise.resolve({ + ChangeSetId: `arn:aws:cloudformation:us-east-1:123456789012:changeSet/${changeSetName}/12345`, + ChangeSetName: changeSetName, + StackId: `arn:aws:cloudformation:us-east-1:123456789012:stack/${stack.stackName}/12345`, + Status: 'CREATE_COMPLETE', + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Modify', + LogicalResourceId: 'TestResource', + ResourceType: 'AWS::S3::Bucket', + }, + }, + ], + }); + } + + public deleteChangeSet(_stack: cxapi.CloudFormationStackArtifact, _changeSetName: string): Promise { + return Promise.resolve(); + } } function cliTest(name: string, handler: (dir: string) => void | Promise): void {