Skip to content

Commit

Permalink
chore(toolkit): requireApproval option for deploy (#32977)
Browse files Browse the repository at this point in the history
adds toolkit tests for deploy

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Jan 17, 2025
1 parent ebe9580 commit 794520c
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 28 deletions.
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
*/
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
*/
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>;

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

0 comments on commit 794520c

Please sign in to comment.