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

fix(core): write Metadata resource in core framework #10306

Merged
merged 18 commits into from
Sep 28, 2020
Merged
Show file tree
Hide file tree
Changes from 12 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
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"5.0.0"}
{"version":"6.0.0"}
19 changes: 15 additions & 4 deletions packages/@aws-cdk/core/lib/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,20 @@ export interface AppProps {
readonly stackTraces?: boolean;

/**
* Include runtime versioning information in cloud assembly manifest
* @default true runtime info is included unless `aws:cdk:disable-runtime-info` is set in the context.
* Include runtime versioning information in the Stacks of this app
*
* @depreacted use `versionReporting` instead
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
* @default Value of 'aws:cdk:version-reporting' context key
*/
readonly runtimeInfo?: boolean;

/**
* Include runtime versioning information in the Stacks of this app
*
* @default Value of 'aws:cdk:version-reporting' context key
*/
readonly versionReporting?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if the correct terminology should be analytics: boolean--broaden the scope a little bit.


/**
* Additional context values for the application.
*
Expand Down Expand Up @@ -101,8 +110,10 @@ export class App extends Stage {
this.node.setContext(cxapi.DISABLE_METADATA_STACK_TRACE, true);
}

if (props.runtimeInfo === false) {
this.node.setContext(cxapi.DISABLE_VERSION_REPORTING, true);
const versionReporting = props.versionReporting ?? props.runtimeInfo;

if (versionReporting !== undefined) {
this.node.setContext(cxapi.VERSION_REPORTING_ENABLED_CONTEXT, versionReporting);
}

const autoSynth = props.autoSynth !== undefined ? props.autoSynth : cxapi.OUTDIR_ENV in process.env;
Expand Down
28 changes: 24 additions & 4 deletions packages/@aws-cdk/core/lib/cfn-fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,18 @@ export class Fn {
* Returns true if all the specified conditions evaluate to true, or returns
* false if any one of the conditions evaluates to false. ``Fn::And`` acts as
* an AND operator. The minimum number of conditions that you can include is
* 2, and the maximum is 10.
* 1.
* @param conditions conditions to AND
* @returns an FnCondition token
*/
public static conditionAnd(...conditions: ICfnConditionExpression[]): ICfnConditionExpression {
return new FnAnd(...conditions);
if (conditions.length === 0) {
throw new Error('Fn.conditionAnd() needs at least one argument');
}
if (conditions.length === 1) {
return conditions[0];
}
return Fn.conditionAnd(..._inGroupsOf(conditions, 10).map(group => new FnAnd(...group)));
}

/**
Expand Down Expand Up @@ -240,12 +246,18 @@ export class Fn {
* Returns true if any one of the specified conditions evaluate to true, or
* returns false if all of the conditions evaluates to false. ``Fn::Or`` acts
* as an OR operator. The minimum number of conditions that you can include is
* 2, and the maximum is 10.
* 1.
* @param conditions conditions that evaluates to true or false.
* @returns an FnCondition token
*/
public static conditionOr(...conditions: ICfnConditionExpression[]): ICfnConditionExpression {
return new FnOr(...conditions);
if (conditions.length === 0) {
throw new Error('Fn.conditionOr() needs at least one argument');
}
if (conditions.length === 1) {
return conditions[0];
}
return Fn.conditionOr(..._inGroupsOf(conditions, 10).map(group => new FnOr(...group)));
}

/**
Expand Down Expand Up @@ -739,3 +751,11 @@ class FnJoin implements IResolvable {
return minimalCloudFormationJoin(this.delimiter, resolvedValues);
}
}

function _inGroupsOf<T>(array: T[], maxGroup: number): T[][] {
const result = new Array<T[]>();
for (let i = 0; i < array.length; i += maxGroup) {
result.push(array.slice(i, i + maxGroup));
}
return result;
}
92 changes: 92 additions & 0 deletions packages/@aws-cdk/core/lib/private/metadata-resource.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import * as cxapi from '@aws-cdk/cx-api';
import { RegionInfo } from '@aws-cdk/region-info';
import { CfnCondition } from '../cfn-condition';
import { Fn } from '../cfn-fn';
import { Aws } from '../cfn-pseudo';
import { CfnResource } from '../cfn-resource';
import { Construct } from '../construct-compat';
import { Lazy } from '../lazy';
import { Stack } from '../stack';
import { Token } from '../token';
import { collectRuntimeInformation } from './runtime-info';

/**
* Construct that will render the metadata resource
*/
export class MetadataResource extends Construct {
/**
* Clear the modules cache
*
* The next time the MetadataResource is rendered, it will do a lookup of the
* modules from the NodeJS module cache again.
*
* Used only for unit tests.
*/
public static clearModulesCache() {
this._modulesPropertyCache = undefined;
}

/**
* Cached version of the _modulesProperty() accessor
*
* No point in calculating this fairly expensive list more than once.
*/
private static _modulesPropertyCache?: string;

/**
* Calculate the modules property
*/
private static modulesProperty(): string {
if (this._modulesPropertyCache === undefined) {
this._modulesPropertyCache = formatModules(collectRuntimeInformation());
}
return this._modulesPropertyCache;
}

constructor(scope: Stack, id: string) {
super(scope, id);

const metadataServiceExists = Token.isUnresolved(scope.region) || RegionInfo.get(scope.region).cdkMetadataResourceAvailable;
eladb marked this conversation as resolved.
Show resolved Hide resolved
if (metadataServiceExists) {
const resource = new CfnResource(this, 'Default', {
type: 'AWS::CDK::Metadata',
properties: {
Modules: Lazy.stringValue({ produce: () => MetadataResource.modulesProperty() }),
},
});

// In case we don't actually know the region, add a condition to determine it at deploy time
if (Token.isUnresolved(scope.region)) {
const condition = new CfnCondition(this, 'Condition', {
expression: makeCdkMetadataAvailableCondition(),
});

// To not cause undue template changes
condition.overrideLogicalId('CDKMetadataAvailable');

resource.cfnOptions.condition = condition;
}
}
}
}

function makeCdkMetadataAvailableCondition() {
return Fn.conditionOr(...RegionInfo.regions
.filter(ri => ri.cdkMetadataResourceAvailable)
.map(ri => Fn.conditionEquals(Aws.REGION, ri.name)));
}

function formatModules(runtime: cxapi.RuntimeInfo): string {
const modules = new Array<string>();

// inject toolkit version to list of modules
const cliVersion = process.env[cxapi.CLI_VERSION_ENV];
if (cliVersion) {
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
modules.push(`aws-cdk=${cliVersion}`);
}

for (const key of Object.keys(runtime.libraries).sort()) {
modules.push(`${key}=${runtime.libraries[key]}`);
}
return modules.join(',');
}
37 changes: 34 additions & 3 deletions packages/@aws-cdk/core/lib/private/synthesis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Aspects, IAspect } from '../aspect';
import { Construct, IConstruct, SynthesisOptions, ValidationError } from '../construct-compat';
import { Stack } from '../stack';
import { Stage, StageSynthesisOptions } from '../stage';
import { MetadataResource } from './metadata-resource';
import { prepareApp } from './prepare-app';
import { TreeMetadata } from './tree-metadata';

Expand All @@ -14,6 +15,8 @@ export function synthesize(root: IConstruct, options: SynthesisOptions = { }): c

invokeAspects(root);

injectMetadataResources(root);

// This is mostly here for legacy purposes as the framework itself does not use prepare anymore.
prepareTree(root);

Expand All @@ -35,9 +38,7 @@ export function synthesize(root: IConstruct, options: SynthesisOptions = { }): c
// stacks to add themselves to the synthesized cloud assembly.
synthesizeTree(root, builder);

return builder.buildAssembly({
runtimeInfo: options.runtimeInfo,
});
return builder.buildAssembly();
}

/**
Expand Down Expand Up @@ -110,6 +111,36 @@ function prepareTree(root: IConstruct) {
visit(root, 'post', construct => construct.onPrepare());
}

/**
* Find all stacks and add Metadata Resources to all of them
*
* There is no good generic place to do this. Can't do it in the constructor
* (because adding a child construct makes it impossible to set context on the
* node), and the generic prepare phase is deprecated.
*
* Only do this on [parent] stacks (not nested stacks), don't do this when
* disabled by the user.
*
* Also, only when running via the CLI. If we do it unconditionally,
* all unit tests everywhere are going to break massively. I've spent a day
* fixing our own, but downstream users would be affected just as badly.
*
* Stop at Assembly boundaries.
*/
function injectMetadataResources(root: IConstruct) {
visit(root, 'post', construct => {
if (!Stack.isStack(construct) || !construct._versionReportingEnabled) { return; }

// Because of https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/assert/lib/synth-utils.ts#L74
// synthesize() may be called more than once on a stack in unit tests, and the below would break
// if we execute it a second time. Guard against the constructs already existing.
const CDKMetadata = 'CDKMetadata';
if (construct.node.tryFindChild(CDKMetadata)) { return; }

new MetadataResource(construct, CDKMetadata);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me what's the rationale for not just adding this in the Stack's constructor based on the prop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't set context anymore

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And why do we need to set context? My view is that the context is just used to determine the default for analyticsReporting when new stacks are created. From that point forward we shouldn't care about the context key, no?

});
}

/**
* Synthesize children in post-order into the given builder
*
Expand Down
26 changes: 24 additions & 2 deletions packages/@aws-cdk/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ export interface StackProps {
* @default false
*/
readonly terminationProtection?: boolean;

/**
* Include runtime versioning information in this Stack
*
* @default `versionReporting setting of containing `App`, or value of
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
* 'aws:cdk:version-reporting' context key
*/
readonly versionReporting?: boolean;
}

/**
Expand Down Expand Up @@ -279,6 +287,15 @@ export class Stack extends Construct implements ITaggable {
*/
public readonly synthesizer: IStackSynthesizer;

/**
* Whether version reporting is enabled for this stack
*
* Controls whether the CDK Metadata resource is injected
*
* @internal
*/
public readonly _versionReportingEnabled: boolean;

/**
* Logical ID generation strategy
*/
Expand Down Expand Up @@ -368,6 +385,10 @@ export class Stack extends Construct implements ITaggable {

this.templateFile = `${this.artifactId}.template.json`;

// Not for nested stacks
this._versionReportingEnabled = (props.versionReporting ?? this.node.tryGetContext(cxapi.VERSION_REPORTING_ENABLED_CONTEXT))
&& !this.nestedStackParent;

this.synthesizer = props.synthesizer ?? (newStyleSynthesisContext
? new DefaultStackSynthesizer()
: new LegacyStackSynthesizer());
Expand Down Expand Up @@ -720,10 +741,11 @@ export class Stack extends Construct implements ITaggable {
// this right now, so some parts still happen here.
const builder = session.assembly;

const template = this._toCloudFormation();

// write the CloudFormation template as a JSON file
const outPath = path.join(builder.outdir, this.templateFile);
const text = JSON.stringify(this._toCloudFormation(), undefined, 2);
fs.writeFileSync(outPath, text);
fs.writeFileSync(outPath, JSON.stringify(template, undefined, 2));

for (const ctx of this._missingContext) {
builder.addMissing(ctx);
Expand Down
3 changes: 0 additions & 3 deletions packages/@aws-cdk/core/lib/stage.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as cxapi from '@aws-cdk/cx-api';
import { Construct, IConstruct } from './construct-compat';
import { Environment } from './environment';
import { collectRuntimeInformation } from './private/runtime-info';
import { synthesize } from './private/synthesis';

/**
Expand Down Expand Up @@ -171,10 +170,8 @@ export class Stage extends Construct {
*/
public synth(options: StageSynthesisOptions = { }): cxapi.CloudAssembly {
if (!this.assembly || options.force) {
const runtimeInfo = this.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation();
this.assembly = synthesize(this, {
skipValidation: options.skipValidation,
runtimeInfo,
});
}

Expand Down
4 changes: 3 additions & 1 deletion packages/@aws-cdk/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@
"dependencies": {
"@aws-cdk/cloud-assembly-schema": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/region-info": "0.0.0",
"constructs": "^3.0.4",
"fs-extra": "^9.0.1",
"minimatch": "^3.0.4"
Expand All @@ -185,7 +186,8 @@
"peerDependencies": {
"@aws-cdk/cloud-assembly-schema": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"constructs": "^3.0.4"
"constructs": "^3.0.4",
"@aws-cdk/region-info": "0.0.0"
},
"engines": {
"node": ">= 10.13.0 <13 || >=13.7.0"
Expand Down
Loading