From a544e329542c8c3d6c43f18a41e48151aafc32ee Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 20 Feb 2019 10:56:31 +0100 Subject: [PATCH] fix(toolkit): only fail if errors are on selected stacks If we report synthesis errors, they should only prevent the affected stack from deploying; right now, since they are checked during the synthesis step, they would abort the entire toolkit run regardless of whether they would be selected or not. Makes it possible to do region-dependent verification. Fixes #1784. ALSO IN THIS COMMIT If the 'cdk synth' output goes to the screen, don't automatically select upstream stacks for synthesis (as that would lead to an immediate error). Fixes #1783. --- packages/aws-cdk/bin/cdk.ts | 12 +++- packages/aws-cdk/lib/api/cxapp/stacks.ts | 85 +++++++++++++++++------- packages/aws-cdk/lib/api/util/sdk.ts | 2 +- packages/aws-cdk/test/api/test.stacks.ts | 69 +++++++++++++++++++ 4 files changed, 142 insertions(+), 26 deletions(-) create mode 100644 packages/aws-cdk/test/api/test.stacks.ts diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index ff07687969d03..d9abddaaa0b03 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -9,6 +9,7 @@ import yargs = require('yargs'); import { bootstrapEnvironment, deployStack, destroyStack, loadToolkitInfo, Mode, SDK } from '../lib'; import { environmentsFromDescriptors, globEnvironmentsFromStacks } from '../lib/api/cxapp/environments'; +import { execProgram } from '../lib/api/cxapp/exec'; import { AppStacks, ExtendedStackSelection, listStackNames } from '../lib/api/cxapp/stacks'; import { leftPad } from '../lib/api/util/string-manipulation'; import { printSecurityDiff, printStackDiff, RequireApproval } from '../lib/diff'; @@ -106,7 +107,11 @@ async function initCommandLine() { await configuration.load(); configuration.logDefaults(); - const appStacks = new AppStacks(argv, configuration, aws); + const appStacks = new AppStacks({ + verbose: argv.trace || argv.verbose, + ignoreErrors: argv.ignoreErrors, + strict: argv.strict, + configuration, aws, synthesizer: execProgram }); const renames = parseRenames(argv.rename); @@ -249,7 +254,10 @@ async function initCommandLine() { outputDir: string|undefined, json: boolean, numbered: boolean): Promise { - const stacks = await appStacks.selectStacks(stackNames, exclusively ? ExtendedStackSelection.None : ExtendedStackSelection.Upstream); + // Only autoselect dependencies if it doesn't interfere with user request or output options + const autoSelectDependencies = !exclusively && outputDir !== undefined; + + const stacks = await appStacks.selectStacks(stackNames, autoSelectDependencies ? ExtendedStackSelection.Upstream : ExtendedStackSelection.None); renames.validateSelectedStacks(stacks); if (doInteractive) { diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index b6def12806b04..7ae9ee348a79a 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -1,14 +1,52 @@ import cxapi = require('@aws-cdk/cx-api'); import colors = require('colors/safe'); import minimatch = require('minimatch'); -import yargs = require('yargs'); import contextproviders = require('../../context-providers'); import { debug, error, print, warning } from '../../logging'; -import { Configuration } from '../../settings'; +import { Configuration, Settings } from '../../settings'; import cdkUtil = require('../../util'); import { SDK } from '../util/sdk'; import { topologicalSort } from '../util/toposort'; -import { execProgram } from './exec'; + +type Synthesizer = (aws: SDK, config: Settings) => Promise; + +export interface AppStacksProps { + /** + * Whether to be verbose + * + * @default false + */ + verbose?: boolean; + + /** + * Don't stop on error metadata + * + * @default false + */ + ignoreErrors?: boolean; + + /** + * Treat warnings in metadata as errors + * + * @default false + */ + strict?: boolean; + + /** + * Application configuration (settings and context) + */ + configuration: Configuration; + + /** + * AWS object (used by synthesizer and contextprovider) + */ + aws: SDK; + + /** + * Callback invoked to synthesize the actual stacks + */ + synthesizer: Synthesizer; +} /** * Routines to get stacks from an app @@ -22,7 +60,7 @@ export class AppStacks { */ private cachedResponse?: cxapi.SynthesizeResponse; - constructor(private readonly argv: yargs.Arguments, private readonly configuration: Configuration, private readonly aws: SDK) { + constructor(private readonly props: AppStacksProps) { } /** @@ -76,7 +114,11 @@ export class AppStacks { } // Filter original array because it is in the right order - return stacks.filter(s => selectedStacks.has(s.name)); + const selectedList = stacks.filter(s => selectedStacks.has(s.name)); + + // Only check selected stacks for errors + this.processMessages(selectedList); + return selectedList; } /** @@ -112,34 +154,24 @@ export class AppStacks { return this.cachedResponse; } - const trackVersions: boolean = this.configuration.combined.get(['versionReporting']); + const trackVersions: boolean = this.props.configuration.combined.get(['versionReporting']); // We may need to run the cloud executable multiple times in order to satisfy all missing context while (true) { - const response: cxapi.SynthesizeResponse = await execProgram(this.aws, this.configuration.combined); + const response: cxapi.SynthesizeResponse = await this.props.synthesizer(this.props.aws, this.props.configuration.combined); const allMissing = cdkUtil.deepMerge(...response.stacks.map(s => s.missing)); if (!cdkUtil.isEmpty(allMissing)) { debug(`Some context information is missing. Fetching...`); - await contextproviders.provideContextValues(allMissing, this.configuration.projectConfig, this.aws); + await contextproviders.provideContextValues(allMissing, this.props.configuration.projectConfig, this.props.aws); // Cache the new context to disk - await this.configuration.saveProjectConfig(); + await this.props.configuration.saveProjectConfig(); continue; } - const { errors, warnings } = this.processMessages(response); - - if (errors && !this.argv.ignoreErrors) { - throw new Error('Found errors'); - } - - if (this.argv.strict && warnings) { - throw new Error('Found warnings (--strict mode)'); - } - if (trackVersions && response.runtime) { const modules = formatModules(response.runtime); for (const stack of response.stacks) { @@ -181,10 +213,10 @@ export class AppStacks { /** * Extracts 'aws:cdk:warning|info|error' metadata entries from the stack synthesis */ - private processMessages(stacks: cxapi.SynthesizeResponse): { errors: boolean, warnings: boolean } { + private processMessages(stacks: cxapi.SynthesizedStack[]) { let warnings = false; let errors = false; - for (const stack of stacks.stacks) { + for (const stack of stacks) { for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; for (const entry of metadata) { @@ -204,13 +236,20 @@ export class AppStacks { } } } - return { warnings, errors }; + + if (errors && !this.props.ignoreErrors) { + throw new Error('Found errors'); + } + + if (this.props.strict && warnings) { + throw new Error('Found warnings (--strict mode)'); + } } private printMessage(logFn: (s: string) => void, prefix: string, id: string, entry: cxapi.MetadataEntry) { logFn(`[${prefix} at ${id}] ${entry.data}`); - if (this.argv.trace || this.argv.verbose) { + if (this.props.verbose) { logFn(` ${entry.trace.join('\n ')}`); } } diff --git a/packages/aws-cdk/lib/api/util/sdk.ts b/packages/aws-cdk/lib/api/util/sdk.ts index a22752c1f5807..639667dd2de16 100644 --- a/packages/aws-cdk/lib/api/util/sdk.ts +++ b/packages/aws-cdk/lib/api/util/sdk.ts @@ -50,7 +50,7 @@ export class SDK { private readonly credentialsCache: CredentialsCache; private readonly profile?: string; - constructor(options: SDKOptions) { + constructor(options: SDKOptions = {}) { this.profile = options.profile; const defaultCredentialProvider = makeCLICompatibleCredentialProvider(options.profile, options.ec2creds); diff --git a/packages/aws-cdk/test/api/test.stacks.ts b/packages/aws-cdk/test/api/test.stacks.ts new file mode 100644 index 0000000000000..ff17b596334d2 --- /dev/null +++ b/packages/aws-cdk/test/api/test.stacks.ts @@ -0,0 +1,69 @@ +import cxapi = require('@aws-cdk/cx-api'); +import { Test } from 'nodeunit'; +import { SDK } from '../../lib'; +import { AppStacks, ExtendedStackSelection } from '../../lib/api/cxapp/stacks'; +import { Configuration } from '../../lib/settings'; + +const FIXED_RESULT: cxapi.SynthesizeResponse = { + version: '1', + stacks: [ + { + name: 'withouterrors', + template: { resource: 'noerrorresource' }, + environment: { name: 'dev', account: '12345', region: 'here' }, + metadata: {}, + }, + { + name: 'witherrors', + template: { resource: 'errorresource' }, + environment: { name: 'dev', account: '12345', region: 'here' }, + metadata: { + '/resource': [ + { + type: cxapi.ERROR_METADATA_KEY, + data: 'this is an error', + trace: [] + } + ] + } + } + ] +}; + +export = { + async 'do not throw when selecting stack without errors'(test: Test) { + // GIVEN + const stacks = new AppStacks({ + configuration: new Configuration(), + aws: new SDK(), + synthesizer: async () => FIXED_RESULT, + }); + + // WHEN + const selected = await stacks.selectStacks(['withouterrors'], ExtendedStackSelection.None); + + // THEN + test.equal(selected[0].template.resource, 'noerrorresource'); + + test.done(); + }, + + async 'do throw when selecting stack with errors'(test: Test) { + // GIVEN + const stacks = new AppStacks({ + configuration: new Configuration(), + aws: new SDK(), + synthesizer: async () => FIXED_RESULT, + }); + + // WHEN + try { + await stacks.selectStacks(['witherrors'], ExtendedStackSelection.None); + test.ok(false, 'Did not get exception'); + } catch (e) { + test.ok(/Found errors/.test(e.toString()), 'Wrong error'); + } + + test.done(); + }, +}; \ No newline at end of file