feat(core): democratize synthesis and introduce artifacts #1889

eladb · 2019-02-26T10:16:17Z

This change enables each construct in the tree to participate in synthesis
by overriding the synthesize and add "artifacts" and write files to a synthesis session.

At the moment, the only construct that implements synthesize is Stack which emits an aws:cloudformation:stack artifact which includes a reference to a template file stored into the synthesis session as well.

App.run() traverses the tree and invokes synthesize on all constructs.

When synthesis is finalized, a manifest is produced and stored in the root manifest.json file. This is accordance with the draft specification for cloud assemblies as authored by @RomainMuller and included in this change.

For backwards compatibility, and until we implement the required changes in the toolkit, the output also contains cdk.out which is 100% compatible with the current output.

Related #1716
Related #1893

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.

This change enables each construct in the tree to participate in synthesis by overriding the "synthesize" method and emitting files to the session directory. It is a small step towards generalizing how synthesis artifacts are being produced, built and deployed by the CDK. This change moves the code that synthesizes a CloudFormation template from a CDK stack to the `Stack` class and emits a `.template.json` file for each stack. The CLI hasn't been changed to work with these new files yet, so the output of this file is ALSO included in the main manifest (defined `cxapi.OUTFILE_NAME`), which is the only file currently being read by the CLI.

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

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

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

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

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

…unit tests

…synthesis

Start moving towards the cloud-assembly specification where an output of a CDK program is a bunch of artifacts and those are processed by the toolkit. Related #956 Related #233 Related #1119

…synthesis

Instead of an empty protected method, use an interface to discover if a construct is synthesizable.

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

packages/@aws-cdk/cdk/lib/runtime-info.ts

sam-goodwin · 2019-02-27T22:18:20Z

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

+    }
+
+    // add an artifact that represents this stack
+    session.addArtifact(this.node.id, artifact);


Shouldn't you be using node.uniqueId instead of node.id? Same question for dependencies. Are we leaving it up to the caller to ensure the uniqueness of the id?

I am using node.id for backwards compatibility purposes. We are going to add support for composite apps (#1582) and will address there.

I decided that for now, I'll introduce the low-level API and the caller will be responsible. As you observed, not many constructs are going to participate in synthesis so I believe we can leave it to them to take responsibility and apply a best practice to use node.uniqueId for now. Happy to iterate after this PR lands to make more implicit/automatic.

sam-goodwin · 2019-02-27T23:43:23Z

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

+      properties: { template }
+    };
+
+    const deps = this.dependencies().map(s => s.node.id);


What's the reasoning behind this? Does a construct dependency always imply a droplet dependency?

These are stack dependencies, not construct dependencies.

sam-goodwin · 2019-02-28T00:00:22Z

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

+    // write the CloudFormation template as a JSON file
+    session.store.writeJson(template, this.toCloudFormation());
+
+    const artifact: cxapi.Artifact = {


So it looks like staging will use the same format as cloud assembly? How would I communicate something like 'build this folder'? If it's an Artifact, then how do we plan on distinguishing between synth-only artifacts and cloud-assembly artifacts?

The output of App.run() is an intermediate step towards a cloud assembly. I've added a build: any to Artifact as a placeholder, with the idea that the toolkit will use this information to build the intermediate artifact and produce the final assembly.

I am not in love with this design, but maybe it's the pragmatic thing to do, especially given the simple case (CloudFormation template) is a no-op between staging=>assembly.

Happy to hear your thoughts. Maybe we can start with this and iterate and see where it takes us.

Thinking about this some more. I think you are right... Something is still not clicking. Crap.

NEED MORE THINKING.

Okay, here's where I am at right now.

Let's say that app.run() produces two files: manifest.json, which is the cloud assembly manifest proper and build.json which build instructions for the toolkit.

Let's assume that manifest.json is not touched by the toolkit and that the build system (which is not modeled yet), will produce outputs that are pre-referenced by the emitted manifest.

As you can see in the example below, the artifact MyHandlerCode is of type aws:s3:object (because this is the resource that will eventually be created) and the filePath property points to file://3cdf5efc05e1254da7d751257c344e74.zip, which doesn't exist when app.run() ends. The file location is always relative to the assembly root.

{ "artifacts": { "MyStack": { "type": "aws:cloudformation:stack", "environment": "aws://account/region", "properties": { "name": "my-stack-name", "parameters": { "MyHandlerLocation": "s3://${MyHandlerCode.bucket}/${MyHandlerCode.key}" }, "template": "file://${assembly}/my-stack.template.json" } }, "MyHandlerCode": { "type": "aws:s3:object", "properties": { "filePath": "file://3cdf5efc05e1254da7d751257c344e74.zip" } } } }

Completely decoupled from that, we also emit build.json (which is not modeled yet) but basically tells the toolkit how to produce 3cdf5efc05e1254da7d751257c344e74.zip. This is where we can get wild with your declarative build system.

So, for the above example, let's say the handler is a Java project and we use lambda builders, then build.json will model the following process:

(future) Generate some magical code into the project directory (only referenced in build.json, not needed by manifest.json).

Run lambda-builders-java for the project directory inside the docker image

Zip up the resulting output (say it's not zipped by lambda-builders, I don't know)

Output to 3cdf5efc05e1254da7d751257c344e74.zip

It is important that artifacts and build commands are completely decoupled, and only connect when a build output is used as an artifact's input. For example, the build output name is based on the content hash of the project directory, so if multiple assets use the same project directory, it's the same build output.

sam-goodwin · 2019-02-28T00:02:02Z

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

+  }
+}
+
+export interface ISessionStore {


How would I make a project directory available for a build? Would I symlink or manually copy each file into the store?

I am thinking that project directories will be referenced directly in the Artifact's build field without copying them to the staging area.

I think that code generation and build will need to happen in-place in order to allow IDE integration to be sane. Otherwise, I can't see how users will be able to sanely consume any generated code.

You mentioned something yesterday about generating complete libraries and not code, and I wanted to loop back on that and tell you that this was not my intention when we talked about code generation last week, and I actually don't think it's very pragmatic to generate full libraries. All I was saying is that you should assume that users never edit or check in any generated code, so if you want users to consume something that you generated, they would do something like:

import { GeneratedClass } from '../cdk.generated'; // use GeneratedClass

Now, the CDK can generate it's code into cdk.generated.

…synthesis

…s during deployment

eladb · 2019-02-28T16:07:39Z

@sam-goodwin I've updated the PR with support for session.addBuildStep which is a first approximation at this idea of a separate set of build steps emitted alongside the manifest.json. Let me know what you think.

I think I will slightly change the structure of the output directory to look like this:

.
├── assembly
│   ├── manifest.json
│   ├── stack1.template.json
│   ├── ... artifacts produced directly from app.run()
└── build.json

This way, the assembly directory is basically the staging area for the cloud assembly and the toolkit only adds stuff to it (results from executing the build steps defined under build.json).

…synthesis

mergify · 2019-09-23T15:07:30Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

eladb requested review from sam-goodwin and a team February 26, 2019 10:16

rix0rrr reviewed Feb 26, 2019

View reviewed changes

Elad Ben-Israel added 2 commits February 26, 2019 14:20

move "prepare" before "validate"

f00f5af

Use stack's uniqueId as artifact name instead of just id

c23f0c6

rix0rrr reviewed Feb 26, 2019

View reviewed changes

packages/@aws-cdk/cdk/lib/core/construct.ts Outdated Show resolved Hide resolved

Elad Ben-Israel added 7 commits February 26, 2019 17:06

Move manifest creation to run and don't use synthesizeStacks

25a96f9

fix visibility modifiers of Stack methods

1920c9b

add "mkdir", "readdir", "exists" and "list" to synth session and add …

89827fd

…unit tests

read synthesized stacks in a uniform way

7a35a98

fix applet tests

82622e7

identify stacks by id and not uniqueid (for now)

6aea527

change findAll API to PreOrder/PostOrder instead of Depth/Breadth

ab0daa3

eladb requested a review from rix0rrr February 26, 2019 16:23

Elad Ben-Israel added 7 commits February 26, 2019 18:59

Merge remote-tracking branch 'origin/master' into benisrae/construct-…

64bb0d6

…synthesis

remove unused bundled deps

3912e28

fix assert test

297b66b

start establishing the concept of "artifacts"

dca97c6

Start moving towards the cloud-assembly specification where an output of a CDK program is a bunch of artifacts and those are processed by the toolkit. Related #956 Related #233 Related #1119

only write "stacks" in legacy cdk.out

b4b3847

Merge remote-tracking branch 'origin/master' into benisrae/construct-…

87165f5

…synthesis

ISynthesizable

362faa5

Instead of an empty protected method, use an interface to discover if a construct is synthesizable.

eladb changed the title ~~feat(core): democratize synthesis~~ feat(core): democratize synthesis and introduce artifacts Feb 27, 2019

eladb requested a review from RomainMuller February 27, 2019 15:53

rix0rrr approved these changes Feb 27, 2019

View reviewed changes

Elad Ben-Israel added 2 commits February 27, 2019 18:05

Rename "finalize" to "close", because Java

d8a7e95

allow disabling legacy manifest in new versions

9536a17

sam-goodwin reviewed Feb 28, 2019

View reviewed changes

Elad Ben-Israel added 2 commits February 28, 2019 12:43

comment changes

38f402c

Merge remote-tracking branch 'origin/master' into benisrae/construct-…

60528dd

…synthesis

Elad Ben-Israel added 5 commits February 28, 2019 12:49

update package-lock.json

b526fac

fix how "disable version reporting" is evaluated

2b3c595

interm

0e12e2a

stack.setParameterValue can be used to assign values to CFN parameter…

5f27239

…s during deployment

build.json and addBuildStep

b3bdd3b

Elad Ben-Israel added 2 commits February 28, 2019 18:18

revert asset.ts change

f923ad5

Merge remote-tracking branch 'origin/master' into benisrae/construct-…

d2d29ed

…synthesis

sam-goodwin merged commit 4ab1cd3 into master Mar 1, 2019

rix0rrr deleted the benisrae/construct-synthesis branch April 23, 2019 07:13

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): democratize synthesis and introduce artifacts #1889

feat(core): democratize synthesis and introduce artifacts #1889

eladb commented Feb 26, 2019 •

edited

Loading

sam-goodwin Feb 27, 2019

eladb Feb 28, 2019

sam-goodwin Feb 27, 2019

eladb Feb 28, 2019

sam-goodwin Feb 28, 2019

eladb Feb 28, 2019

eladb Feb 28, 2019

eladb Feb 28, 2019

sam-goodwin Feb 28, 2019

eladb Feb 28, 2019

eladb commented Feb 28, 2019

mergify bot commented Sep 23, 2019

feat(core): democratize synthesis and introduce artifacts #1889

feat(core): democratize synthesis and introduce artifacts #1889

Conversation

eladb commented Feb 26, 2019 • edited Loading

Pull Request Checklist

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

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 commented Feb 28, 2019

mergify bot commented Sep 23, 2019

eladb commented Feb 26, 2019 •

edited

Loading