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

feat(cli): add --no-lookups flag to disable context lookups #11489

Merged
merged 6 commits into from
Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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