Skip to content

Commit

Permalink
refactor(core): move dependency logic to app and stack tags to synth
Browse files Browse the repository at this point in the history
As a follow up to aws#7187, extract the rest of the code from `Stack.prepare`: the logic that converted construct-level dependencies to resource-level dependencies was moved to `App.prepare` and the logic that added top-level stack tags to the cloud assembly metadata was moved to `Stack.synthesize`.
  • Loading branch information
Elad Ben-Israel authored Apr 28, 2020
1 parent 1ee629e commit d96f361
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 29 deletions.
22 changes: 21 additions & 1 deletion packages/@aws-cdk/core/lib/private/prepare-app.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ConstructOrder } from 'constructs';
import { Construct } from '../construct-compat';
import { CfnResource } from '../cfn-resource';
import { Construct, IConstruct } from '../construct-compat';
import { Stack } from '../stack';
import { resolveReferences } from './refs';

Expand All @@ -18,6 +19,18 @@ export function prepareApp(root: Construct) {
throw new Error('prepareApp must be called on the root node');
}

// apply dependencies between resources in depending subtrees
for (const dependency of root.node.dependencies) {
const targetCfnResources = findCfnResources(dependency.target);
const sourceCfnResources = findCfnResources(dependency.source);

for (const target of targetCfnResources) {
for (const source of sourceCfnResources) {
source.addDependsOn(target);
}
}
}

// depth-first (children first) queue of nested stacks. We will pop a stack
// from the head of this queue to prepare it's template asset.
const queue = findAllNestedStacks(root);
Expand Down Expand Up @@ -60,6 +73,13 @@ function findAllNestedStacks(root: Construct) {
return result;
}

/**
* Find all resources in a set of constructs
*/
function findCfnResources(root: IConstruct): CfnResource[] {
return root.node.findAll().filter(CfnResource.isCfnResource);
}

interface INestedStackPrivateApi {
_prepareTemplateAsset(): boolean;
}
37 changes: 10 additions & 27 deletions packages/@aws-cdk/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -734,19 +734,6 @@ export class Stack extends Construct implements ITaggable {
* Find all dependencies as well and add the appropriate DependsOn fields.
*/
protected prepare() {
// Resource dependencies
for (const dependency of this.node.dependencies) {
for (const target of findCfnResources([ dependency.target ])) {
for (const source of findCfnResources([ dependency.source ])) {
source.addDependsOn(target);
}
}
}

if (this.tags.hasTags()) {
this.node.addMetadata(cxschema.ArtifactMetadataEntryType.STACK_TAGS, this.tags.renderTags());
}

// if this stack is a roort (e.g. in unit tests), call `prepareApp` so that
// we resolve cross-references and nested stack assets.
if (!this.node.scope) {
Expand All @@ -771,9 +758,6 @@ export class Stack extends Construct implements ITaggable {
return;
}

const deps = this.dependencies.map(s => s.artifactId);
const meta = this.collectMetadata();

// backwards compatibility since originally artifact ID was always equal to
// stack name the stackName attribute is optional and if it is not specified
// the CLI will use the artifact ID as the stack name. we *could have*
Expand All @@ -785,11 +769,21 @@ export class Stack extends Construct implements ITaggable {
? { }
: { stackName: this.stackName };

// nested stack tags are applied at the AWS::CloudFormation::Stack resource
// level and are not needed in the cloud assembly.
// TODO: move these to the cloud assembly artifact properties instead of metadata
if (this.tags.hasTags()) {
this.node.addMetadata(cxschema.ArtifactMetadataEntryType.STACK_TAGS, this.tags.renderTags());
}

const properties: cxapi.AwsCloudFormationStackProperties = {
templateFile: this.templateFile,
...stackNameProperty,
};

const deps = this.dependencies.map(s => s.artifactId);
const meta = this.collectMetadata();

// add an artifact that represents this stack
builder.addArtifact(this.artifactId, {
type: cxschema.ArtifactType.AWS_CLOUDFORMATION_STACK,
Expand Down Expand Up @@ -1054,17 +1048,6 @@ import { IResolvable } from './resolvable';
import { ITaggable, TagManager } from './tag-manager';
import { Token } from './token';

/**
* Find all resources in a set of constructs
*/
function findCfnResources(roots: Iterable<IConstruct>): CfnResource[] {
const ret = new Array<CfnResource>();
for (const root of roots) {
ret.push(...root.node.findAll().filter(CfnResource.isCfnResource));
}
return ret;
}

interface StackDependency {
stack: Stack;
reasons: string[];
Expand Down
27 changes: 26 additions & 1 deletion packages/@aws-cdk/core/test/test.stack.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as cxapi from '@aws-cdk/cx-api';
import { Test } from 'nodeunit';
import { App, CfnCondition, CfnInclude, CfnOutput, CfnParameter, CfnResource, Construct, ConstructNode, Lazy, ScopedAws, Stack, validateString } from '../lib';
import {
App, CfnCondition, CfnInclude, CfnOutput, CfnParameter,
CfnResource, Construct, ConstructNode, Lazy, ScopedAws, Stack, Tag, validateString } from '../lib';
import { Intrinsic } from '../lib/private/intrinsic';
import { PostResolveToken } from '../lib/util';
import { toCloudFormation } from './util';
Expand Down Expand Up @@ -828,6 +830,29 @@ export = {
]);
test.done();
},

'stack tags are reflected in the stack cloud assembly artifact'(test: Test) {
// GIVEN
const app = new App({ stackTraces: false });
const stack1 = new Stack(app, 'stack1');
const stack2 = new Stack(stack1, 'stack2');

// WHEN
Tag.add(app, 'foo', 'bar');

// THEN
const asm = app.synth();
const expected = [
{
type: 'aws:cdk:stack-tags',
data: [ { key: 'foo', value: 'bar' } ],
},
];

test.deepEqual(asm.getStackArtifact(stack1.artifactId).manifest.metadata, { '/stack1': expected });
test.deepEqual(asm.getStackArtifact(stack2.artifactId).manifest.metadata, { '/stack1/stack2': expected });
test.done();
},
};

class StackWithPostProcessor extends Stack {
Expand Down

0 comments on commit d96f361

Please sign in to comment.