Skip to content

Commit

Permalink
chore(core): inject TreeMetadata resource at the end (during synthe…
Browse files Browse the repository at this point in the history
…sis) (#22780)

The `TreeMetadata` resource was being created as part of the `App` constructor, which meant that you could not add any context to the App node (you must add context _before_ any constructs are added). This moves the creation of `TreeMetadata` to be injected during `synthesis` similar to how we currently inject the metadata resource.

A side effect of this change is that we will now include tree metadata for resources that are added in `Stack.synthesize()` methods. The [synthesizeTree](https://github.com/aws/aws-cdk/blob/95e9e0f54a1dfb628d5146bd9b69c292547cf090/packages/@aws-cdk/core/lib/private/synthesis.ts#L193-L210) function visits nodes in the order that they were added to the tree. Previously the `TreeMetadata` was the first node so the TreeMetadata was generated first. Then if any Stacks would call `Stack.synthesize()` and add additional constructs (e.g. `BootstrapVersion`) these would not be added to the tree.

re #22749

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
corymhall committed Nov 4, 2022
1 parent e2a9c77 commit b883616
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 15 deletions.
12 changes: 8 additions & 4 deletions packages/@aws-cdk/core/lib/app.ts
Expand Up @@ -2,7 +2,6 @@ import * as cxapi from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import * as fs from 'fs-extra';
import { addCustomSynthesis, ICustomSynthesis } from './private/synthesis';
import { TreeMetadata } from './private/tree-metadata';
import { Stage } from './stage';

const APP_SYMBOL = Symbol.for('@aws-cdk/core.App');
Expand Down Expand Up @@ -133,6 +132,13 @@ export class App extends Stage {
return APP_SYMBOL in obj;
}

/**
* Include construct tree metadata as part of the Cloud Assembly.
*
* @internal
*/
public readonly _treeMetadata: boolean;

/**
* Initializes a CDK application.
* @param props initialization properties
Expand Down Expand Up @@ -163,9 +169,7 @@ export class App extends Stage {
process.once('beforeExit', () => this.synth());
}

if (props.treeMetadata === undefined || props.treeMetadata) {
new TreeMetadata(this);
}
this._treeMetadata = props.treeMetadata ?? true;
}

private loadContext(defaults: { [key: string]: string } = { }, final: { [key: string]: string } = {}) {
Expand Down
19 changes: 19 additions & 0 deletions packages/@aws-cdk/core/lib/private/synthesis.ts
@@ -1,6 +1,7 @@
import * as cxapi from '@aws-cdk/cx-api';
import { IConstruct } from 'constructs';
import { Annotations } from '../annotations';
import { App } from '../app';
import { Aspects, IAspect } from '../aspect';
import { Stack } from '../stack';
import { ISynthesisSession } from '../stack-synthesizers/types';
Expand All @@ -21,6 +22,8 @@ export interface SynthesisOptions extends StageSynthesisOptions {
}

export function synthesize(root: IConstruct, options: SynthesisOptions = { }): cxapi.CloudAssembly {
// add the TreeMetadata resource to the App first
injectTreeMetadata(root);
// we start by calling "synth" on all nested assemblies (which will take care of all their children)
synthNestedAssemblies(root, options);

Expand Down Expand Up @@ -166,6 +169,22 @@ function injectMetadataResources(root: IConstruct) {
});
}

/**
* Find the root App and add the TreeMetadata resource (if enabled).
*
* 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.
*/
function injectTreeMetadata(root: IConstruct) {
visit(root, 'post', construct => {
if (!App.isApp(construct) || !construct._treeMetadata) return;
const CDKTreeMetadata = 'Tree';
if (construct.node.tryFindChild(CDKTreeMetadata)) return;
new TreeMetadata(construct);
});
}

/**
* Synthesize children in post-order into the given builder
*
Expand Down
52 changes: 41 additions & 11 deletions packages/@aws-cdk/core/test/private/tree-metadata.test.ts
Expand Up @@ -44,6 +44,22 @@ describe('tree metadata', () => {
id: 'mystack',
path: 'mystack',
children: {
BootstrapVersion: {
constructInfo: {
fqn: '@aws-cdk/core.CfnParameter',
version: '0.0.0',
},
id: 'BootstrapVersion',
path: 'mystack/BootstrapVersion',
},
CheckBootstrapVersion: {
constructInfo: {
fqn: '@aws-cdk/core.CfnRule',
version: '0.0.0',
},
id: 'CheckBootstrapVersion',
path: 'mystack/CheckBootstrapVersion',
},
myconstruct: expect.objectContaining({
id: 'myconstruct',
path: 'mystack/myconstruct',
Expand All @@ -53,7 +69,6 @@ describe('tree metadata', () => {
},
}),
});

});

test('tree metadata for a Cfn resource', () => {
Expand Down Expand Up @@ -91,7 +106,7 @@ describe('tree metadata', () => {
mystack: expect.objectContaining({
id: 'mystack',
path: 'mystack',
children: {
children: expect.objectContaining({
mycfnresource: expect.objectContaining({
id: 'mycfnresource',
path: 'mystack/mycfnresource',
Expand All @@ -107,7 +122,7 @@ describe('tree metadata', () => {
},
},
}),
},
}),
}),
},
}),
Expand Down Expand Up @@ -156,14 +171,14 @@ describe('tree metadata', () => {
fqn: expect.stringMatching(codeBuild ? /\bStack$/ : /\bStack$|^constructs.Construct$/),
version: expect.any(String),
},
children: {
children: expect.objectContaining({
myconstruct: expect.objectContaining({
constructInfo: {
fqn: expect.stringMatching(codeBuild ? /\bCfnResource$/ : /\bCfnResource$|^constructs.Construct$/),
version: expect.any(String),
},
}),
},
}),
}),
}),
}),
Expand Down Expand Up @@ -205,7 +220,7 @@ describe('tree metadata', () => {
mystack: expect.objectContaining({
id: 'mystack',
path: 'mystack',
children: {
children: expect.objectContaining({
mycfnparam: expect.objectContaining({
id: 'mycfnparam',
path: 'mystack/mycfnparam',
Expand All @@ -221,7 +236,7 @@ describe('tree metadata', () => {
},
},
}),
},
}),
}),
},
}),
Expand Down Expand Up @@ -283,7 +298,7 @@ describe('tree metadata', () => {
myfirststack: expect.objectContaining({
id: 'myfirststack',
path: 'myfirststack',
children: {
children: expect.objectContaining({
myfirstresource: expect.objectContaining({
id: 'myfirstresource',
path: 'myfirststack/myfirstresource',
Expand All @@ -294,12 +309,12 @@ describe('tree metadata', () => {
},
},
}),
},
}),
}),
mysecondstack: expect.objectContaining({
id: 'mysecondstack',
path: 'mysecondstack',
children: {
children: expect.objectContaining({
mysecondresource: expect.objectContaining({
id: 'mysecondresource',
path: 'mysecondstack/mysecondresource',
Expand All @@ -310,13 +325,28 @@ describe('tree metadata', () => {
},
},
}),
},
}),
}),
},
}),
});
});

test('tree metadata can be disabled', () => {
// GIVEN
const app = new App({
treeMetadata: false,
});

// WHEN
const stack = new Stack(app, 'mystack');
new Construct(stack, 'myconstruct');

const assembly = app.synth();
const treeArtifact = assembly.tree();

// THEN
expect(treeArtifact).not.toBeDefined();
});

test('failing nodes', () => {
Expand Down

0 comments on commit b883616

Please sign in to comment.