Skip to content

Commit

Permalink
feat(cli): add --no-lookups flag to disable context lookups (#11489)
Browse files Browse the repository at this point in the history
Context lookups are supposed to be performed on developer desktops, and
committed to `cdk.context.json`. If you don't, your CI build might try
to perform a lookup and fail with an unclear error message about
permissions, or worse: appear to work properly but leave you with a
nondeterministic build.

Introduce a CLI flag called `--no-lookups` that throws an appropriately
descriptive error message if you forgot to perform context lookups
before committing.

This now also makes it possible to write an integration test for
PR #11461.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Nov 24, 2020
1 parent 94fbddd commit 0445a6e
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 39 deletions.
3 changes: 3 additions & 0 deletions packages/aws-cdk/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ than one test will run at a time in that region.
If `AWS_REGIONS` is not set, all tests will sequentially run in the one
region set in `AWS_REGION`.

Run with `env INTEG_NO_CLEAN=1` to forego cleaning up the temporary directory,
in order to be able to debug 'cdk synth' output.

### CLI integration tests

CLI tests will exercise a number of common CLI scenarios, and deploy actual
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/bin/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ async function parseCommandLineArguments() {
.option('plugin', { type: 'array', alias: 'p', desc: 'Name or path of a node package that extend the CDK features. Can be specified multiple times', nargs: 1 })
.option('trace', { type: 'boolean', desc: 'Print trace for stack warnings' })
.option('strict', { type: 'boolean', desc: 'Do not construct stacks with warnings' })
.option('lookups', { type: 'boolean', desc: 'Perform context lookups (synthesis fails if this is disabled and context lookups need to be performed)', default: true })
.option('ignore-errors', { type: 'boolean', default: false, desc: 'Ignores synthesis errors, which will likely produce an invalid output' })
.option('json', { type: 'boolean', alias: 'j', desc: 'Use JSON output instead of YAML when templates are printed to STDOUT', default: false })
.option('verbose', { type: 'boolean', alias: 'v', desc: 'Show debug logs (specify multiple times to increase verbosity)', default: false })
Expand Down
13 changes: 12 additions & 1 deletion packages/aws-cdk/lib/api/cxapp/cloud-executable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,16 @@ export class CloudExecutable {
while (true) {
const assembly = await this.props.synthesizer(this.props.sdkProvider, this.props.configuration);

if (assembly.manifest.missing) {
if (assembly.manifest.missing && assembly.manifest.missing.length > 0) {
const missingKeys = missingContextKeys(assembly.manifest.missing);

if (!this.canLookup) {
throw new Error(
'Context lookups have been disabled. '
+ 'Make sure all necessary context is already in \'cdk.context.json\' by running \'cdk synth\' on a machine with sufficient AWS credentials and committing the result. '
+ `Missing context keys: '${Array.from(missingKeys).join(', ')}'`);
}

let tryLookup = true;
if (previouslyMissingKeys && setsEqual(missingKeys, previouslyMissingKeys)) {
debug('Not making progress trying to resolve environmental context. Giving up.');
Expand Down Expand Up @@ -162,6 +169,10 @@ export class CloudExecutable {
await fs.writeFile(stack.templateFullPath, JSON.stringify(stack.template, undefined, 2), { encoding: 'utf-8' });
}
}

private get canLookup() {
return !!(this.props.configuration.settings.get(['lookups']) ?? true);
}
}

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk/lib/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type Arguments = {
readonly _: [Command, ...string[]];
readonly exclusively?: boolean;
readonly STACKS?: string[];
readonly lookups?: boolean;
readonly [name: string]: unknown;
};

Expand Down Expand Up @@ -245,6 +246,7 @@ export class Settings {
output: argv.output,
progress: argv.progress,
bundlingStacks,
lookups: argv.lookups,
});
}

Expand Down
19 changes: 19 additions & 0 deletions packages/aws-cdk/test/api/cloud-executable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,25 @@ test('stop executing if context providers are not making progress', async () =>
// THEN: the test finishes normally});
});

test('fails if lookups are disabled and missing context is synthesized', async () => {
// GIVEN
const cloudExecutable = new MockCloudExecutable({
stacks: [{
stackName: 'thestack',
template: { resource: 'noerrorresource' },
}],
// Always return the same missing keys, synthesis should still finish.
missing: [
{ key: 'abcdef', props: { account: '1324', region: 'us-east-1' }, provider: cxschema.ContextProvider.AVAILABILITY_ZONE_PROVIDER },
],
});
cloudExecutable.configuration.settings.set(['lookups'], false);

// WHEN
await expect(cloudExecutable.synthesize()).rejects.toThrow(/Context lookups have been disabled/);
});


async function testCloudExecutable({ env, versionReporting = true }: { env?: string, versionReporting?: boolean } = {}) {
const cloudExec = new MockCloudExecutable({
stacks: [{
Expand Down
117 changes: 80 additions & 37 deletions packages/aws-cdk/test/integ/cli/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@ class YourStack extends cdk.Stack {
}
}

class StackUsingContext extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
new core.CfnResource(this, 'Handle', {
type: 'AWS::CloudFormation::WaitConditionHandle'
});

new core.CfnOutput(this, 'Output', {
value: this.availabilityZones,
});
}
}

class ParameterStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
Expand Down Expand Up @@ -247,53 +260,83 @@ class SomeStage extends cdk.Stage {
}
}

class StageUsingContext extends cdk.Stage {
constructor(parent, id, props) {
super(parent, id, props);

new StackUsingContext(this, 'StackInStage');
}
}

const app = new cdk.App();

const defaultEnv = {
account: process.env.CDK_DEFAULT_ACCOUNT,
region: process.env.CDK_DEFAULT_REGION
};

// Deploy all does a wildcard ${stackPrefix}-test-*
new MyStack(app, `${stackPrefix}-test-1`, { env: defaultEnv });
new YourStack(app, `${stackPrefix}-test-2`);
// Deploy wildcard with parameters does ${stackPrefix}-param-test-*
new ParameterStack(app, `${stackPrefix}-param-test-1`);
new OtherParameterStack(app, `${stackPrefix}-param-test-2`);
// Deploy stack with multiple parameters
new MultiParameterStack(app, `${stackPrefix}-param-test-3`);
// Deploy stack with outputs does ${stackPrefix}-outputs-test-*
new OutputsStack(app, `${stackPrefix}-outputs-test-1`);
new AnotherOutputsStack(app, `${stackPrefix}-outputs-test-2`);
// Not included in wildcard
new IamStack(app, `${stackPrefix}-iam-test`, { env: defaultEnv });
const providing = new ProvidingStack(app, `${stackPrefix}-order-providing`);
new ConsumingStack(app, `${stackPrefix}-order-consuming`, { providingStack: providing });

new MissingSSMParameterStack(app, `${stackPrefix}-missing-ssm-parameter`, { env: defaultEnv });

new LambdaStack(app, `${stackPrefix}-lambda`);
new DockerStack(app, `${stackPrefix}-docker`);
new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`);
new FailedStack(app, `${stackPrefix}-failed`)

if (process.env.ENABLE_VPC_TESTING) { // Gating so we don't do context fetching unless that's what we are here for
const env = { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION };
if (process.env.ENABLE_VPC_TESTING === 'DEFINE')
new DefineVpcStack(app, `${stackPrefix}-define-vpc`, { env });
if (process.env.ENABLE_VPC_TESTING === 'IMPORT')
new ImportVpcStack(app, `${stackPrefix}-import-vpc`, { env });
}
// Sometimes we don't want to synthesize all stacks because it will impact the results
const stackSet = process.env.INTEG_STACK_SET || 'default';

switch (stackSet) {
case 'default':
// Deploy all does a wildcard ${stackPrefix}-test-*
new MyStack(app, `${stackPrefix}-test-1`, { env: defaultEnv });
new YourStack(app, `${stackPrefix}-test-2`);
// Deploy wildcard with parameters does ${stackPrefix}-param-test-*
new ParameterStack(app, `${stackPrefix}-param-test-1`);
new OtherParameterStack(app, `${stackPrefix}-param-test-2`);
// Deploy stack with multiple parameters
new MultiParameterStack(app, `${stackPrefix}-param-test-3`);
// Deploy stack with outputs does ${stackPrefix}-outputs-test-*
new OutputsStack(app, `${stackPrefix}-outputs-test-1`);
new AnotherOutputsStack(app, `${stackPrefix}-outputs-test-2`);
// Not included in wildcard
new IamStack(app, `${stackPrefix}-iam-test`, { env: defaultEnv });
const providing = new ProvidingStack(app, `${stackPrefix}-order-providing`);
new ConsumingStack(app, `${stackPrefix}-order-consuming`, { providingStack: providing });

new MissingSSMParameterStack(app, `${stackPrefix}-missing-ssm-parameter`, { env: defaultEnv });

new LambdaStack(app, `${stackPrefix}-lambda`);
new DockerStack(app, `${stackPrefix}-docker`);
new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`);
new FailedStack(app, `${stackPrefix}-failed`)

if (process.env.ENABLE_VPC_TESTING) { // Gating so we don't do context fetching unless that's what we are here for
const env = { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION };
if (process.env.ENABLE_VPC_TESTING === 'DEFINE')
new DefineVpcStack(app, `${stackPrefix}-define-vpc`, { env });
if (process.env.ENABLE_VPC_TESTING === 'IMPORT')
new ImportVpcStack(app, `${stackPrefix}-import-vpc`, { env });
}

new ConditionalResourceStack(app, `${stackPrefix}-conditional-resource`)
new ConditionalResourceStack(app, `${stackPrefix}-conditional-resource`)

new StackWithNestedStack(app, `${stackPrefix}-with-nested-stack`);
new StackWithNestedStackUsingParameters(app, `${stackPrefix}-with-nested-stack-using-parameters`);
new StackWithNestedStack(app, `${stackPrefix}-with-nested-stack`);
new StackWithNestedStackUsingParameters(app, `${stackPrefix}-with-nested-stack-using-parameters`);

new YourStack(app, `${stackPrefix}-termination-protection`, {
terminationProtection: process.env.TERMINATION_PROTECTION !== 'FALSE' ? true : false,
});
new YourStack(app, `${stackPrefix}-termination-protection`, {
terminationProtection: process.env.TERMINATION_PROTECTION !== 'FALSE' ? true : false,
});

new SomeStage(app, `${stackPrefix}-stage`);
break;

case 'stage-using-context':
// Cannot be combined with other test stacks, because we use this to test
// that stage context is propagated up and causes synth to fail when combined
// with '--no-lookups'.

new SomeStage(app, `${stackPrefix}-stage`);
// Needs a dummy stack at the top level because the CLI will fail otherwise
new YourStack(app, `${stackPrefix}-toplevel`, { env: defaultEnv });
new StageUsingContext(app, `${stackPrefix}-stage-using-context`, {
env: defaultEnv,
});
break;

default:
throw new Error(`Unrecognized INTEG_STACK_SET: '${stackSet}'`);
}

app.synth();
13 changes: 12 additions & 1 deletion packages/aws-cdk/test/integ/cli/cdk-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@ export function withCdkApp<A extends TestContext & AwsContext>(block: (context:
success = false;
throw e;
} finally {
await fixture.dispose(success);
if (process.env.INTEG_NO_CLEAN) {
process.stderr.write(`Left test directory in '${integTestDir}' ($INTEG_NO_CLEAN)\n`);
} else {
await fixture.dispose(success);
}
}
};
}
Expand Down Expand Up @@ -177,6 +181,13 @@ export class TestFixture {
...this.fullStackName(stackNames)], options);
}

public async cdkSynth(options: CdkCliOptions = {}) {
return this.cdk([
'synth',
...(options.options ?? []),
], options);
}

public async cdkDestroy(stackNames: string | string[], options: CdkCliOptions = {}) {
stackNames = typeof stackNames === 'string' ? [stackNames] : stackNames;

Expand Down
11 changes: 11 additions & 0 deletions packages/aws-cdk/test/integ/cli/cli.integtest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ integTest('context setting', withDefaultFixture(async (fixture) => {
}
}));

integTest('context in stage propagates to top', withDefaultFixture(async (fixture) => {
await expect(fixture.cdkSynth({
// This will make it error to prove that the context bubbles up, and also that we can fail on command
options: ['--no-lookups'],
modEnv: {
INTEG_STACK_SET: 'stage-using-context',
},
allowErrExit: true,
})).resolves.toContain('Context lookups have been disabled');
}));

integTest('deploy', withDefaultFixture(async (fixture) => {
const stackArn = await fixture.cdkDeploy('test-2', { captureStderr: false });

Expand Down

0 comments on commit 0445a6e

Please sign in to comment.