feat(core): can use Constructs to model collections of Stacks #1940

rix0rrr · 2019-03-04T13:58:25Z

Stacks no longer accept a cdk.App as scope, but a cdk.Construct like
other constructs. This makes it possible define a Construct to
represent your entire application, which can consist of a collection of
Stacks.

Stack names will automatically be derived from their location in the
construct tree, but it's also possible to specify the deployed name of
the stack by specifying the stackName property.

ALSO IN THIS CHANGE

Calling app.run() is no longer necessary, but can still safely
be done.
Update output message of awslint so that the linter rule suppression key
can easily be copy pasted.

Fixes #1479. This PR replaces #1582.

Pull Request Checklist

Testing
- Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
- CLI change?: coordinate update of integration tests with team
- cdk-init template change?: coordinated update of integration tests with team
Docs
- jsdocs: All public APIs documented
- README: README and/or documentation topic updated
Title and Description
- Change type: title prefixed with fix, feat will appear in changelog
- Title: use lower-case and doesn't end with a period
- Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
- Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
Sensitive Modules (requires 2 PR approvers)
- IAM Policy Document (in @aws-cdk/aws-iam)
- EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
- Grant APIs (only if not based on official documentation with a reference)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

…teable and severity is at the front.

Stacks no longer accept a `cdk.App` as scope, but a `cdk.Construct` like other constructs. This makes it possible define a Construct to represent your entire application, which can consist of a collection of Stacks. Stack names will automatically be derived from their location in the construct tree, but it's also possible to specify the deployed name of the stack by specifying the `stackName` property. Calling `app.run()` is no longer necessary, but can still safely be done. Fixes #1479.

rix0rrr · 2019-03-04T13:59:08Z

cc @Doug-AWS, the user guide is going to need an update after this PR has been merged.

eladb

A few minor comments

packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn-cross-region.expected.json

packages/@aws-cdk/cdk/lib/app.ts

packages/@aws-cdk/cdk/lib/cloudformation/stack.ts

eladb · 2019-03-04T14:05:56Z

packages/@aws-cdk/cdk/lib/core/construct.ts

+  /**
+   * Return the path of components up to but excluding the root
+   */
+  public rootPath(): IConstruct[] {


Maybe we can just delete this. Is this really a useful thing? I also don't like that it's called path because it doesn't return a path (paths are strings), it returns an array of ancestors without the first one, which brings me to ask, what's rong with construct.node.ancestors().slice(1)

Can you also make ancestors a property please?

"root path" is the term for the nodes in a tree that lie between a given node and the root of the trie. I wouldn't say a path is a string, maybe that a path can be represented as a string.

rootPath could (and should) have more logic for the purposes of IDs, but honestly our tree structure and which things exist and have ids and whether those ids should be taken into account is completely messed up and inconsistent between real applications and unit tests, insofar as that anything that deals with names and paths is a tangle of messy ifs.

So sure, I'll inline the definition of rootPath() everywhere.

ancestors() takes an optional argument, so it cannot be made a property.

eladb · 2019-03-04T14:06:35Z

packages/@aws-cdk/cdk/test/test.app.ts

@@ -4,7 +4,7 @@ import { Construct, Resource, Stack, StackProps } from '../lib';
 import { App } from '../lib/app';

 function withApp(context: { [key: string]: any } | undefined, block: (app: App) => void): cxapi.SynthesizeResponse {
-  const app = new App(context);
+  const app = new App({ context, autoRun: false });


Make autoRun: false the default when running outside the context of the toolkit (e.g. CDK_OUT is not set)

autoRun false is not needed now i guess

Doug-AWS · 2019-03-04T14:40:45Z

Are we also going to change the templates to use this new approach?

rix0rrr · 2019-03-19T14:31:53Z

Are we also going to change the templates to use this new approach?

This is only for applications that consist of multiple stacks, our templates are single-stack applications. So, no.

function

…-can-represent-apps

eladb

yey! looking forward for this to be merged

eladb · 2019-03-19T15:06:07Z

packages/@aws-cdk/cdk/lib/app.ts

+   *
+   * If you set this, you don't have to call `run()` anymore.
+   *
+   * @default true if running via CDK toolkit, false otherwise


Be more specific about the mechanism: true if running via the toolkit (CDK_OUTDIR is set)

eladb · 2019-03-19T15:08:52Z

packages/@aws-cdk/cdk/test/test.app.ts

@@ -4,7 +4,7 @@ import { Construct, Resource, Stack, StackProps } from '../lib';
 import { App } from '../lib/app';

 function withApp(context: { [key: string]: any } | undefined, block: (app: App) => void): cxapi.SynthesizeResponse {
-  const app = new App(context);
+  const app = new App({ context, autoRun: false });


autoRun false is not needed now i guess

eladb · 2019-03-19T15:09:32Z

packages/aws-cdk/lib/init-templates/app/typescript/bin/%name%.template.ts

@@ -5,4 +5,3 @@ import { %name.PascalCased%Stack } from '../lib/%name%-stack';

 const app = new cdk.App();
 new %name.PascalCased%Stack(app, '%name.PascalCased%Stack');
-app.run();


What about other langs?

eladb

I expect some integ tests to fail (no idea which one, but ...)

otterley · 2019-03-21T03:08:00Z

Do you have some examples of how one might use this new feature?

rix0rrr added 2 commits March 4, 2019 11:01

Change error message of awslint so suppression key is easily copy-pas…

832e4a8

…teable and severity is at the front.

rix0rrr requested review from RomainMuller, skinny85 and a team as code owners March 4, 2019 13:58

rix0rrr added the documentation label Mar 4, 2019

eladb reviewed Mar 4, 2019

View reviewed changes

rix0rrr self-assigned this Mar 15, 2019

rix0rrr added 4 commits March 19, 2019 15:32

Selective autoRun, calculate unique stack names, remove rootPath()

f66f46e

function

Merge remote-tracking branch 'origin/master' into huijbers/constructs…

99a587a

…-can-represent-apps

Update tests

a1a57bb

Handle case if there are nested stacks

22c502e

eladb approved these changes Mar 19, 2019

View reviewed changes

rix0rrr added 2 commits March 19, 2019 16:12

Review comments

3173496

Add package-lock.json

c272cd0

eladb approved these changes Mar 19, 2019

View reviewed changes

rix0rrr and others added 4 commits March 19, 2019 17:35

Commit missing package-lock

c97e3fe

Update all references to ImportValue to include a stack name

4c82f16

Update more tests

e00a11b

Fix inheritance of other examples

da33e4c

rix0rrr merged commit 32c2377 into master Mar 20, 2019

rix0rrr deleted the huijbers/constructs-can-represent-apps branch March 20, 2019 09:42

BDQ mentioned this pull request Mar 22, 2019

Support for multiple instances of a cdk.App #1479

Closed

NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019

This was referenced Dec 12, 2019

[Snyk] Upgrade @aws-cdk/cdk from 0.22.0 to 0.35.0 MechanicalRock/scdk#3

Closed

[Snyk] Upgrade @aws-cdk/cdk from 0.24.1 to 0.35.0 MechanicalRock/cdk-constructs#8

Open

snyk-bot mentioned this pull request Dec 13, 2019

[Snyk] Upgrade @aws-cdk/cdk from 0.24.1 to 0.35.0 MechanicalRock/account-reaper#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): can use Constructs to model collections of Stacks #1940

feat(core): can use Constructs to model collections of Stacks #1940

rix0rrr commented Mar 4, 2019

rix0rrr commented Mar 4, 2019

eladb left a comment

eladb Mar 4, 2019

eladb Mar 4, 2019

rix0rrr Mar 19, 2019

eladb Mar 4, 2019

eladb Mar 19, 2019

Doug-AWS commented Mar 4, 2019

rix0rrr commented Mar 19, 2019 •

edited

Loading

eladb left a comment

eladb Mar 19, 2019

eladb Mar 19, 2019

eladb Mar 19, 2019

eladb left a comment

otterley commented Mar 21, 2019

feat(core): can use Constructs to model collections of Stacks #1940

feat(core): can use Constructs to model collections of Stacks #1940

Conversation

rix0rrr commented Mar 4, 2019

Pull Request Checklist

rix0rrr commented Mar 4, 2019

eladb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Doug-AWS commented Mar 4, 2019

rix0rrr commented Mar 19, 2019 • edited Loading

eladb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eladb left a comment

Choose a reason for hiding this comment

otterley commented Mar 21, 2019

rix0rrr commented Mar 19, 2019 •

edited

Loading