diff --git a/packages/@aws-cdk/toolkit/lib/actions/deploy/index.ts b/packages/@aws-cdk/toolkit/lib/actions/deploy/index.ts index 60098cd16fb5a..b92b5ec411602 100644 --- a/packages/@aws-cdk/toolkit/lib/actions/deploy/index.ts +++ b/packages/@aws-cdk/toolkit/lib/actions/deploy/index.ts @@ -110,8 +110,10 @@ export class StackParameters { export interface BaseDeployOptions { /** * Criteria for selecting stacks to deploy + * + * @default - all stacks */ - readonly stacks: StackSelector; + readonly stacks?: StackSelector; /** * @deprecated set on toolkit @@ -148,9 +150,9 @@ export interface BaseDeployOptions { * A 'hotswap' deployment will attempt to short-circuit CloudFormation * and update the affected resources like Lambda functions directly. * - * @default - `HotswapMode.FALL_BACK` for regular deployments, `HotswapMode.HOTSWAP_ONLY` for 'watch' deployments + * @default - no hotswap */ - readonly hotswap: HotswapMode; + readonly hotswap?: HotswapMode; /** * Rollback failed deployments @@ -182,7 +184,7 @@ export interface DeployOptions extends BaseDeployOptions { /** * What kind of security changes require approval * - * @default RequireApproval.Broadening + * @default RequireApproval.NEVER */ readonly requireApproval?: RequireApproval; diff --git a/packages/@aws-cdk/toolkit/lib/actions/diff/private/helpers.ts b/packages/@aws-cdk/toolkit/lib/actions/diff/private/helpers.ts new file mode 100644 index 0000000000000..98155f3d02c0e --- /dev/null +++ b/packages/@aws-cdk/toolkit/lib/actions/diff/private/helpers.ts @@ -0,0 +1,24 @@ +import { DescribeChangeSetOutput, fullDiff } from '@aws-cdk/cloudformation-diff'; +import * as cxapi from '@aws-cdk/cx-api'; +import { ToolkitError } from '../../../api/errors'; +import { RequireApproval } from '../../deploy'; + +/** + * Return whether the diff has security-impacting changes that need confirmation + */ +export function diffRequiresApproval( + oldTemplate: any, + newTemplate: cxapi.CloudFormationStackArtifact, + requireApproval: RequireApproval, + changeSet?: DescribeChangeSetOutput, +): boolean { + // @todo return or print the full diff. + const diff = fullDiff(oldTemplate, newTemplate.template, changeSet); + + switch (requireApproval) { + case RequireApproval.NEVER: return false; + case RequireApproval.ANY_CHANGE: return diff.permissionsAnyChanges; + case RequireApproval.BROADENING: return diff.permissionsBroadened; + default: throw new ToolkitError(`Unrecognized approval level: ${requireApproval}`); + } +} diff --git a/packages/@aws-cdk/toolkit/lib/actions/diff/private/index.ts b/packages/@aws-cdk/toolkit/lib/actions/diff/private/index.ts new file mode 100644 index 0000000000000..c5f595cf9d428 --- /dev/null +++ b/packages/@aws-cdk/toolkit/lib/actions/diff/private/index.ts @@ -0,0 +1 @@ +export * from './helpers'; diff --git a/packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts index 2cc77d2adaa61..cdc8d63f1a34b 100644 --- a/packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts @@ -8,6 +8,7 @@ import { AssetBuildTime, DeployOptions, RequireApproval } from '../actions/deplo import { buildParameterMap, removePublishedAssets } from '../actions/deploy/private'; import { DestroyOptions } from '../actions/destroy'; import { DiffOptions } from '../actions/diff'; +import { diffRequiresApproval } from '../actions/diff/private'; import { ListOptions } from '../actions/list'; import { RollbackOptions } from '../actions/rollback'; import { SynthOptions } from '../actions/synth'; @@ -183,11 +184,11 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab /** * Deploys the selected stacks into an AWS account */ - public async deploy(cx: ICloudAssemblySource, options: DeployOptions): Promise { + public async deploy(cx: ICloudAssemblySource, options: DeployOptions = {}): Promise { const ioHost = withAction(this.ioHost, 'deploy'); const timer = Timer.start(); const assembly = await this.assemblyFromSource(cx); - const stackCollection = assembly.selectStacksV2(options.stacks); + const stackCollection = assembly.selectStacksV2(options.stacks ?? ALL_STACKS); await this.validateStacksMetadata(stackCollection, ioHost); const synthTime = timer.end(); @@ -207,7 +208,7 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab }); await migrator.tryMigrateResources(stackCollection, options); - // const requireApproval = options.requireApproval ?? RequireApproval.BROADENING; + const requireApproval = options.requireApproval ?? RequireApproval.NEVER; const parameterMap = buildParameterMap(options.parameters?.parameters); @@ -281,17 +282,16 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab return; } - // @TODO - // if (requireApproval !== RequireApproval.NEVER) { - // const currentTemplate = await deployments.readCurrentTemplate(stack); - // if (printSecurityDiff(currentTemplate, stack, requireApproval)) { - // await askUserConfirmation( - // concurrency, - // '"--require-approval" is enabled and stack includes security-sensitive updates', - // 'Do you wish to deploy these changes', - // ); - // } - // } + if (requireApproval !== RequireApproval.NEVER) { + const currentTemplate = await deployments.readCurrentTemplate(stack); + if (diffRequiresApproval(currentTemplate, stack, requireApproval)) { + const motivation = '"--require-approval" is enabled and stack includes security-sensitive updates.'; + const question = `${motivation}\nDo you wish to deploy these changes`; + // @todo reintroduce concurrency and corked logging in CliHost + const confirmed = await ioHost.requestResponse(confirm('CDK_TOOLKIT_I5060', question, motivation, true, concurrency)); + if (!confirmed) { throw new ToolkitError('Aborted by user'); } + } + } // Following are the same semantics we apply with respect to Notification ARNs (dictated by the SDK) // diff --git a/packages/@aws-cdk/toolkit/package.json b/packages/@aws-cdk/toolkit/package.json index cf62cf7c97389..e8bc1c5950683 100644 --- a/packages/@aws-cdk/toolkit/package.json +++ b/packages/@aws-cdk/toolkit/package.json @@ -50,6 +50,7 @@ "@types/node": "^18.18.14", "aws-cdk": "0.0.0", "aws-cdk-lib": "0.0.0", + "aws-sdk-client-mock": "^4.0.1", "esbuild": "^0.24.0", "jest": "^29.7.0", "typedoc": "^0.27.6", diff --git a/packages/@aws-cdk/toolkit/test/_fixtures/stack-with-role/index.ts b/packages/@aws-cdk/toolkit/test/_fixtures/stack-with-role/index.ts new file mode 100644 index 0000000000000..38c3f675b7549 --- /dev/null +++ b/packages/@aws-cdk/toolkit/test/_fixtures/stack-with-role/index.ts @@ -0,0 +1,11 @@ +import * as iam from 'aws-cdk-lib/aws-iam'; +import * as core from 'aws-cdk-lib/core'; + +export default async() => { + const app = new core.App(); + const stack = new core.Stack(app, 'Stack1'); + new iam.Role(stack, 'Role', { + assumedBy: new iam.ArnPrincipal('arn'), + }); + return app.synth() as any; +}; diff --git a/packages/@aws-cdk/toolkit/test/_helpers/test-io-host.ts b/packages/@aws-cdk/toolkit/test/_helpers/test-io-host.ts index d8d0dd1641a3d..8b80aeed1602d 100644 --- a/packages/@aws-cdk/toolkit/test/_helpers/test-io-host.ts +++ b/packages/@aws-cdk/toolkit/test/_helpers/test-io-host.ts @@ -5,20 +5,24 @@ import { IIoHost, IoMessage, IoMessageLevel, IoRequest, isMessageRelevantForLeve * Optional set a level to filter out all irrelevant messages. */ export class TestIoHost implements IIoHost { - public readonly spy: jest.Mock; + public readonly notifySpy: jest.Mock; + public readonly requestSpy: jest.Mock; constructor(public level: IoMessageLevel = 'info') { - this.spy = jest.fn(); + this.notifySpy = jest.fn(); + this.requestSpy = jest.fn(); } public async notify(msg: IoMessage): Promise { if (isMessageRelevantForLevel(msg, this.level)) { - this.spy(msg); + this.notifySpy(msg); } } public async requestResponse(msg: IoRequest): Promise { - await this.notify(msg); + if (isMessageRelevantForLevel(msg, this.level)) { + this.requestSpy(msg); + } return msg.defaultResponse; } } diff --git a/packages/@aws-cdk/toolkit/test/actions/deploy.test.ts b/packages/@aws-cdk/toolkit/test/actions/deploy.test.ts new file mode 100644 index 0000000000000..8604cf5bc4648 --- /dev/null +++ b/packages/@aws-cdk/toolkit/test/actions/deploy.test.ts @@ -0,0 +1,76 @@ +import { RequireApproval } from '../../lib'; +import { Toolkit } from '../../lib/toolkit'; +import { builderFixture, TestIoHost } from '../_helpers'; + +const ioHost = new TestIoHost(); +const toolkit = new Toolkit({ ioHost }); + +jest.mock('../../lib/api/aws-cdk', () => { + return { + ...jest.requireActual('../../lib/api/aws-cdk'), + Deployments: jest.fn().mockImplementation(() => ({ + deployStack: jest.fn().mockResolvedValue({ + type: 'did-deploy-stack', + stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack', + outputs: {}, + noOp: false, + }), + resolveEnvironment: jest.fn().mockResolvedValue({}), + isSingleAssetPublished: jest.fn().mockResolvedValue(true), + readCurrentTemplate: jest.fn().mockResolvedValue({ Resources: {} }), + })), + }; +}); + +beforeEach(() => { + ioHost.notifySpy.mockClear(); + ioHost.requestSpy.mockClear(); + jest.clearAllMocks(); +}); + +describe('deploy', () => { + test('deploy from builder', async () => { + // WHEN + const cx = await builderFixture(toolkit, 'two-empty-stacks'); + await toolkit.deploy(cx); + + // THEN + expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + action: 'deploy', + level: 'info', + message: expect.stringContaining('Deployment time:'), + })); + }); + + test('request response when require approval is set', async () => { + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + requireApproval: RequireApproval.ANY_CHANGE, + }); + + // THEN + expect(ioHost.requestSpy).toHaveBeenCalledWith(expect.objectContaining({ + action: 'deploy', + level: 'info', + code: 'CDK_TOOLKIT_I5060', + message: expect.stringContaining('Do you wish to deploy these changes'), + })); + }); + + test('skips response by default', async () => { + // WHEN + const cx = await builderFixture(toolkit, 'stack-with-role'); + await toolkit.deploy(cx, { + requireApproval: RequireApproval.NEVER, + }); + + // THEN + expect(ioHost.requestSpy).not.toHaveBeenCalledWith(expect.objectContaining({ + action: 'deploy', + level: 'info', + code: 'CDK_TOOLKIT_I5060', + message: expect.stringContaining('Do you wish to deploy these changes'), + })); + }); +}); diff --git a/packages/@aws-cdk/toolkit/test/actions/synth.test.ts b/packages/@aws-cdk/toolkit/test/actions/synth.test.ts index 3b64136b1d397..79b1af63c9835 100644 --- a/packages/@aws-cdk/toolkit/test/actions/synth.test.ts +++ b/packages/@aws-cdk/toolkit/test/actions/synth.test.ts @@ -5,7 +5,8 @@ const ioHost = new TestIoHost(); const toolkit = new Toolkit({ ioHost }); beforeEach(() => { - ioHost.spy.mockClear(); + ioHost.notifySpy.mockClear(); + ioHost.requestSpy.mockClear(); }); describe('synth', () => { @@ -15,7 +16,7 @@ describe('synth', () => { await toolkit.synth(cx); // THEN - expect(ioHost.spy).toHaveBeenCalledWith(expect.objectContaining({ + expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({ action: 'synth', level: 'info', message: expect.stringContaining('Successfully synthesized'), @@ -28,7 +29,7 @@ describe('synth', () => { await toolkit.synth(cx); // THEN - expect(ioHost.spy).toHaveBeenCalledWith(expect.objectContaining({ + expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({ action: 'synth', level: 'info', message: expect.stringContaining('Successfully synthesized'), @@ -41,7 +42,7 @@ describe('synth', () => { await toolkit.synth(cx); // THEN - expect(ioHost.spy).toHaveBeenCalledWith(expect.objectContaining({ + expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({ action: 'synth', level: 'info', code: 'CDK_TOOLKIT_I0001', @@ -62,7 +63,7 @@ describe('synth', () => { await toolkit.synth(await appFixture(toolkit, 'two-empty-stacks')); // THEN - expect(ioHost.spy).toHaveBeenCalledWith(expect.objectContaining({ + expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({ action: 'synth', level: 'info', code: 'CDK_TOOLKIT_I0002', diff --git a/packages/@aws-cdk/toolkit/test/api/cloud-assembly/source-builder.test.ts b/packages/@aws-cdk/toolkit/test/api/cloud-assembly/source-builder.test.ts index a69c4992591e2..7f3f1b4283dd7 100644 --- a/packages/@aws-cdk/toolkit/test/api/cloud-assembly/source-builder.test.ts +++ b/packages/@aws-cdk/toolkit/test/api/cloud-assembly/source-builder.test.ts @@ -5,7 +5,8 @@ const ioHost = new TestIoHost(); const toolkit = new Toolkit({ ioHost }); beforeEach(() => { - ioHost.spy.mockClear(); + ioHost.notifySpy.mockClear(); + ioHost.requestSpy.mockClear(); }); describe('fromAssemblyBuilder', () => {