Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(toolkit): requireApproval option for deploy #32977

Merged
merged 11 commits into from
Jan 17, 2025
10 changes: 6 additions & 4 deletions packages/@aws-cdk/toolkit/lib/actions/deploy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
mrgrain marked this conversation as resolved.
Show resolved Hide resolved
*/
readonly hotswap: HotswapMode;
readonly hotswap?: HotswapMode;

/**
* Rollback failed deployments
Expand Down Expand Up @@ -182,7 +184,7 @@ export interface DeployOptions extends BaseDeployOptions {
/**
* What kind of security changes require approval
*
* @default RequireApproval.Broadening
* @default RequireApproval.NEVER
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
*/
readonly requireApproval?: RequireApproval;

Expand Down
24 changes: 24 additions & 0 deletions packages/@aws-cdk/toolkit/lib/actions/diff/private/helpers.ts
Original file line number Diff line number Diff line change
@@ -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}`);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './helpers';
28 changes: 14 additions & 14 deletions packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<void> {
public async deploy(cx: ICloudAssemblySource, options: DeployOptions = {}): Promise<void> {
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();
Expand All @@ -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);

Expand Down Expand Up @@ -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)
//
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/toolkit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
11 changes: 11 additions & 0 deletions packages/@aws-cdk/toolkit/test/_fixtures/stack-with-role/index.ts
Original file line number Diff line number Diff line change
@@ -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;
};
12 changes: 8 additions & 4 deletions packages/@aws-cdk/toolkit/test/_helpers/test-io-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any, any, any>;
public readonly notifySpy: jest.Mock<any, any, any>;
public readonly requestSpy: jest.Mock<any, any, any>;
Comment on lines +8 to +9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!


constructor(public level: IoMessageLevel = 'info') {
this.spy = jest.fn();
this.notifySpy = jest.fn();
this.requestSpy = jest.fn();
}

public async notify<T>(msg: IoMessage<T>): Promise<void> {
if (isMessageRelevantForLevel(msg, this.level)) {
this.spy(msg);
this.notifySpy(msg);
}
}

public async requestResponse<T, U>(msg: IoRequest<T, U>): Promise<U> {
await this.notify(msg);
if (isMessageRelevantForLevel(msg, this.level)) {
this.requestSpy(msg);
}
return msg.defaultResponse;
}
}
76 changes: 76 additions & 0 deletions packages/@aws-cdk/toolkit/test/actions/deploy.test.ts
Original file line number Diff line number Diff line change
@@ -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'),
}));
});
});
11 changes: 6 additions & 5 deletions packages/@aws-cdk/toolkit/test/actions/synth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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'),
Expand All @@ -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'),
Expand All @@ -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',
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading