Skip to content

Commit

Permalink
require approval is live
Browse files Browse the repository at this point in the history
  • Loading branch information
kaizencc committed Jan 17, 2025
1 parent ced7269 commit 77863b7
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 26 deletions.
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';
24 changes: 12 additions & 12 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 @@ -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
36 changes: 22 additions & 14 deletions packages/@aws-cdk/toolkit/test/actions/deploy.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import * as s3 from 'aws-cdk-lib/aws-s3';
import * as core from 'aws-cdk-lib/core';
import { Toolkit } from '../../lib/toolkit';
import { fixture, TestIoHost } from '../_helpers';
import { CloudFormationClient, DescribeChangeSetCommand, DescribeStacksCommand, StackStatus } from '@aws-sdk/client-cloudformation';
import * as iam from 'aws-cdk-lib/aws-iam';
import * as core from 'aws-cdk-lib/core';
import { mockClient } from 'aws-sdk-client-mock';
import { StackSelectionStrategy } from '../../lib';
import { RequireApproval, StackSelectionStrategy } from '../../lib';
import { Toolkit } from '../../lib/toolkit';
import { TestIoHost } from '../_helpers';

const ioHost = new TestIoHost();
const notifySpy = jest.spyOn(ioHost, 'notify');
Expand All @@ -23,10 +23,6 @@ const cxFromBuilder = async () => {
});
};

const cxFromApp = async (name: string) => {
return cdk.fromCdkApp(`node ${fixture(name)}`);
};

beforeEach(() => {
requestResponseSpy.mockClear();
notifySpy.mockClear();
Expand Down Expand Up @@ -70,19 +66,31 @@ describe('deploy', () => {
}));
});

test('deploy from app', async () => {
test('request response when require approval is set', async () => {
// WHEN
await cdk.deploy(await cxFromApp('two-empty-stacks'));
const cx = await cdk.fromAssemblyBuilder(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;
});

await cdk.deploy(cx, {
requireApproval: RequireApproval.ANY_CHANGE,
});

// THEN
expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({
expect(requestResponseSpy).toHaveBeenCalledWith(expect.objectContaining({
action: 'deploy',
level: 'info',
message: expect.stringContaining('Deployment time:'),
code: 'CDK_TOOLKIT_I5060',
message: expect.stringContaining('Do you wish to deploy these changes'),
}));
});

test('deploy no resources results in warning', async () => {
test('stack information is returned when successfully deployed', async () => {
// WHEN
const cx = await cxFromBuilder();
await cdk.deploy(cx, {
Expand Down

0 comments on commit 77863b7

Please sign in to comment.