From a9bb9884f2e9fbf68929a229aacb5c604b0b0f0a Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 9 Jul 2020 21:30:02 +0300 Subject: [PATCH 01/29] initial revision --- text/0192-remove-constructs-compat.md | 630 ++++++++++++++++++++++++++ 1 file changed, 630 insertions(+) create mode 100644 text/0192-remove-constructs-compat.md diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md new file mode 100644 index 000000000..78bbacb79 --- /dev/null +++ b/text/0192-remove-constructs-compat.md @@ -0,0 +1,630 @@ +--- +feature name: remove-constructs-compat +start date: 2020-07-05 +rfc pr: (leave this empty) +related issue: [#192](https://github.com/aws/aws-cdk-rfcs/issues/192) +--- + +# Summary + +As part of our effort to broaden the applicability of the CDK's programming +model to other domains such as [Kubernetes](https://cdk8s.io), we have extracted +the base `Construct` class (and a few related types) to an independent library +called [constructs](https://github.com/aws/constructs). + +To preserve backwards compatibility in AWS CDK 1.x, a "compatibility layer" +([construct-compat.ts]) has been added to the AWS CDK. This layer served as an +API shim so that CDK code will continue to function without change. + +As part [AWS CDK v2.0], we plan to remove this redundant layer and make some +improvements to a few APIs in "constructs" based on our learnings from CDK v1.x +and new projects such as CDK for Kubernetes. + +This RFC describes the motivation, implications and plan for this project. + +[AWS CDK v2.0]: https://github.com/aws/aws-cdk-rfcs/issues/156 +[construct-compat.ts]: https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/core/lib/construct-compat.ts + +# Release Notes + +> This section "works backwards" from the v2.0 release notes in order to +> describe the user impact of this change. + +As part of CDK v2.0, all types related to the *constructs programming model* +have been removed from the AWS CDK and should be used directly from the +[constructs](https://github.com/aws/constructs) library. + +``` +BREAKING CHANGE +``` + +For the majority of CDK libraries and apps, you will simply need to add +`constructs@^4` to your `package.json` and replace: + +```ts +import { Construct } from '@aws-cdk/core'; +``` + +with: + +```ts +import { Cosntruct } from 'constructs'; +``` + +The following sections describe all the breaking changes related to this +project. + +## Removal of base types + +The following `@aws-cdk/core` types have stand-in replacements "constructs": + +- The `@aws-cdk/core.Construct` class has been replaced with `constructs.Construct` +- The `@aws-cdk/core.IConstruct` type has been replaced with `constructs.IConstruct` +- The `@aws-cdk/core.ConstructOrder` class has been replaced with `constructs.ConstructOrder` +- The `@aws-cdk/core.ConstructNode` class has been replaced with `constructs.Node` +- `Construct.isConstruct(x)` should be replaced with `x instanceof Construct`. + +## Changes in Aspects API + +Aspects are not part of the "constructs" library, and therefore instead of +`construct.node.applyAspect(aspect)` use `Aspects.of(construct).apply(aspect)`. + +To apply AWS tags to a scope, prefer `Tag.add(scope, name, value)`. + +> TODO: should we change to `Tags.of(scope).add(name, value)`? + +## Changes to IDependable implementation + +The method `construct.node.addDependency(otherConstruct)` did not change, and it +is likely the primary usage, and therefore most users won't be impacted. + +If you need to implement `IDependable`, read on: + +- The `@aws-cdk/core.IDependable` type has been replaced with `constructs.IDependable` +- The `@aws-cdk/core.DependencyTrait` class has been replaced with `constructs.Dependable` +- `@aws-cdk.core.DependencyTrait.get(x)` is now `constructs.Dependable.of(x)` +- `construct.node.dependencies` is now **non-transitive** and returns only the dependencies added to the current node. + +> TODO: You can use `construct.node.dependencyGraph` to access a rich object +> model for reflecting on the node's dependency graph. + +## Stacks as root constructs + +It is common in unit tests to use `Stack` as the root of the tree: + +```ts +const stack = new Stack(); +const myConstruct = new MyConstruct(stack, 'MyConstruct'); +// make assertions +``` + +This is still a supported idiom, but in 2.x these root stacks will have an +implicit `App` parent. This means that `stack.node.scope` will be an `App` +instance, while previously it was `undefined`. The "root" stack will have a +construct ID of `Stack` (unless otherwise specified). + +This has implications on the value of `construct.node.path` (it will be prefixed +with `Stack/`) and the value of `construct.node.uniqueId` (and any derivatives +of these two). + +This means that as you migrate to v2.x, some unit tests may need to be updated +to reflect this change. + +## Stack traces no longer attached to metadata by default + +For performance reasons, the `construct.addMetadata()` method will *not* attach +stack traces to metadata entries. You can explicitly request to attach metadata +by passing `{ stackTrace: true }` as the the 3rd argument to `addMetadata()`. + +## Lifecycle hooks removal + +The "constructs" library intentionally does not include support for `prepare()` +and `synthesize()` as life-cycle hooks. As our understanding of the CDK evolved, +we realized that these activities are domain-specific and should be implemented +at the application scope (in the AWS CDK, this is the `Stage`/`App`). + +To that end, one of the major changes of v2.x is removal of support for the +`prepare()` and `synthesize()` hooks. In most cases, an alternative solution is +easy to come by. + +> TODO: create a github issue for consultation around this topic + +### Prepare + +The **prepare** hook (`construct.onPrepare()` and `construct.prepare()`) is no +longer supported as it can be abused easily and cause construct tree corruption. + +The `ConstructNode.prepare(node)` method no longer exists. To prepare an app or +a stage, simply call `app.synth()` or `stage.synth()`. + +To obtain the top-level app stage from any construct, you can use: + +```ts +const stage = Stage.of(construct); +stage.synth(); +``` + +Consider a design where you mutate the tree in-band (preferable), or use `Lazy` +values or Aspects if appropriate. + +### Synthesis + +The **synthesis** hook (`construct.onSynthesize()` and `construct.synthesize()`) +is no longer supported. Synthesis is now implemented only at the app level. + +The `ConstructNode.synthesize(node)` method no longer exists. To prepare an app +or a stage, simply call `app.synth()` or `stage.synth()`. + +To write files into the cloud assembly directory, use `Stage.of(this).outdir`. + +### Validation + +Validation is still supported, but instead of a protected base class method it +is defined through an interface called `constructs.IValidation`. + +Practically this means that if you wish to implement validation for a custom +construct, implement the `IValidation` interface and change your `validate` +method from `protected` to `public`. + +The static method `ConstructNode.validate(node)` is no longer available. You can +use `construct.node.validate()` which only validates the _current_ construct and +returns the list of error messages (whether or not the construct implements +`IValidation`). + +## Summary + +The following table summarizes the API changes between 1.x and 2.x: + +1.x|2.x +------|----- +`@aws-cdk/*` | `aws-cdk-lib` and `constructs@^4` +`import { Construct } from '@aws-cdk/core'` | `import { Construct } from 'constructs'` +`@aws-cdk/core.Construct` | `constructs.Construct` +`@aws-cdk/core.IConstruct` | `constructs.IConstruct` +`@aws-cdk/core.ConstructOrder` | `constructs.ConstructOrder` +`@aws-cdk/core.ConstructNode` | `constructs.Node` +`Construct.isConstruct(x)` | `x instanceof Construct` +`construct.node.applyAspect(aspect)` | `Aspects.of(construct).apply(aspect)` +`@aws-cdk/core.IDependable` | `constructs.IDependable` +`@aws-cdk/core.DependencyTrait` | `constructs.Dependable` +`@aws-cdk.core.DependencyTrait.get(x)` | `constructs.Dependable.of(x)` +`construct.node.dependencies` | Is now non-transitive +`construct.addMetadata()` | Stack trace not attached by default +`ConstructNode.prepare(node)`, `onPrepare()`, `prepare()` | Not supported +`ConstructNode.synthesize(node)`, `onSynthesize()`, `synthesize()` | Not supported +`construct.onValidate()`, `construct.validate()` hooks | Implement `constructs.IValidation` +`ConstructNode.validate(node)` | `construct.node.validate()` + +# Motivation + +There are various motivations for this change: + +1. Removal of a redundant layer +2. User confusion caused by multiple `Construct` types +3. Inability to compose AWS CDK constructs into other domains +4. Non-intuitive dependency requirement on `constructs` + +### 1. Redundant layer + +The current compatibility layer does not have any logic in it. It is pure glue +introduced in order to avoid breaking v1.x users. As we release v2.0 we are able +to clean up this layer, and with it, improve maintainability and code hygiene. + +### 2. Multiple `Construct` types + +The current situation is error-prone since we have two `Construct` classes in +the type closure. For example, when a developer types `"Construct"` and uses +VSCode to _automatically add import statements_, the IDE will actually add an +import for `constructs.Construct`, which is _not_ the type they need. Using the +wrong type will fail during compilation (similar error as above). + +### 3. Composability with other domains + +The main motivation for this change is to enable composition of AWS CDK +constructs with constructs from other domains. + +It is currently impossible to define AWS CDK constructs within a non-AWS-CDK +construct scope. + +For example, consider the +[Terrastack](https://github.com/TerraStackIO/terrastack) project or a similar +one, which uses CDK constructs to define stacks through Terraform. Say we want to +use an AWS CDK L2 inside a Terraform stack construct: + +```ts +const stack = new terrastack.Stack(...); + +// COMPILATION ERROR: `this` is not a `cdk.Construct`. +new s3.Bucket(this, 'my-bucket'); +``` + +Being able to create construct compositions from multiple domains is a powerful +direction for the CDK ecosystem, and this change is required in order to enable +these use case. + +### 4. Non-intuitive dependency requirement + +As we transition to +[monolithic packaging] +as part of v2.x, CDK users will have to take a _peer dependency_ on both the CDK +library (`aws-cdk-lib`) and `constructs`. + +The reason `constructs` will also be required (whether leave the compatibility +layer or not) is due to the fact that all CDK constructs eventually extend the +base `constructs.Construct` class. This means that this type is part of their +public API and therefore a peer dependency is required (otherwise, there could +be incompatible copies of `Construct` in the node runtime). + +See the RFC for [monolithic packaging] for more details. + +[monolithic packaging]: https://github.com/aws/aws-cdk-rfcs/blob/master/text/0006-monolothic-packaging.md + +# Design + +This section analysis the required changes and discusses the implementation +approach and alternatives. + +This design is based on this [proof of concept](). + +## Removal of the base types (`cdk.Construct`, `cdk.IConstruct`, ...) + +Once `construct-compat.ts` is removed from `@aws-cdk/core`, all CDK code (both +the framework itself and user code) would need to be changed to use the types +from `constructs`. + +Since the APIs are similar in almost all cases, this is a simple mechanical +change as shown in the [release notes](#release-notes). + +The main concern for the CDK codebase is maintaining this change alongside a 1.x +branch until we switch over to 2.x. Since this change includes modifications to +all `import` sections in almost all of our files, merge conflicts are imminent. + +Possible solutions: +1. Take the hit and resolve the conflicts manually every time. These are + mechanical fixes and easy to do. +2. Add some automation using eslint to fix these merge conflicts. +3. Perform the change on the 1.x codebase such that the soon-to-be-moved types + are imported separately from other types of `@aws-cdk/core`. This will + violate an eslint rule, but we can disable it for the time being. + +We can start with #1 for a while and see how bad it is. If it is, we can try to +introduce some automation or backporting. + +## Removal of "synthesize" + +Version 4.x of the `constructs` library does not contain a lifecycle hook for +synthesis. + +### Motivation + +The reason this is not available at the base class is because the abstraction +did not "hold water" as the AWS CDK evolved and new CDKs emerged. In the AWS +CDK, we eventually ended up with a centralized synthesis logic at the +`Stage`-level ([TODO: ref]). The main reason was that we needed to "break" the +recursion in various domain-specific points (e.g. stages, nested stacks) which +meant that the generic logic of "traverse the entire tree and call `synthesize`" +did not hold. In `cdk8s`, the support for [TODO: chart dependencies] required +that the name of the output manifest will be determined based on the topologic +order at the app level. Here again, the generic approach failed. + +In leu of those failures, we decided that there is no additional value in +actually offering a synthesis mechanism at the `constructs` level. Each +CDK-domain implements synthesis at the "right" level. This does not mean that +specific domains can't offer a decentralized approach (i.e. call a method called +"synthesize" on all constructs in the tree), it just means that this is not +provided at the base `Construct` class. + +### Implications on CDK code base + +In the AWS CDK itself, `synthesize()` was used in three locations: + +1. `Stack` - creates a CFN template and adds itself to the cloud assembly manifest. +2. `AssetStaging` - stages an asset into the cloud assembly. +3. `TreeMetadata` - creates `tree.json` with information about the construct tree. + +For `Stack` and `TreeMetadata`, we will convert the generic `synthesize()` +method to `_synthesizeTemplate()` and `_synthesizeTree()` and will call them +from the centralized synthesis function. + +The `AssetStaging` construct does not really need to wait until synthesis in +order to stage the asset. In fact, all the information required already exists +during initialization. The only missing information is the cloud assembly output +directory, and this information is actually known during initialization (we know +this as soon as the CDK app is created). Therefore, the solution for +`AssetStaging` is to move the staging logic to the constructor and use +`Stage.of(this).outdir` to find the output directory of the current stage. + +### Implications on end-users + +Participation in synthesis is an "advanced" feature of the CDK framework and se +assume most end-users don't use this directly. + +If they need "last minute processing", they would likely use `prepare()` (which +is also being [TODO: removed]) but not `synthesize()`. + +The use case of emitting arbitrary files into the cloud assembly directory is +weak. The cloud assembly is a well-defined format, and is currently "closed". +There are no extension points that tools can identify. To that end, just writing +files to the cloud assembly output directory does not make tons of sense. Having +said that, it is still possible to do in the same way we plan for +`AssetStaging`. + + +### Using stacks as roots for unit tests + +If we move staging of assets to the initialization phase, it means we need to +know at that time where is the cloud assembly output directory. As mentioned +above, in production this information is available from the enclosing `Stage`. + +This works in production but introduces a minor issue with unit tests which use +a `Stack` as the root construct. This is a very common pattern in the CDK +codebase today: + +```ts +const stack = new Stack(); +const foo = new MyFoo(stack, 'MyFoo'); + +expect(stack).to(haveResource('AWS::Foo')); +``` + +In such cases, assets will not work because the output directory is only +determined later (in the `expect` call). + +One alternative would be to simply modify these unit tests so that stacks are no +longer used as roots (basically add `new App()` as the scope). This approach +would require a change in many unit tests across the code base with no clear +value to users. + +We propose to modify the `Stack` construct such that if a stack is created +without an explicit `scope`, an `App` instance will automatically be created and +used as it's scope. + +```ts +const stack = new Stack(); +assert(stack.node.scope instanceof App); // previously it was `undefined` +``` + +Since only the root construct may have an empty ID, we will also need to assign +an ID. We propose to use `"Stack"` since we already have fallback logic that +uses this as the stack name when the stack does not have an ID (see [TODO]). + +This change will allow us to remove any special casing we have for stacks in the +testing framework and throughout the synthesis code path (we have quite a lot of +that), because we will be able to assume that `Stage.of(construct.node.root)` is +never `undefined` and has a `synth()` method which returns a cloud assembly. + +Since unit tests sometimes use "incremental tests" for synthesized templates, +and `stage.synth()` would reuse the synthesized output if called twice, we will +also need to introduce a `stage.synth({ forceResynth: true })` option. This will +be the default behavior when using `expect(stack)` or `SynthUtils.synth()`. + +The main side effect of this change is that construct paths in unit tests will +now change. In the above example, `foo.node.path` will change from `MyFoo` to +`Stack/MyFoo`. Additionally, tests for resources that utilized `node.uniqueId` +to generate names will also change given `uniqueId` is based on the path. + +Since app-less stacks are only used during tests, this should not have +implications on production code, but it does break some of our test suite. + +### Should we do this on 1.x? + +In order to reduce [merge conflicts](#repository-migration-efforts) between 1.x +and 2.x we propose to introduce this change on the 1.x branch prior to forking +off 2.x. + +However, this could be perceived as a breaking change by end-users since +`node.path` and `node.uniqueId`, and their derivatives, will change for trees +rooted by a `Stack` and unit tests will need to be updated. + +Therefore we propose to introduce this change as a feature flag over the 1.x codebase and enable it in our unit tests (I don't believe we have a way to enable feature flags for all unit tests, but we can devise one). + +This will allow us to update our tests in 1.x and avoid the merge conflicts +forking on 2.x + +---- + + + + +constructs 4.x: + +- [x] Migrate to projen +- [x] Branch to 4.x and release as @next +- [x] Reintroduce `c.node` instead of `Node.of(c)` +- [x] Removal of `onPrepare` and `onSynthesize` and all synthesis-related code. +- [x] Reintroduce dependencies +- [x] Change `node.dependencies` to return the list of node dependency (non recursive) and add `node.depgraph` which returns a `Graph` object from cdk8s. +- [x] Stack trace control + +CDK changes: + +- [x] cfn2ts +- [ ] Consider aliasing in TypeScript to reduce potential merge conflicts. + +- [ ] Require `app` when defining a `Stack`. +- [ ] assets/compat.ts + + + + + +## Removal of "prepare" + +The "prepare" hook was removed from constructs since it is a very fragile API. +Since the tree can be mutated during prepare, the order of `prepare` invocations +becomes critical, and almost impossible to get right without a rich model of +relationships between these "prepare" calls. + +The prepare hook was used in the CDK in a few cases: +1. Resolution of references across stacks and nested stacks +2. Resolution of dependencies between constructs across stacks +3. Calculation of logical IDs based on hashes of other resources (API GW + Deployment, Lambda Version). + +The first two use cases have already been addressed by centralizing the "prepare" logic at the stage level (into [prepare-app.ts](TODO)). + +The 3rd item can be addressed using `Lazy` tokens, and will be addressed on 1.x +prior to the 2.x fork. + +## Validation changes + +Since construct validation is quite rare, in constructs 4.x, the `validate()` protected method was removed to clean up the namespace. The alternative is to implement an interface `IValidation` with a similar method. + +The breaking change is that users who implemented `validate()` would need to +implement `IValidation` and change their `validate()` method from `protected` to +`public`. + +## Stack trace settings + +Since stack traces are not attached to metadata entries by default in constructs +4.x, we will need to pass `stackTrace: true` for `CfnResource`s. This will +preserve the deploy-time stack traces which are very important for users. + +Other metadata entries will not get stack traces by default, and that's a +reasonable change. + +When stack traces are disabled (either through the CDK context or through +`CDK_STACK_TRACE_DISABLE`), we will need to set the appropriate context key in +`App` so that this will propagate to "constructs". + +## Info/warning/error message metadata key changes + +The construct metadata keys for `addInfo()`, `addWarning()` and `addError()` are +`aws:cdk:info`, etc. These are not the keys used by default in "constructs" +since the library is not part of the AWS CDK. + +To address this, the "constructs" library allows customizing these keys through a settings module. We will need to add that configuration to the `App` level so this behavior will be preserved. + +Alternatively, we can consider also modifying the CLI to accept the new keys as +well, but since these are weakly coupled, it may introduce unwanted breakage, +without much need. + +We recommend the first approach. + +# Drawbacks + +## User migration effort + +The main drawback from users' point of view is the introduction of the +aforementioned breaking changes as part of the transition to CDK 2.0. As +mentioned above, for the majority of users, the migration will be trivial and +mechanical (import from "constructs" instead of "@aws-cdk/core"). + +The removal of the "prepare" and "synthesize" hooks may require users to rethink +their design in very advanced scenarios. We will create a GitHub issue to +consult users on alternative designs. + +## CDK codebase migration efforts + +The AWS CDK codebase itself utilizes all of these APIs, and the migration effort +is quite substantial. + +Having said that, the majority of this work is already complete and a branch is +being maintained with these changes as a pre-cursor to the v2.x fork. + +## Staging of the 2.x fork + +Since this change involves modifications to the CDK's source code, it may cause +merge conflicts as during the period in which we need to forward-port or +back-port code between the v1.x branch and the v2.x branches. + +The key would be to continuously merge between the branches. + +# Rationale and Alternatives + +As a general rule, software layers which do not provider value to users should +not exist. The constructs compatibility layer was added as solution for +maintaining backwards compatibility within the v1.x version line while we +extract `constructs` into an independent library. + +Redundant layers are expensive to maintain and are prone to idiosyncrasies as +they evolve over time (for example, a CDK engineer may be tempted to add an +AWS-specific feature in this layer, making it harder to clean up later). + +If we consider the various [reasons](#drawbacks) not to take this change, the +main reason would be to simplify the [migration for users from 1.x to +2.x](#user-migration-effort). The major version 2.x is already required to +introduce monolithic packaging, and this change, for most users, is likely to be +trivial (see [above](#breaking-changes)). Therefore, we believe this is probably +not the correct motivation to reject this proposal. + +The [repository migration](#repository-migration-efforts) efforts and +[co-existence of 2.x/1.x](#co-existence-of-2x1x) are both one-off costs this +proposal suggests ways to reduce the chance for merge conflicts across these +branches. + +## Alternatives considered + +At a high-level, we may consider to postpone this change to v3.x or to never +take it, leaving this compatibility layer in place for eternity. + +If we examine the various [motivations](#motivation) for this change, we may +come up with various alternatives, all of which eventually cause a breaking +change to our users. + +For example, we considered only changing the type of the `scope` argument to all +CDK constructs to use `constructs.Construct`, while the base class will still +extend `cdk.Construct`. This will likely confuse users who design their own +constructs as they won't know which construct to extend, the two base classes +will slowly diverge from each other as both layers evolve. + +Another alternative is to rename `cdk.Construct` to something like +`AwsConstruct`. This, would take up most of the cost of this change (which is +the CDK codebase change and merge risks against the fork). + +Postponing to v3.x will leave us with the set exact set of problems, only with a more mature ecosystem which is harder to migrate off of. + +The [Design](#design) section describes alternatives for various aspects of this +project. + +# Adoption Strategy + +See [Release Notes](#release-notes). + +# Unresolved questions + +- [ ] Automation of `import` conflict resolution. + +> - What parts of the design do you expect to resolve through the RFC process +> before this gets merged? +> - What parts of the design do you expect to resolve through the implementation +> of this feature before stabilization? +> - What related issues do you consider out of scope for this RFC that could be +> addressed in the future independently of the solution that comes out of this +> RFC? + +# Future Possibilities + +> Think about what the natural extension and evolution of your proposal would be +> and how it would affect CDK as whole. Try to use this section as a tool to more +> fully consider all possible interactions with the project and ecosystem in your +> proposal. Also consider how this fits into the roadmap for the project. +> +> This is a good place to "dump ideas", if they are out of scope for the RFC you +> are writing but are otherwise related. +> +> If you have tried and cannot think of any future possibilities, you may simply +> state that you cannot think of anything. + +# Implementation Plan + +INTENTIONALLY LEFT BLANK: an implementation plan will be added when the RFC is +scheduled for implementation. + +> The implementation plan should analyze all the tasks needed in order to +> implement this RFC, and the planned order of implementation based on +> dependencies and analysis of the critical path. +> +> Either add the plan here or add link which references a separate document +> and/or a GitHub Project Board (and reference it here) which manages the +> execution of your plan. + +We will try to front load as much of this change to 1.x in order to reduce the +merge conflict potential. + +- [ ] Initial prototype +- [ ] Migration guide in https://github.com/aws/aws-cdk/issues/8909 +- [ ] GitHub issue for "synthesize" and "prepare" guidance. +- [ ] Remove the use of "prepare" and "synthesize" in 1.x +- [ ] Implicit `App` for `Stack`s without a scope behind a feature flag and + enable in our unit tests in 1.x \ No newline at end of file From 62101d2780505aa5de066d8240c2e09acd5d957e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 9 Jul 2020 21:39:16 +0300 Subject: [PATCH 02/29] add toc --- text/0192-remove-constructs-compat.md | 40 +++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 78bbacb79..7f092c4d8 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -25,19 +25,49 @@ This RFC describes the motivation, implications and plan for this project. [AWS CDK v2.0]: https://github.com/aws/aws-cdk-rfcs/issues/156 [construct-compat.ts]: https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/core/lib/construct-compat.ts +# Table of Contents + +- [Summary](#summary) +- [Table of Contents](#table-of-contents) +- [Release Notes](#release-notes) + - [Removal of base types](#removal-of-base-types) + - [Changes in Aspects API](#changes-in-aspects-api) + - [Changes to IDependable implementation](#changes-to-idependable-implementation) + - [Stacks as root constructs](#stacks-as-root-constructs) + - [Stack traces no longer attached to metadata by default](#stack-traces-no-longer-attached-to-metadata-by-default) + - [Lifecycle hooks removal](#lifecycle-hooks-removal) + - [Summary](#summary-1) +- [Motivation](#motivation) +- [Design](#design) + - [Removal of the base types (`cdk.Construct`, `cdk.IConstruct`, ...)](#removal-of-the-base-types-cdkconstruct-cdkiconstruct-) + - [Removal of "synthesize"](#removal-of-synthesize) + - [Removal of "prepare"](#removal-of-prepare) + - [Validation changes](#validation-changes) + - [Stack trace settings](#stack-trace-settings) + - [Info/warning/error message metadata key changes](#infowarningerror-message-metadata-key-changes) +- [Drawbacks](#drawbacks) + - [User migration effort](#user-migration-effort) + - [CDK codebase migration efforts](#cdk-codebase-migration-efforts) + - [Staging of the 2.x fork](#staging-of-the-2x-fork) +- [Rationale and Alternatives](#rationale-and-alternatives) + - [Alternatives considered](#alternatives-considered) +- [Adoption Strategy](#adoption-strategy) +- [Unresolved questions](#unresolved-questions) +- [Future Possibilities](#future-possibilities) +- [Implementation Plan](#implementation-plan) + + # Release Notes > This section "works backwards" from the v2.0 release notes in order to > describe the user impact of this change. +**BREAKING CHANGE** + As part of CDK v2.0, all types related to the *constructs programming model* have been removed from the AWS CDK and should be used directly from the [constructs](https://github.com/aws/constructs) library. -``` -BREAKING CHANGE -``` - For the majority of CDK libraries and apps, you will simply need to add `constructs@^4` to your `package.json` and replace: @@ -52,7 +82,7 @@ import { Cosntruct } from 'constructs'; ``` The following sections describe all the breaking changes related to this -project. +project: ## Removal of base types From 7478396a0c936231ea9e43ad87e0979725ea96ef Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 9 Jul 2020 21:43:52 +0300 Subject: [PATCH 03/29] add links to POC --- text/0192-remove-constructs-compat.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 7f092c4d8..dd8ac3e93 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -294,7 +294,7 @@ See the RFC for [monolithic packaging] for more details. This section analysis the required changes and discusses the implementation approach and alternatives. -This design is based on this [proof of concept](). +This design is based on this [proof of concept](https://github.com/aws/aws-cdk/pull/8962). ## Removal of the base types (`cdk.Construct`, `cdk.IConstruct`, ...) @@ -652,7 +652,7 @@ scheduled for implementation. We will try to front load as much of this change to 1.x in order to reduce the merge conflict potential. -- [ ] Initial prototype +- [ ] Initial prototype: https://github.com/aws/aws-cdk/pull/8962 - [ ] Migration guide in https://github.com/aws/aws-cdk/issues/8909 - [ ] GitHub issue for "synthesize" and "prepare" guidance. - [ ] Remove the use of "prepare" and "synthesize" in 1.x From 2706a814a37fae66e672857f64d82fb601f16265 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 9 Jul 2020 21:46:43 +0300 Subject: [PATCH 04/29] tweaks --- text/0192-remove-constructs-compat.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index dd8ac3e93..1e807d662 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -318,7 +318,12 @@ Possible solutions: violate an eslint rule, but we can disable it for the time being. We can start with #1 for a while and see how bad it is. If it is, we can try to -introduce some automation or backporting. +introduce some automation or back-porting. + +Something that we can definitely do on 1.x is replace all the variants such as +`core.Construct` and `cdk.Construct` to just `Construct`. This will slightly +help. + ## Removal of "synthesize" @@ -493,8 +498,9 @@ The prepare hook was used in the CDK in a few cases: The first two use cases have already been addressed by centralizing the "prepare" logic at the stage level (into [prepare-app.ts](TODO)). -The 3rd item can be addressed using `Lazy` tokens, and will be addressed on 1.x -prior to the 2.x fork. +The 3rd item can be addressed using `Lazy` tokens (see +[example](https://github.com/aws/aws-cdk/pull/8962/files#diff-51d435d71a31c2607f923fc4d96cac56R140)), +and will be addressed on 1.x prior to the 2.x fork. ## Validation changes @@ -657,4 +663,5 @@ merge conflict potential. - [ ] GitHub issue for "synthesize" and "prepare" guidance. - [ ] Remove the use of "prepare" and "synthesize" in 1.x - [ ] Implicit `App` for `Stack`s without a scope behind a feature flag and - enable in our unit tests in 1.x \ No newline at end of file + enable in our unit tests in 1.x +- [ ] Normalize reference to base types (`cdk.Construct` => `Construct`). \ No newline at end of file From 811a19775e4957663864148c3042724df5bac918 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 12 Jul 2020 13:12:54 +0300 Subject: [PATCH 05/29] CR comments --- text/0192-remove-constructs-compat.md | 36 ++++++++++++++++----------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 1e807d662..288d9919d 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -23,7 +23,7 @@ and new projects such as CDK for Kubernetes. This RFC describes the motivation, implications and plan for this project. [AWS CDK v2.0]: https://github.com/aws/aws-cdk-rfcs/issues/156 -[construct-compat.ts]: https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/core/lib/construct-compat.ts +[construct-compat.ts]: https://github.com/aws/aws-cdk/blob/fcca86e82235387b88371a0682cd0fc88bc1b67e/packages/%40aws-cdk/core/lib/construct-compat.ts # Table of Contents @@ -92,7 +92,6 @@ The following `@aws-cdk/core` types have stand-in replacements "constructs": - The `@aws-cdk/core.IConstruct` type has been replaced with `constructs.IConstruct` - The `@aws-cdk/core.ConstructOrder` class has been replaced with `constructs.ConstructOrder` - The `@aws-cdk/core.ConstructNode` class has been replaced with `constructs.Node` -- `Construct.isConstruct(x)` should be replaced with `x instanceof Construct`. ## Changes in Aspects API @@ -182,10 +181,16 @@ values or Aspects if appropriate. The **synthesis** hook (`construct.onSynthesize()` and `construct.synthesize()`) is no longer supported. Synthesis is now implemented only at the app level. -The `ConstructNode.synthesize(node)` method no longer exists. To prepare an app -or a stage, simply call `app.synth()` or `stage.synth()`. +If your use case for overriding `synthesize()` was to write into the cloud +assembly directory, you can now find the current cloud assembly output directory +using `Stage.of(this).outdir`. -To write files into the cloud assembly directory, use `Stage.of(this).outdir`. +The `ConstructNode.synthesize(node)` method no longer exists. However, since now +`Stage.of(construct)` is always defined and returns the enclosing stage/app, you +can can synthesize a construct node through `Stage.of(construct).synth()`. + +For additional questions/guidance on how to implement your use case without this +hook, please post a comment on this GitHub issue: [TODO]. ### Validation @@ -279,11 +284,12 @@ As we transition to as part of v2.x, CDK users will have to take a _peer dependency_ on both the CDK library (`aws-cdk-lib`) and `constructs`. -The reason `constructs` will also be required (whether leave the compatibility -layer or not) is due to the fact that all CDK constructs eventually extend the -base `constructs.Construct` class. This means that this type is part of their -public API and therefore a peer dependency is required (otherwise, there could -be incompatible copies of `Construct` in the node runtime). +The reason `constructs` will also be required (whether we leave the +compatibility layer or not) is due to the fact that all CDK constructs +eventually extend the base `constructs.Construct` class. This means that this +type is part of their public API and therefore a peer dependency is required +(otherwise, there could be incompatible copies of `Construct` in the node +runtime). See the RFC for [monolithic packaging] for more details. @@ -315,7 +321,7 @@ Possible solutions: 2. Add some automation using eslint to fix these merge conflicts. 3. Perform the change on the 1.x codebase such that the soon-to-be-moved types are imported separately from other types of `@aws-cdk/core`. This will - violate an eslint rule, but we can disable it for the time being. + violate an eslint rule (TODO), but we can disable it for the time being. We can start with #1 for a while and see how bad it is. If it is, we can try to introduce some automation or back-porting. @@ -430,7 +436,7 @@ never `undefined` and has a `synth()` method which returns a cloud assembly. Since unit tests sometimes use "incremental tests" for synthesized templates, and `stage.synth()` would reuse the synthesized output if called twice, we will -also need to introduce a `stage.synth({ forceResynth: true })` option. This will +also need to introduce a `stage.synth({ force: true })` option. This will be the default behavior when using `expect(stack)` or `SynthUtils.synth()`. The main side effect of this change is that construct paths in unit tests will @@ -568,7 +574,7 @@ The key would be to continuously merge between the branches. # Rationale and Alternatives -As a general rule, software layers which do not provider value to users should +As a general rule, software layers which do not provide value to users should not exist. The constructs compatibility layer was added as solution for maintaining backwards compatibility within the v1.x version line while we extract `constructs` into an independent library. @@ -664,4 +670,6 @@ merge conflict potential. - [ ] Remove the use of "prepare" and "synthesize" in 1.x - [ ] Implicit `App` for `Stack`s without a scope behind a feature flag and enable in our unit tests in 1.x -- [ ] Normalize reference to base types (`cdk.Construct` => `Construct`). \ No newline at end of file +- [ ] Normalize reference to base types (`cdk.Construct` => `Construct`). +- [ ] Import https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s/src/dependency.ts to "constructs" +- [ ] `constructs` documentation on how to implement centralized synthesis. \ No newline at end of file From 2f23bc21325bdc3c32c0e6c238730fe7f019f69e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 12 Jul 2020 15:51:25 +0300 Subject: [PATCH 06/29] CR updates --- text/0192-remove-constructs-compat.md | 130 +++++++++++++++----------- 1 file changed, 76 insertions(+), 54 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 288d9919d..abc99a358 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -44,7 +44,7 @@ This RFC describes the motivation, implications and plan for this project. - [Removal of "prepare"](#removal-of-prepare) - [Validation changes](#validation-changes) - [Stack trace settings](#stack-trace-settings) - - [Info/warning/error message metadata key changes](#infowarningerror-message-metadata-key-changes) + - [Logging API changes](#logging-api-changes) - [Drawbacks](#drawbacks) - [User migration effort](#user-migration-effort) - [CDK codebase migration efforts](#cdk-codebase-migration-efforts) @@ -68,7 +68,7 @@ As part of CDK v2.0, all types related to the *constructs programming model* have been removed from the AWS CDK and should be used directly from the [constructs](https://github.com/aws/constructs) library. -For the majority of CDK libraries and apps, you will simply need to add +For most CDK libraries and apps, you will probably just need to add `constructs@^4` to your `package.json` and replace: ```ts @@ -104,8 +104,8 @@ To apply AWS tags to a scope, prefer `Tag.add(scope, name, value)`. ## Changes to IDependable implementation -The method `construct.node.addDependency(otherConstruct)` did not change, and it -is likely the primary usage, and therefore most users won't be impacted. +The method `construct.node.addDependency(otherConstruct)` __did not change__, +which is likely the only API you are using related to construct dependencies. If you need to implement `IDependable`, read on: @@ -141,9 +141,13 @@ to reflect this change. ## Stack traces no longer attached to metadata by default -For performance reasons, the `construct.addMetadata()` method will *not* attach -stack traces to metadata entries. You can explicitly request to attach metadata -by passing `{ stackTrace: true }` as the the 3rd argument to `addMetadata()`. +For performance reasons, the `construct.node.addMetadata()` method will *not* +attach stack traces to metadata entries. You can explicitly request to attach +stack traces to a metadata entry using the `stackTrace` option: + +```ts +construct.node.addMetadata(key, value, { stackTrace: true }) +``` ## Lifecycle hooks removal @@ -201,6 +205,9 @@ Practically this means that if you wish to implement validation for a custom construct, implement the `IValidation` interface and change your `validate` method from `protected` to `public`. +If you implemented validation through the `onValidate()` method, it should be +renamed to `validate()`. + The static method `ConstructNode.validate(node)` is no longer available. You can use `construct.node.validate()` which only validates the _current_ construct and returns the list of error messages (whether or not the construct implements @@ -250,8 +257,10 @@ to clean up this layer, and with it, improve maintainability and code hygiene. The current situation is error-prone since we have two `Construct` classes in the type closure. For example, when a developer types `"Construct"` and uses VSCode to _automatically add import statements_, the IDE will actually add an -import for `constructs.Construct`, which is _not_ the type they need. Using the -wrong type will fail during compilation (similar error as above). +import for `constructs.Construct`. If they define a custom construct class which +extends this type instead of the `core.Construct` type, it won't be possible to +pass an instance of this class as a scope to AWS CDK constructs such as `Stack` +for example. ### 3. Composability with other domains @@ -279,10 +288,12 @@ these use case. ### 4. Non-intuitive dependency requirement -As we transition to -[monolithic packaging] -as part of v2.x, CDK users will have to take a _peer dependency_ on both the CDK -library (`aws-cdk-lib`) and `constructs`. +As we transition to [monolithic packaging] as part of v2.x, CDK users will have +to take a _peer dependency_ on both the CDK library (`aws-cdk-lib`) and +`constructs`. + +> Currently, the AWS CDK also takes `constructs` as a normal dependency (similar +> to all dependencies), but this is about to change with mono-cdk. The reason `constructs` will also be required (whether we leave the compatibility layer or not) is due to the fact that all CDK constructs @@ -434,10 +445,27 @@ testing framework and throughout the synthesis code path (we have quite a lot of that), because we will be able to assume that `Stage.of(construct.node.root)` is never `undefined` and has a `synth()` method which returns a cloud assembly. -Since unit tests sometimes use "incremental tests" for synthesized templates, -and `stage.synth()` would reuse the synthesized output if called twice, we will -also need to introduce a `stage.synth({ force: true })` option. This will -be the default behavior when using `expect(stack)` or `SynthUtils.synth()`. +Unit tests sometimes use "incremental tests" for synthesized templates. For +example: + +```ts +const stack = new Stack(); +const c1 = new MyConstruct(stack, 'c1', { foos: [ 'bar' ] }); +expect(stack).toHaveResource('AWS::Resource', { + Foos: [ 'bar' ] +}); + +// now add a "foo" and verify that the synthesized template contains two items +c1.addFoo('baz'); +expect(stack).toHaveResource('AWS::Resource', { + Foos: [ 'bar', 'baz' ] +}); +``` + +Since `stage.synth()` (which is called by `expect(stack)`) would reuse the +synthesized output if called twice, we will also need to introduce a +`stage.synth({ force: true })` option. This will be the default behavior when +using `expect(stack)` or `SynthUtils.synth()`. The main side effect of this change is that construct paths in unit tests will now change. In the above example, `foo.node.path` will change from `MyFoo` to @@ -462,33 +490,6 @@ Therefore we propose to introduce this change as a feature flag over the 1.x cod This will allow us to update our tests in 1.x and avoid the merge conflicts forking on 2.x ----- - - - - -constructs 4.x: - -- [x] Migrate to projen -- [x] Branch to 4.x and release as @next -- [x] Reintroduce `c.node` instead of `Node.of(c)` -- [x] Removal of `onPrepare` and `onSynthesize` and all synthesis-related code. -- [x] Reintroduce dependencies -- [x] Change `node.dependencies` to return the list of node dependency (non recursive) and add `node.depgraph` which returns a `Graph` object from cdk8s. -- [x] Stack trace control - -CDK changes: - -- [x] cfn2ts -- [ ] Consider aliasing in TypeScript to reduce potential merge conflicts. - -- [ ] Require `app` when defining a `Stack`. -- [ ] assets/compat.ts - - - - - ## Removal of "prepare" The "prepare" hook was removed from constructs since it is a very fragile API. @@ -529,19 +530,25 @@ When stack traces are disabled (either through the CDK context or through `CDK_STACK_TRACE_DISABLE`), we will need to set the appropriate context key in `App` so that this will propagate to "constructs". -## Info/warning/error message metadata key changes +## Logging API changes -The construct metadata keys for `addInfo()`, `addWarning()` and `addError()` are -`aws:cdk:info`, etc. These are not the keys used by default in "constructs" -since the library is not part of the AWS CDK. +The `construct.node.addInfo()`, `construct.node.addWarning()` and +`construct.node.Error()` methods are now available under the +`Logging.of(construct)` API: -To address this, the "constructs" library allows customizing these keys through a settings module. We will need to add that configuration to the `App` level so this behavior will be preserved. +Instead of: -Alternatively, we can consider also modifying the CLI to accept the new keys as -well, but since these are weakly coupled, it may introduce unwanted breakage, -without much need. +```ts +construct.node.addWarning('my warning'); +``` -We recommend the first approach. +Use: + +```ts +import { Logging } from '@aws-cdk/core'; + +Logging.of(construct).addWarning('my warning'); +``` # Drawbacks @@ -664,6 +671,21 @@ scheduled for implementation. We will try to front load as much of this change to 1.x in order to reduce the merge conflict potential. +constructs 4.x + +- [x] Migrate to projen +- [x] Branch to 4.x and release as @next +- [x] Reintroduce `c.node` instead of `Node.of(c)` +- [x] Removal of `onPrepare` and `onSynthesize` and all synthesis-related code. +- [x] Reintroduce dependencies +- [x] Change `node.dependencies` to return the list of node dependency (non recursive) and add `node.depgraph` which returns a `Graph` object from cdk8s. +- [x] Stack trace control + +CDK changes: + +- [x] cfn2ts +- [ ] Consider aliasing in TypeScript to reduce potential merge conflicts. +- [ ] assets/compat.ts - [ ] Initial prototype: https://github.com/aws/aws-cdk/pull/8962 - [ ] Migration guide in https://github.com/aws/aws-cdk/issues/8909 - [ ] GitHub issue for "synthesize" and "prepare" guidance. @@ -672,4 +694,4 @@ merge conflict potential. enable in our unit tests in 1.x - [ ] Normalize reference to base types (`cdk.Construct` => `Construct`). - [ ] Import https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s/src/dependency.ts to "constructs" -- [ ] `constructs` documentation on how to implement centralized synthesis. \ No newline at end of file +- [ ] `constructs` documentation on how to implement centralized synthesis. From a7840273d1de137fb83cffd84aee2ed85aee8cf8 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 13 Jul 2020 13:47:33 +0300 Subject: [PATCH 07/29] updates --- text/0192-remove-constructs-compat.md | 507 ++++++++++++++++---------- 1 file changed, 315 insertions(+), 192 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index abc99a358..b6670b7d0 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -30,21 +30,26 @@ This RFC describes the motivation, implications and plan for this project. - [Summary](#summary) - [Table of Contents](#table-of-contents) - [Release Notes](#release-notes) - - [Removal of base types](#removal-of-base-types) - - [Changes in Aspects API](#changes-in-aspects-api) - - [Changes to IDependable implementation](#changes-to-idependable-implementation) - - [Stacks as root constructs](#stacks-as-root-constructs) - - [Stack traces no longer attached to metadata by default](#stack-traces-no-longer-attached-to-metadata-by-default) - - [Lifecycle hooks removal](#lifecycle-hooks-removal) - - [Summary](#summary-1) + - [01-BASE-TYPES: Removal of base types](#01-base-types-removal-of-base-types) + - [02-ASPECTS: Changes in Aspects API](#02-aspects-changes-in-aspects-api) + - [03-DEPENDABLE: Changes to IDependable implementation](#03-dependable-changes-to-idependable-implementation) + - [04-STACK-ROOT: Stacks as root constructs](#04-stack-root-stacks-as-root-constructs) + - [05-METADATA-TRACES: Stack traces no longer attached to metadata by default](#05-metadata-traces-stack-traces-no-longer-attached-to-metadata-by-default) + - [06-NO-PREPARE: The `prepare` hook is no longer supported](#06-no-prepare-the-prepare-hook-is-no-longer-supported) + - [07-NO-SYNTHESIZE: The `synthesize` hook is no longer supported](#07-no-synthesize-the-synthesize-hook-is-no-longer-supported) + - [08-VALIDATION: The `validate()` hook is now `node.addValidation()`](#08-validation-the-validate-hook-is-now-nodeaddvalidation) + - [09-LOGGING: Logging API changes](#09-logging-logging-api-changes) - [Motivation](#motivation) - [Design](#design) - - [Removal of the base types (`cdk.Construct`, `cdk.IConstruct`, ...)](#removal-of-the-base-types-cdkconstruct-cdkiconstruct-) - - [Removal of "synthesize"](#removal-of-synthesize) - - [Removal of "prepare"](#removal-of-prepare) - - [Validation changes](#validation-changes) - - [Stack trace settings](#stack-trace-settings) - - [Logging API changes](#logging-api-changes) + - [01-BASE-TYPES](#01-base-types) + - [02-ASPECTS](#02-aspects) + - [03-DEPENDABLE](#03-dependable) + - [04-STACK-ROOT](#04-stack-root) + - [05-METADATA-TRACES](#05-metadata-traces) + - [06-NO-PREPARE](#06-no-prepare) + - [07-NO-SYNTHESIZE](#07-no-synthesize) + - [08-VALIDATION](#08-validation) + - [09-LOGGING](#09-logging) - [Drawbacks](#drawbacks) - [User migration effort](#user-migration-effort) - [CDK codebase migration efforts](#cdk-codebase-migration-efforts) @@ -55,6 +60,9 @@ This RFC describes the motivation, implications and plan for this project. - [Unresolved questions](#unresolved-questions) - [Future Possibilities](#future-possibilities) - [Implementation Plan](#implementation-plan) + - [Base types](#base-types) + - [Synthesis](#synthesis) + - [Prepare](#prepare) # Release Notes @@ -68,56 +76,83 @@ As part of CDK v2.0, all types related to the *constructs programming model* have been removed from the AWS CDK and should be used directly from the [constructs](https://github.com/aws/constructs) library. -For most CDK libraries and apps, you will probably just need to add -`constructs@^4` to your `package.json` and replace: +For most CDK libraries and apps, you will likely just need change this: ```ts import { Construct } from '@aws-cdk/core'; ``` -with: +With this: ```ts import { Cosntruct } from 'constructs'; ``` -The following sections describe all the breaking changes related to this -project: +The following table summarizes the API changes between 1.x and 2.x: -## Removal of base types +1.x|2.x +------|----- +`@aws-cdk/*` | `aws-cdk-lib` and `constructs@^4` +`import { Construct } from '@aws-cdk/core'` | `import { Construct } from 'constructs'` +`@aws-cdk/core.Construct` | `constructs.Construct` +`@aws-cdk/core.IConstruct` | `constructs.IConstruct` +`@aws-cdk/core.ConstructOrder` | `constructs.ConstructOrder` +`@aws-cdk/core.ConstructNode` | `constructs.Node` +`Construct.isConstruct(x)` | `x instanceof Construct` +`construct.node.applyAspect(aspect)` | `Aspects.of(construct).apply(aspect)` +`@aws-cdk/core.IDependable` | `constructs.IDependable` +`@aws-cdk/core.DependencyTrait` | `constructs.Dependable` +`@aws-cdk.core.DependencyTrait.get(x)` | `constructs.Dependable.of(x)` +`construct.node.dependencies` | Is now non-transitive +`construct.addMetadata()` | Stack trace not attached by default +`ConstructNode.prepare(node)`, `onPrepare()`, `prepare()` | Not supported +`ConstructNode.synthesize(node)`, `onSynthesize()`, `synthesize()` | Not supported +`construct.onValidate()`, `construct.validate()` hooks | Implement `constructs.IValidation` and call `node.addValidation()` +`ConstructNode.validate(node)` | `construct.node.validate()` + +The following sections describe all the related breaking changes and details +migration strategies for each change. + +## 01-BASE-TYPES: Removal of base types -The following `@aws-cdk/core` types have stand-in replacements "constructs": +The following `@aws-cdk/core` types have stand-in replacements in `constructs`: - The `@aws-cdk/core.Construct` class has been replaced with `constructs.Construct` - The `@aws-cdk/core.IConstruct` type has been replaced with `constructs.IConstruct` - The `@aws-cdk/core.ConstructOrder` class has been replaced with `constructs.ConstructOrder` - The `@aws-cdk/core.ConstructNode` class has been replaced with `constructs.Node` -## Changes in Aspects API +## 02-ASPECTS: Changes in Aspects API Aspects are not part of the "constructs" library, and therefore instead of `construct.node.applyAspect(aspect)` use `Aspects.of(construct).apply(aspect)`. -To apply AWS tags to a scope, prefer `Tag.add(scope, name, value)`. +The `Tag.add(scope, name, value)` API has been removed. To apply AWS tags to a +scope, use: -> TODO: should we change to `Tags.of(scope).add(name, value)`? - -## Changes to IDependable implementation +```ts +Tags.of(scope).add(name, value); +``` -The method `construct.node.addDependency(otherConstruct)` __did not change__, -which is likely the only API you are using related to construct dependencies. +## 03-DEPENDABLE: Changes to IDependable implementation -If you need to implement `IDependable`, read on: +If you need to implement `IDependable`: -- The `@aws-cdk/core.IDependable` type has been replaced with `constructs.IDependable` -- The `@aws-cdk/core.DependencyTrait` class has been replaced with `constructs.Dependable` +- The `@aws-cdk/core.IDependable` type has been replaced with + `constructs.IDependable` +- The `@aws-cdk/core.DependencyTrait` class has been replaced with + `constructs.Dependable` - `@aws-cdk.core.DependencyTrait.get(x)` is now `constructs.Dependable.of(x)` -- `construct.node.dependencies` is now **non-transitive** and returns only the dependencies added to the current node. +- `construct.node.dependencies` is now **non-transitive** and returns only the + dependencies added to the current node. + +The method `construct.node.addDependency(otherConstruct)` __did not change__ and +can be used as before. > TODO: You can use `construct.node.dependencyGraph` to access a rich object > model for reflecting on the node's dependency graph. -## Stacks as root constructs +## 04-STACK-ROOT: Stacks as root constructs It is common in unit tests to use `Stack` as the root of the tree: @@ -139,7 +174,7 @@ of these two). This means that as you migrate to v2.x, some unit tests may need to be updated to reflect this change. -## Stack traces no longer attached to metadata by default +## 05-METADATA-TRACES: Stack traces no longer attached to metadata by default For performance reasons, the `construct.node.addMetadata()` method will *not* attach stack traces to metadata entries. You can explicitly request to attach @@ -149,93 +184,88 @@ stack traces to a metadata entry using the `stackTrace` option: construct.node.addMetadata(key, value, { stackTrace: true }) ``` -## Lifecycle hooks removal - -The "constructs" library intentionally does not include support for `prepare()` -and `synthesize()` as life-cycle hooks. As our understanding of the CDK evolved, -we realized that these activities are domain-specific and should be implemented -at the application scope (in the AWS CDK, this is the `Stage`/`App`). - -To that end, one of the major changes of v2.x is removal of support for the -`prepare()` and `synthesize()` hooks. In most cases, an alternative solution is -easy to come by. - -> TODO: create a github issue for consultation around this topic - -### Prepare +## 06-NO-PREPARE: The `prepare` hook is no longer supported The **prepare** hook (`construct.onPrepare()` and `construct.prepare()`) is no -longer supported as it can be abused easily and cause construct tree corruption. +longer supported as it can easily be abused and cause construct tree corruption +when the tree is mutated during this stage. -The `ConstructNode.prepare(node)` method no longer exists. To prepare an app or -a stage, simply call `app.synth()` or `stage.synth()`. +Consider a design where you mutate the tree in-band, or use `Lazy` values or +Aspects if appropriate. -To obtain the top-level app stage from any construct, you can use: +[TODO examples from the CDK itself] -```ts -const stage = Stage.of(construct); -stage.synth(); -``` +The `ConstructNode.prepare(node)` method no longer exists. To realize references +& dependencies in a scope call `Stage.of(scope).synth()`. -Consider a design where you mutate the tree in-band (preferable), or use `Lazy` -values or Aspects if appropriate. +## 07-NO-SYNTHESIZE: The `synthesize` hook is no longer supported -### Synthesis +The `synthesize()` overload (or `onSynthesize()`) is no longer supported. +Synthesis is now implemented only at the app level. -The **synthesis** hook (`construct.onSynthesize()` and `construct.synthesize()`) -is no longer supported. Synthesis is now implemented only at the app level. - -If your use case for overriding `synthesize()` was to write into the cloud +If your use case for overriding `synthesize()` was to emit files into the cloud assembly directory, you can now find the current cloud assembly output directory -using `Stage.of(this).outdir`. +during initialization using `Stage.of(this).outdir`. The `ConstructNode.synthesize(node)` method no longer exists. However, since now -`Stage.of(construct)` is always defined and returns the enclosing stage/app, you -can can synthesize a construct node through `Stage.of(construct).synth()`. +`Stage.of(scope)` is always defined and returns the enclosing stage/app, you +can can synthesize a construct node through `Stage.of(scope).synth()`. For additional questions/guidance on how to implement your use case without this hook, please post a comment on this GitHub issue: [TODO]. -### Validation +## 08-VALIDATION: The `validate()` hook is now `node.addValidation()` -Validation is still supported, but instead of a protected base class method it -is defined through an interface called `constructs.IValidation`. +To add validation logic to a construct, use `construct.node.addValidation()` +method instead of overriding a protected `validate()` method: -Practically this means that if you wish to implement validation for a custom -construct, implement the `IValidation` interface and change your `validate` -method from `protected` to `public`. +Before: -If you implemented validation through the `onValidate()` method, it should be -renamed to `validate()`. +```ts +class MyConstruct extends Construct { + protected validate(): string[] { + return [ 'validation-error' ]; + } +} +``` + +After: + +```ts +class MyConstruct extends Construct { + constructor(scope: Construct, id: string) { + super(scope, id); + + this.node.addValidation({ validate: () => [ 'validation-error' ] }); + } +} +``` The static method `ConstructNode.validate(node)` is no longer available. You can use `construct.node.validate()` which only validates the _current_ construct and -returns the list of error messages (whether or not the construct implements -`IValidation`). +returns the list of all error messages returned from calling +`validation.validate()` on all validations added to this node. -## Summary +## 09-LOGGING: Logging API changes -The following table summarizes the API changes between 1.x and 2.x: +The `construct.node.addInfo()`, `construct.node.addWarning()` and +`construct.node.Error()` methods are now available under the +`Logging.of(construct)` API: + +Instead of: + +```ts +construct.node.addWarning('my warning'); +``` + +Use: + +```ts +import { Logging } from '@aws-cdk/core'; + +Logging.of(construct).addWarning('my warning'); +``` -1.x|2.x -------|----- -`@aws-cdk/*` | `aws-cdk-lib` and `constructs@^4` -`import { Construct } from '@aws-cdk/core'` | `import { Construct } from 'constructs'` -`@aws-cdk/core.Construct` | `constructs.Construct` -`@aws-cdk/core.IConstruct` | `constructs.IConstruct` -`@aws-cdk/core.ConstructOrder` | `constructs.ConstructOrder` -`@aws-cdk/core.ConstructNode` | `constructs.Node` -`Construct.isConstruct(x)` | `x instanceof Construct` -`construct.node.applyAspect(aspect)` | `Aspects.of(construct).apply(aspect)` -`@aws-cdk/core.IDependable` | `constructs.IDependable` -`@aws-cdk/core.DependencyTrait` | `constructs.Dependable` -`@aws-cdk.core.DependencyTrait.get(x)` | `constructs.Dependable.of(x)` -`construct.node.dependencies` | Is now non-transitive -`construct.addMetadata()` | Stack trace not attached by default -`ConstructNode.prepare(node)`, `onPrepare()`, `prepare()` | Not supported -`ConstructNode.synthesize(node)`, `onSynthesize()`, `synthesize()` | Not supported -`construct.onValidate()`, `construct.validate()` hooks | Implement `constructs.IValidation` -`ConstructNode.validate(node)` | `construct.node.validate()` # Motivation @@ -311,9 +341,13 @@ See the RFC for [monolithic packaging] for more details. This section analysis the required changes and discusses the implementation approach and alternatives. +For each change, we added a **What can we do on 1.x?** section which discusses +our strategy for front-loading the change into the 1.x branch to reduce forking +costs and/or alert users of the upcoming deprecation. + This design is based on this [proof of concept](https://github.com/aws/aws-cdk/pull/8962). -## Removal of the base types (`cdk.Construct`, `cdk.IConstruct`, ...) +## 01-BASE-TYPES Once `construct-compat.ts` is removed from `@aws-cdk/core`, all CDK code (both the framework itself and user code) would need to be changed to use the types @@ -322,87 +356,88 @@ from `constructs`. Since the APIs are similar in almost all cases, this is a simple mechanical change as shown in the [release notes](#release-notes). +### What can we do on 1.x? + The main concern for the CDK codebase is maintaining this change alongside a 1.x branch until we switch over to 2.x. Since this change includes modifications to all `import` sections in almost all of our files, merge conflicts are imminent. -Possible solutions: -1. Take the hit and resolve the conflicts manually every time. These are - mechanical fixes and easy to do. -2. Add some automation using eslint to fix these merge conflicts. -3. Perform the change on the 1.x codebase such that the soon-to-be-moved types - are imported separately from other types of `@aws-cdk/core`. This will - violate an eslint rule (TODO), but we can disable it for the time being. +To reduce these costs, we propose to modify the `scope` argument on all 1.x to +accept `constructs.Construct` instead of `core.Construct`. -We can start with #1 for a while and see how bad it is. If it is, we can try to -introduce some automation or back-porting. +This will provide the following benefits (enforced by an `awslint` rule): -Something that we can definitely do on 1.x is replace all the variants such as -`core.Construct` and `cdk.Construct` to just `Construct`. This will slightly -help. +1. The `import` statement for `import { Construct } from 'construct'` would + already exist which will reduce merge conflicts. +2. It will unlock composition of framework constructs into other domains (e.g. + it will be possible to pass an AWS CDK L2 to a terraform stack). -## Removal of "synthesize" +Note that we will *not* change the base classes to `constructs.Construct` +because it is technically (and practically) a breaking API change (we must maintain +the invariant that "`s3.Bucket` is a `core.Construct`". -Version 4.x of the `constructs` library does not contain a lifecycle hook for -synthesis. +The remaining change in 2.x will be to update any base classes to use +`constructs.Construct` but this is actually not very prevalent in the framework +code because the majority of constructs actually extend `core.Resource`. -### Motivation +Alternatives considered: +- **Do nothing in 1.x**: will incur an ongoing maintenance cost of the 1.x -> 2.x + merge conflicts. +- **Automatic merge resolution and import organization**: requires research and + development, not necessarily worth it. -The reason this is not available at the base class is because the abstraction -did not "hold water" as the AWS CDK evolved and new CDKs emerged. In the AWS -CDK, we eventually ended up with a centralized synthesis logic at the -`Stage`-level ([TODO: ref]). The main reason was that we needed to "break" the -recursion in various domain-specific points (e.g. stages, nested stacks) which -meant that the generic logic of "traverse the entire tree and call `synthesize`" -did not hold. In `cdk8s`, the support for [TODO: chart dependencies] required -that the name of the output manifest will be determined based on the topologic -order at the app level. Here again, the generic approach failed. +## 02-ASPECTS -In leu of those failures, we decided that there is no additional value in -actually offering a synthesis mechanism at the `constructs` level. Each -CDK-domain implements synthesis at the "right" level. This does not mean that -specific domains can't offer a decentralized approach (i.e. call a method called -"synthesize" on all constructs in the tree), it just means that this is not -provided at the base `Construct` class. +Aspects are actually a form of "prepare" and as such, if they mutate the tree, +their execution order becomes critical and extremely hard to get right. To that +end, we decided to remove them from the `constructs` library as they pose a risk +to the programming model. -### Implications on CDK code base +However, we are aware that aspects are used in by AWS CDK apps and even 3rd +party libraries such as [cdk-watchful](https://github.com/eladb/cdk-watchful). -In the AWS CDK itself, `synthesize()` was used in three locations: +Therefore, we propose to continue to support aspects in 2.x, with the goal of +rethinking this programming model for a future major version. One future +direction is to turn aspects into "reactive" so that they subscribe to tree +events and react in-band during initialization, and not as a separate phase. -1. `Stack` - creates a CFN template and adds itself to the cloud assembly manifest. -2. `AssetStaging` - stages an asset into the cloud assembly. -3. `TreeMetadata` - creates `tree.json` with information about the construct tree. +Since aspects are no longer part of the base programming model, we need a way to +apply aspects to scopes for AWS CDK apps. To do that, we propose to use the +"trait" pattern, which is becoming a common idiom for offering APIs over CDK +scopes: -For `Stack` and `TreeMetadata`, we will convert the generic `synthesize()` -method to `_synthesizeTemplate()` and `_synthesizeTree()` and will call them -from the centralized synthesis function. +```ts +Aspects.of(scope).apply(aspect); +``` -The `AssetStaging` construct does not really need to wait until synthesis in -order to stage the asset. In fact, all the information required already exists -during initialization. The only missing information is the cloud assembly output -directory, and this information is actually known during initialization (we know -this as soon as the CDK app is created). Therefore, the solution for -`AssetStaging` is to move the staging logic to the constructor and use -`Stage.of(this).outdir` to find the output directory of the current stage. +The major downside of this change is discoverability, but +`construct.node.applyAspect` is not necessarily more discoverable. We will make +sure documentation is clear. -### Implications on end-users +We will use this opportunity to normalize the tags API and change it to use the same +pattern: `Tags.of(x).add(name, value)`. -Participation in synthesis is an "advanced" feature of the CDK framework and se -assume most end-users don't use this directly. +### What can we do on 1.x? -If they need "last minute processing", they would likely use `prepare()` (which -is also being [TODO: removed]) but not `synthesize()`. +- We will migrate the 1.x branch to `Aspects.of(x)` and add a deprecation + warning to `this.node.applyAspect()`. +- Introduce `Tags.of(x).add()` and add a deprecation warning to `Tag.add()`. -The use case of emitting arbitrary files into the cloud assembly directory is -weak. The cloud assembly is a well-defined format, and is currently "closed". -There are no extension points that tools can identify. To that end, just writing -files to the cloud assembly output directory does not make tons of sense. Having -said that, it is still possible to do in the same way we plan for -`AssetStaging`. +## 03-DEPENDABLE + +The `constructs` library supports dependencies through `node.addDependency` like +in 1.x, but the API to implement `IDependable` has been slightly changed. + +The `constructs` library also introduces `DependencyGroup` which is a mix +between `CompositeDependable` and `ConcreteDependable`. +### What can we do on 1.x? -### Using stacks as roots for unit tests +It should be possible to migrate the 1.x codebase to use the new APIs without +any breaking changes to users. + +## 04-STACK-ROOT If we move staging of assets to the initialization phase, it means we need to know at that time where is the cloud assembly output directory. As mentioned @@ -475,22 +510,38 @@ to generate names will also change given `uniqueId` is based on the path. Since app-less stacks are only used during tests, this should not have implications on production code, but it does break some of our test suite. -### Should we do this on 1.x? +### What can we do on 1.x? In order to reduce [merge conflicts](#repository-migration-efforts) between 1.x -and 2.x we propose to introduce this change on the 1.x branch prior to forking +and 2.x we considered introducing this change on the 1.x branch prior to forking off 2.x. -However, this could be perceived as a breaking change by end-users since +However, this is technically a breaking (behavioral) change for end-users since `node.path` and `node.uniqueId`, and their derivatives, will change for trees -rooted by a `Stack` and unit tests will need to be updated. +rooted by a `Stack`, and unit tests will need to be updated. -Therefore we propose to introduce this change as a feature flag over the 1.x codebase and enable it in our unit tests (I don't believe we have a way to enable feature flags for all unit tests, but we can devise one). +Therefore we propose to introduce this change as a feature flag over the 1.x +codebase and migrate all of our unit tests (I don't believe we have a way to +enable feature flags for all unit tests, but we can devise one). This will allow us to update our tests in 1.x and avoid the merge conflicts forking on 2.x -## Removal of "prepare" +## 05-METADATA-TRACES + +Since stack traces are not attached to metadata entries by default in constructs +4.x, we will need to pass `stackTrace: true` for `CfnResource`s. This will +preserve the deploy-time stack traces which are very important for users. + +Other metadata entries will not get stack traces by default, and that's a +reasonable behavioral change. + +### What can we do on 1.x? + +No need to introduce over 1.x as the change is very local to `CfnResource` and +therefore can be applies over 2.x without risk. + +## 06-NO-PREPARE The "prepare" hook was removed from constructs since it is a very fragile API. Since the tree can be mutated during prepare, the order of `prepare` invocations @@ -503,52 +554,112 @@ The prepare hook was used in the CDK in a few cases: 3. Calculation of logical IDs based on hashes of other resources (API GW Deployment, Lambda Version). -The first two use cases have already been addressed by centralizing the "prepare" logic at the stage level (into [prepare-app.ts](TODO)). +The first two use cases have already been addressed by centralizing the +"prepare" logic at the stage level (into [prepare-app.ts](TODO)). -The 3rd item can be addressed using `Lazy` tokens (see -[example](https://github.com/aws/aws-cdk/pull/8962/files#diff-51d435d71a31c2607f923fc4d96cac56R140)), -and will be addressed on 1.x prior to the 2.x fork. +### What can we do on 1.x? -## Validation changes +- The 3rd item can be addressed using `Lazy` tokens (see + [example](https://github.com/aws/aws-cdk/pull/8962/files#diff-51d435d71a31c2607f923fc4d96cac56R140)), + and will be addressed on 1.x prior to the 2.x fork. +- We will also add a deprecation warning on 1.x which will identify constructs + that implement "prepare" and refer users to a GitHub issue for details and + consultation. -Since construct validation is quite rare, in constructs 4.x, the `validate()` protected method was removed to clean up the namespace. The alternative is to implement an interface `IValidation` with a similar method. +## 07-NO-SYNTHESIZE -The breaking change is that users who implemented `validate()` would need to -implement `IValidation` and change their `validate()` method from `protected` to -`public`. +Version 4.x of the `constructs` library does not contain a lifecycle hook for +synthesis as described [above](#the-synthesize-hook-is-no-longer-supported). -## Stack trace settings +### Motivation -Since stack traces are not attached to metadata entries by default in constructs -4.x, we will need to pass `stackTrace: true` for `CfnResource`s. This will -preserve the deploy-time stack traces which are very important for users. +The reason this is not available at the base class is because the abstraction +did not "hold water" as the AWS CDK evolved and new CDKs emerged. In the AWS +CDK, we eventually ended up with a centralized synthesis logic at the +`Stage`-level ([TODO: ref]). The main reason was that we needed to "break" the +recursion in various domain-specific points (e.g. stages, nested stacks) which +meant that the generic logic of "traverse the entire tree and call `synthesize`" +did not hold. In `cdk8s`, the support for [TODO: chart dependencies] required +that the name of the output manifest will be determined based on the topologic +order at the app level. Here again, the generic approach failed. -Other metadata entries will not get stack traces by default, and that's a -reasonable change. +In leu of those failures, we decided that there is no additional value in +actually offering a synthesis mechanism at the `constructs` level. Each +CDK-domain implements synthesis at the "right" level. This does not mean that +specific domains can't offer a decentralized approach (i.e. call a method called +"synthesize" on all constructs in the tree), it just means that this is not +provided at the base `Construct` class. -When stack traces are disabled (either through the CDK context or through -`CDK_STACK_TRACE_DISABLE`), we will need to set the appropriate context key in -`App` so that this will propagate to "constructs". +### Implications on CDK code base -## Logging API changes +In the AWS CDK itself, `synthesize()` was used in three locations: -The `construct.node.addInfo()`, `construct.node.addWarning()` and -`construct.node.Error()` methods are now available under the -`Logging.of(construct)` API: +1. `Stack` - creates a CFN template and adds itself to the cloud assembly manifest. +2. `AssetStaging` - stages an asset into the cloud assembly. +3. `TreeMetadata` - creates `tree.json` with information about the construct tree. -Instead of: +For `Stack` and `TreeMetadata`, we will convert the generic `synthesize()` +method to `_synthesizeTemplate()` and `_synthesizeTree()` and will call them +from the centralized synthesis function. -```ts -construct.node.addWarning('my warning'); -``` +The `AssetStaging` construct does not really need to wait until synthesis in +order to stage the asset. In fact, all the information required already exists +during initialization. The only missing information is the cloud assembly output +directory, and this information is actually known during initialization (we know +this as soon as the CDK app is created). Therefore, the solution for +`AssetStaging` is to move the staging logic to the constructor and use +`Stage.of(this).outdir` to find the output directory of the current stage. -Use: +### Implications on end-users -```ts -import { Logging } from '@aws-cdk/core'; +Participation in synthesis is an "advanced" feature of the CDK framework and se +assume most end-users don't use this directly. -Logging.of(construct).addWarning('my warning'); -``` +If they need "last minute processing", they would likely use `prepare()` (which +is also being [TODO: removed]) but not `synthesize()`. + +The use case of emitting arbitrary files into the cloud assembly directory is +weak. The cloud assembly is a well-defined format, and is currently "closed". +There are no extension points that tools can identify. To that end, just writing +files to the cloud assembly output directory does not make tons of sense. Having +said that, it is still possible to do in the same way we plan for +`AssetStaging`. + +### What can we do on 1.x? + +1. The framework changes should be done on the 1.x branch as they are non-breaking. +2. We will also add a deprecation notice that identifies the existence of a + `synthesize()` method on a construct (during synthesis) and warns users that + this hook will no longer be available in 2.x, offering a GitHub issue for + details consultation. + +## 08-VALIDATION + +Since construct validation is quite rare and we want to encourage users to +validate in entry points, in constructs 4.x, the `validate()` protected method +was removed and `node.addValidation()` can be used to add objects that implement +`IValidation`. + +An error will be thrown if a `validate()` method is found on constructs with +instructions on how to implement validation in 2.x. + +### What can we do on 1.x? + +We can introduce this change over the 1.x as long as we continue to support +`validate()` alongside a deprecation warning with instructions on how to migrate +to the new API. + +## 09-LOGGING + +We decided that logging is not generic enough to include in `constructs`. It +emits construct metadata that is very CLI specific (e.g. `aws:cdk:warning`) and +currently there is no strong abstraction. + +To continue to enable logging, we will utilize the `Logging.of(x).addWarning()` pattern. + +### What can we do on 1.x? + +We can introduce this change on 1.x and add a deprecation warning. # Drawbacks @@ -695,3 +806,15 @@ CDK changes: - [ ] Normalize reference to base types (`cdk.Construct` => `Construct`). - [ ] Import https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s/src/dependency.ts to "constructs" - [ ] `constructs` documentation on how to implement centralized synthesis. + +## Base types + +- [ ] 1.x: Convert all `scope` to use `constructs.Construct` via an `awslint` rule. + +## Synthesis + +- [ ] 1.x: Implement synthesis changes +- [ ] 1.x: Add deprecation warning if `synthesize()` is implemented + +## Prepare + From ac95f6785f196cc548dd7b39cf9f4bc80984362a Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 14 Jul 2020 19:09:18 +0300 Subject: [PATCH 08/29] another revision --- text/0192-remove-constructs-compat.md | 210 +++++++++++++++++--------- 1 file changed, 136 insertions(+), 74 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index b6670b7d0..32e66f550 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -40,6 +40,10 @@ This RFC describes the motivation, implications and plan for this project. - [08-VALIDATION: The `validate()` hook is now `node.addValidation()`](#08-validation-the-validate-hook-is-now-nodeaddvalidation) - [09-LOGGING: Logging API changes](#09-logging-logging-api-changes) - [Motivation](#motivation) + - [1. Redundant layer](#1-redundant-layer) + - [2. Multiple `Construct` types](#2-multiple-construct-types) + - [3. Composability with other domains](#3-composability-with-other-domains) + - [4. Non-intuitive dependency requirement](#4-non-intuitive-dependency-requirement) - [Design](#design) - [01-BASE-TYPES](#01-base-types) - [02-ASPECTS](#02-aspects) @@ -60,21 +64,18 @@ This RFC describes the motivation, implications and plan for this project. - [Unresolved questions](#unresolved-questions) - [Future Possibilities](#future-possibilities) - [Implementation Plan](#implementation-plan) - - [Base types](#base-types) - - [Synthesis](#synthesis) - - [Prepare](#prepare) - + - [Preparation of 1.x](#preparation-of-1x) + - [constructs 4.x](#constructs-4x) + - [2.x Work](#2x-work) # Release Notes > This section "works backwards" from the v2.0 release notes in order to > describe the user impact of this change. -**BREAKING CHANGE** - -As part of CDK v2.0, all types related to the *constructs programming model* -have been removed from the AWS CDK and should be used directly from the -[constructs](https://github.com/aws/constructs) library. +**BREAKING CHANGE**: As part of CDK v2.0, all types related to the *constructs +programming model* have been removed from the AWS CDK and should be used +directly from the [constructs](https://github.com/aws/constructs) library. For most CDK libraries and apps, you will likely just need change this: @@ -122,6 +123,8 @@ The following `@aws-cdk/core` types have stand-in replacements in `constructs`: - The `@aws-cdk/core.ConstructOrder` class has been replaced with `constructs.ConstructOrder` - The `@aws-cdk/core.ConstructNode` class has been replaced with `constructs.Node` +See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). + ## 02-ASPECTS: Changes in Aspects API Aspects are not part of the "constructs" library, and therefore instead of @@ -134,6 +137,8 @@ scope, use: Tags.of(scope).add(name, value); ``` +See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). + ## 03-DEPENDABLE: Changes to IDependable implementation If you need to implement `IDependable`: @@ -149,9 +154,11 @@ If you need to implement `IDependable`: The method `construct.node.addDependency(otherConstruct)` __did not change__ and can be used as before. -> TODO: You can use `construct.node.dependencyGraph` to access a rich object +> You can use the new `construct.node.dependencyGraph` to access a rich object > model for reflecting on the node's dependency graph. +See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). + ## 04-STACK-ROOT: Stacks as root constructs It is common in unit tests to use `Stack` as the root of the tree: @@ -174,6 +181,8 @@ of these two). This means that as you migrate to v2.x, some unit tests may need to be updated to reflect this change. +See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/69e221ab5e2fcf564283f337ab8111196608520e). + ## 05-METADATA-TRACES: Stack traces no longer attached to metadata by default For performance reasons, the `construct.node.addMetadata()` method will *not* @@ -184,6 +193,8 @@ stack traces to a metadata entry using the `stackTrace` option: construct.node.addMetadata(key, value, { stackTrace: true }) ``` +See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). + ## 06-NO-PREPARE: The `prepare` hook is no longer supported The **prepare** hook (`construct.onPrepare()` and `construct.prepare()`) is no @@ -193,7 +204,14 @@ when the tree is mutated during this stage. Consider a design where you mutate the tree in-band, or use `Lazy` values or Aspects if appropriate. -[TODO examples from the CDK itself] +See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/3d4fcb5ab72ca0777f3abfa2c4aa10e0d7deba6b). + +Although we recommend that you rethink the use of "prepare", you can use this +idiom to implement "prepare" using aspects: + +```ts +Aspects.of(this).apply({ visit: () => this.prepare() }); +``` The `ConstructNode.prepare(node)` method no longer exists. To realize references & dependencies in a scope call `Stage.of(scope).synth()`. @@ -207,12 +225,14 @@ If your use case for overriding `synthesize()` was to emit files into the cloud assembly directory, you can now find the current cloud assembly output directory during initialization using `Stage.of(this).outdir`. +See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/02b45b50c27fffae1bfbc4f61f6adc1f8fd92672). + The `ConstructNode.synthesize(node)` method no longer exists. However, since now `Stage.of(scope)` is always defined and returns the enclosing stage/app, you can can synthesize a construct node through `Stage.of(scope).synth()`. For additional questions/guidance on how to implement your use case without this -hook, please post a comment on this GitHub issue: [TODO]. +hook, please post a comment on [this GitHub issue](https://github.com/aws/aws-cdk/issues/8909). ## 08-VALIDATION: The `validate()` hook is now `node.addValidation()` @@ -266,6 +286,7 @@ import { Logging } from '@aws-cdk/core'; Logging.of(construct).addWarning('my warning'); ``` +See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). # Motivation @@ -276,13 +297,13 @@ There are various motivations for this change: 3. Inability to compose AWS CDK constructs into other domains 4. Non-intuitive dependency requirement on `constructs` -### 1. Redundant layer +## 1. Redundant layer The current compatibility layer does not have any logic in it. It is pure glue introduced in order to avoid breaking v1.x users. As we release v2.0 we are able to clean up this layer, and with it, improve maintainability and code hygiene. -### 2. Multiple `Construct` types +## 2. Multiple `Construct` types The current situation is error-prone since we have two `Construct` classes in the type closure. For example, when a developer types `"Construct"` and uses @@ -292,7 +313,7 @@ extends this type instead of the `core.Construct` type, it won't be possible to pass an instance of this class as a scope to AWS CDK constructs such as `Stack` for example. -### 3. Composability with other domains +## 3. Composability with other domains The main motivation for this change is to enable composition of AWS CDK constructs with constructs from other domains. @@ -316,7 +337,7 @@ Being able to create construct compositions from multiple domains is a powerful direction for the CDK ecosystem, and this change is required in order to enable these use case. -### 4. Non-intuitive dependency requirement +## 4. Non-intuitive dependency requirement As we transition to [monolithic packaging] as part of v2.x, CDK users will have to take a _peer dependency_ on both the CDK library (`aws-cdk-lib`) and @@ -345,7 +366,7 @@ For each change, we added a **What can we do on 1.x?** section which discusses our strategy for front-loading the change into the 1.x branch to reduce forking costs and/or alert users of the upcoming deprecation. -This design is based on this [proof of concept](https://github.com/aws/aws-cdk/pull/8962). +This design is based on this [proof of concept](https://github.com/aws/aws-cdk/pull/9056). ## 01-BASE-TYPES @@ -367,12 +388,11 @@ accept `constructs.Construct` instead of `core.Construct`. This will provide the following benefits (enforced by an `awslint` rule): -1. The `import` statement for `import { Construct } from 'construct'` would +1. The `import` statement for `import { Construct } from 'constructs'` would already exist which will reduce merge conflicts. 2. It will unlock composition of framework constructs into other domains (e.g. it will be possible to pass an AWS CDK L2 to a terraform stack). - Note that we will *not* change the base classes to `constructs.Construct` because it is technically (and practically) a breaking API change (we must maintain the invariant that "`s3.Bucket` is a `core.Construct`". @@ -382,6 +402,7 @@ The remaining change in 2.x will be to update any base classes to use code because the majority of constructs actually extend `core.Resource`. Alternatives considered: + - **Do nothing in 1.x**: will incur an ongoing maintenance cost of the 1.x -> 2.x merge conflicts. - **Automatic merge resolution and import organization**: requires research and @@ -473,7 +494,7 @@ assert(stack.node.scope instanceof App); // previously it was `undefined` Since only the root construct may have an empty ID, we will also need to assign an ID. We propose to use `"Stack"` since we already have fallback logic that -uses this as the stack name when the stack does not have an ID (see [TODO]). +uses this as the stack name when the stack does not have an ID (see [stack.ts](https://github.com/aws/aws-cdk/blob/8c0142030dce359591aa76fe314f19fce9eddbe6/packages/%40aws-cdk/core/lib/stack.ts#L920)). This change will allow us to remove any special casing we have for stacks in the testing framework and throughout the synthesis code path (we have quite a lot of @@ -549,13 +570,14 @@ becomes critical, and almost impossible to get right without a rich model of relationships between these "prepare" calls. The prepare hook was used in the CDK in a few cases: + 1. Resolution of references across stacks and nested stacks 2. Resolution of dependencies between constructs across stacks 3. Calculation of logical IDs based on hashes of other resources (API GW Deployment, Lambda Version). The first two use cases have already been addressed by centralizing the -"prepare" logic at the stage level (into [prepare-app.ts](TODO)). +"prepare" logic at the stage level (into [prepare-app.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/core/lib/private/prepare-app.ts)). ### What can we do on 1.x? @@ -576,12 +598,15 @@ synthesis as described [above](#the-synthesize-hook-is-no-longer-supported). The reason this is not available at the base class is because the abstraction did not "hold water" as the AWS CDK evolved and new CDKs emerged. In the AWS CDK, we eventually ended up with a centralized synthesis logic at the -`Stage`-level ([TODO: ref]). The main reason was that we needed to "break" the -recursion in various domain-specific points (e.g. stages, nested stacks) which -meant that the generic logic of "traverse the entire tree and call `synthesize`" -did not hold. In `cdk8s`, the support for [TODO: chart dependencies] required -that the name of the output manifest will be determined based on the topologic -order at the app level. Here again, the generic approach failed. +`Stage`-level +([synthesis.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/core/lib/private/synthesis.ts)). +The main reason was that we needed to "break" the recursion in various +domain-specific points (e.g. stages, nested stacks) which meant that the generic +logic of "traverse the entire tree and call `synthesize`" did not hold. In +`cdk8s`, the support for [chart +dependencies](https://github.com/awslabs/cdk8s/blob/ef95b9ffce8a39200e028c2fe8acc55a9915161c/packages/cdk8s/src/app.ts#L39) +required that the name of the output manifest will be determined based on the +topologic order at the app level. Here again, the generic approach failed. In leu of those failures, we decided that there is no additional value in actually offering a synthesis mechanism at the `constructs` level. Each @@ -616,7 +641,8 @@ Participation in synthesis is an "advanced" feature of the CDK framework and se assume most end-users don't use this directly. If they need "last minute processing", they would likely use `prepare()` (which -is also being [TODO: removed]) but not `synthesize()`. +is also being [removed](#06-no-prepare-the-prepare-hook-is-no-longer-supported)) +but not `synthesize()`. The use case of emitting arbitrary files into the cloud assembly directory is weak. The cloud assembly is a well-defined format, and is currently "closed". @@ -768,53 +794,89 @@ See [Release Notes](#release-notes). # Implementation Plan -INTENTIONALLY LEFT BLANK: an implementation plan will be added when the RFC is -scheduled for implementation. - -> The implementation plan should analyze all the tasks needed in order to -> implement this RFC, and the planned order of implementation based on -> dependencies and analysis of the critical path. -> -> Either add the plan here or add link which references a separate document -> and/or a GitHub Project Board (and reference it here) which manages the -> execution of your plan. +## Preparation of 1.x We will try to front load as much of this change to 1.x in order to reduce the merge conflict potential. -constructs 4.x - -- [x] Migrate to projen -- [x] Branch to 4.x and release as @next -- [x] Reintroduce `c.node` instead of `Node.of(c)` -- [x] Removal of `onPrepare` and `onSynthesize` and all synthesis-related code. -- [x] Reintroduce dependencies -- [x] Change `node.dependencies` to return the list of node dependency (non recursive) and add `node.depgraph` which returns a `Graph` object from cdk8s. -- [x] Stack trace control - -CDK changes: - -- [x] cfn2ts -- [ ] Consider aliasing in TypeScript to reduce potential merge conflicts. -- [ ] assets/compat.ts -- [ ] Initial prototype: https://github.com/aws/aws-cdk/pull/8962 -- [ ] Migration guide in https://github.com/aws/aws-cdk/issues/8909 -- [ ] GitHub issue for "synthesize" and "prepare" guidance. -- [ ] Remove the use of "prepare" and "synthesize" in 1.x -- [ ] Implicit `App` for `Stack`s without a scope behind a feature flag and - enable in our unit tests in 1.x -- [ ] Normalize reference to base types (`cdk.Construct` => `Construct`). -- [ ] Import https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s/src/dependency.ts to "constructs" -- [ ] `constructs` documentation on how to implement centralized synthesis. - -## Base types - -- [ ] 1.x: Convert all `scope` to use `constructs.Construct` via an `awslint` rule. - -## Synthesis - -- [ ] 1.x: Implement synthesis changes -- [ ] 1.x: Add deprecation warning if `synthesize()` is implemented - -## Prepare - +To that end, we will continuously merge from "master" into [the POC +branch](https://github.com/aws/aws-cdk/pull/9056) and slowly back port changes +from it into `master` as much as possible. The goal is the minimize the changes +between master and the POC branch which will be merged into the 2.x branch once +created. + +- [01-BASE-TYPES](#01-base-types) + - [ ] Normalize reference to base types (`cdk.Construct` => `Construct`). + - [ ] Use an `awslint` rule to modify the `scope` argument on all 1.x to + accept `constructs.Construct` instead of `core.Construct` +- [02-ASPECTS](#02-aspects) + - [ ] Introduce `Aspects.of(x)` and deprecate `applyAspect` + - [ ] Introduce `Tags.of()` and deprecate `Tag.add()` +- [03-DEPENDABLE](#03-dependable) + - [ ] Introduce `Dependable` as an alias to `DependencyTrait`, introduce + `DependencyGroup` +- [04-STACK-ROOT](#04-stack-root) + - [ ] Implicit `App` for root `Stack` under a feature flag and modify all + [relevant + tests](https://github.com/aws/aws-cdk/pull/9056/commits/69e221ab5e2fcf564283f337ab8111196608520e) + to run behind this flag. +- [05-METADATA-TRACES](#05-metadata-traces) + - N/A +- [06-NO-PREPARE](#06-no-prepare) + - [ ] Back port [this + commit](https://github.com/aws/aws-cdk/pull/9056/commits/3d4fcb5ab72ca0777f3abfa2c4aa10e0d7deba6b) + to master + - [ ] Add a deprecation warning if `onPrepare()` or `prepare()` is identified + on a construct during synthesis +- [07-NO-SYNTHESIZE](#07-no-synthesize) + - [ ] Back port the changes related to synthesis from [this + commit](https://github.com/aws/aws-cdk/pull/9056/commits/02b45b50c27fffae1bfbc4f61f6adc1f8fd92672) + - [ ] Add a deprecation warning if `onSynthezize()` or `synthesize()` is + declared on a construct +- [08-VALIDATION](#08-validation) + - [ ] Introduce `node.addValidation()` and deprecate `validate()` and + `onValidate()` by back porting [this + commit](https://github.com/aws/aws-cdk/pull/9056/commits/42bd929aa36dc97ae39b15b77cf1f69d754c4a92) + to master +- [09-LOGGING](#09-logging) + - [ ] Introduce `Logging.of()` deprecate `node.addWarning/error/info`. + +## constructs 4.x + +[This branch](https://github.com/aws/constructs/pull/133) is the staging branch +for constructs 4.x. + +- [01-BASE-TYPES](#01-base-types) + - [x] Reintroduce `construct.node` instead of `Node.of(construct)` + - [ ] Reintroduce `Construct.isConstruct()`. +- [02-ASPECTS](#02-aspects) + - [x] Remove aspects (`IAspect` and `node.applyAspect`). +- [03-DEPENDABLE](#03-dependable) + - [x] Reintroduce dependencies (`IDependable`, `Dependable`, + `DependencyGroup`) + - [x] Change `node.dependencies` to return the list of node dependency (non + recursive) and add `node.depgraph` which returns a `Graph` object from + cdk8s. + - [x] Change `addDependency` to accept `IDependable` instead of `IConstruct`. + - [x] Return only local dependencies in `node.dependencies` + - [ ] Migrate [DependencyGraph](https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s/src/dependency.ts) from cdk8s into `constructs`. +- [04-STACK-ROOT](#04-stack-root) + - [x] Do not skip root construct when creating `path` and `uniqueId`. +- [05-METADATA-TRACES](#05-metadata-traces) + - [x] Do not emit stack traces in `addMetadata` (`{ stackTrace: true }`). +- [06-NO-PREPARE](#06-no-prepare) + - [x] Removal of `onPrepare` and `node.prepare()` +- [07-NO-SYNTHESIZE](#07-no-synthesize) + - [x] Removal of `onSynthesize` and `node.synthesize()` + - [ ] Expose `lock()` and `unlock()`. +- [08-VALIDATION](#08-validation) + - [ ] Introduce `IValidation`, `addValidation()` and `node.validate()`. +- [09-LOGGING](#09-logging) + - [x] Remove `node.addWarning()`, `node.addError()`, ... + +## 2.x Work + +- [ ] Once the 2.x branch will be created, we will merge the remaining changes + from the POC branch into it. +- [ ] Write a migration guide with guidance on `synthesize` and `prepare` in + From cb98ed9e40a6cf012aa780aced22165b5060122f Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 15 Jul 2020 19:14:09 +0300 Subject: [PATCH 09/29] Update text/0192-remove-constructs-compat.md Co-authored-by: Neta Nir --- text/0192-remove-constructs-compat.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 32e66f550..90030ef88 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -415,7 +415,7 @@ their execution order becomes critical and extremely hard to get right. To that end, we decided to remove them from the `constructs` library as they pose a risk to the programming model. -However, we are aware that aspects are used in by AWS CDK apps and even 3rd +However, we are aware that aspects are used by AWS CDK apps and even 3rd party libraries such as [cdk-watchful](https://github.com/eladb/cdk-watchful). Therefore, we propose to continue to support aspects in 2.x, with the goal of From f6bf6aa9cb5bf526bedee29981b233529b997491 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 16 Jul 2020 09:12:14 +0300 Subject: [PATCH 10/29] change strategy for root stacks using relocate() --- text/0192-remove-constructs-compat.md | 72 +++++++++++++++------------ 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 90030ef88..dcddf4d93 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -172,16 +172,11 @@ const myConstruct = new MyConstruct(stack, 'MyConstruct'); This is still a supported idiom, but in 2.x these root stacks will have an implicit `App` parent. This means that `stack.node.scope` will be an `App` instance, while previously it was `undefined`. The "root" stack will have a -construct ID of `Stack` (unless otherwise specified). +construct ID of `Stack` unless otherwise specified. -This has implications on the value of `construct.node.path` (it will be prefixed -with `Stack/`) and the value of `construct.node.uniqueId` (and any derivatives -of these two). - -This means that as you migrate to v2.x, some unit tests may need to be updated -to reflect this change. - -See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/69e221ab5e2fcf564283f337ab8111196608520e). +> Technically, this change should have resulted in the `node.path` and +> `node.uniqueId` of all constructs defined within such trees to change, but we +> are utilizing `node.relicate()` to maintain the current behavior. ## 05-METADATA-TRACES: Stack traces no longer attached to metadata by default @@ -494,7 +489,8 @@ assert(stack.node.scope instanceof App); // previously it was `undefined` Since only the root construct may have an empty ID, we will also need to assign an ID. We propose to use `"Stack"` since we already have fallback logic that -uses this as the stack name when the stack does not have an ID (see [stack.ts](https://github.com/aws/aws-cdk/blob/8c0142030dce359591aa76fe314f19fce9eddbe6/packages/%40aws-cdk/core/lib/stack.ts#L920)). +uses this as the stack name when the stack does not have an ID (see +[stack.ts](https://github.com/aws/aws-cdk/blob/8c0142030dce359591aa76fe314f19fce9eddbe6/packages/%40aws-cdk/core/lib/stack.ts#L920)). This change will allow us to remove any special casing we have for stacks in the testing framework and throughout the synthesis code path (we have quite a lot of @@ -523,30 +519,41 @@ synthesized output if called twice, we will also need to introduce a `stage.synth({ force: true })` option. This will be the default behavior when using `expect(stack)` or `SynthUtils.synth()`. -The main side effect of this change is that construct paths in unit tests will -now change. In the above example, `foo.node.path` will change from `MyFoo` to -`Stack/MyFoo`. Additionally, tests for resources that utilized `node.uniqueId` -to generate names will also change given `uniqueId` is based on the path. +### Preserving paths and unique IDs using `node.relocate()` -Since app-less stacks are only used during tests, this should not have -implications on production code, but it does break some of our test suite. +A side effect of adding a `App` parent to "root" stacks is that construct paths +and unique IDs in unit tests will now change. In the above example, +`foo.node.path` will change from `MyFoo` to `Stack/MyFoo`. Additionally, tests +for resources that utilized `node.uniqueId` to generate names will also change +given `uniqueId` is based on the path. -### What can we do on 1.x? -In order to reduce [merge conflicts](#repository-migration-efforts) between 1.x -and 2.x we considered introducing this change on the 1.x branch prior to forking -off 2.x. +To address this, we propose to introduce a new capability in `constructs` to +"**relocate**" a scope to a different path. This capability will also be useful +generally when refactoring code (for example, if I want to add a level of +encapsulation to my app without breaking production deployments). + +Using this capability, we will relocate the "root" stack to the `""` path so that +all paths and unique IDs will be preserved. -However, this is technically a breaking (behavioral) change for end-users since -`node.path` and `node.uniqueId`, and their derivatives, will change for trees -rooted by a `Stack`, and unit tests will need to be updated. +### Alternative considered -Therefore we propose to introduce this change as a feature flag over the 1.x -codebase and migrate all of our unit tests (I don't believe we have a way to -enable feature flags for all unit tests, but we can devise one). +We explored the option of fixing all these test expectations throughout the CDK +code base and back port this change over the 1.x behind a feature flag in order +to reduce the potential merge conflicts between 1.x and 2.x. + +The downsides of this approach are: + +1. This is technically a breaking (behavioral) change for end-users since + `node.path` and `node.uniqueId`, and their derivatives, will change for trees + rooted by a `Stack`, and unit tests will need to be updated. +1. We currently don't have a way to implicitly run all our unit tests behind a + feature flag, and it is not a trivial capability to add. + +### What can we do on 1.x? -This will allow us to update our tests in 1.x and avoid the merge conflicts -forking on 2.x +We will add `node.relocate()` to `constructs` 3.x and implement this change over +1.x. ## 05-METADATA-TRACES @@ -816,10 +823,8 @@ created. - [ ] Introduce `Dependable` as an alias to `DependencyTrait`, introduce `DependencyGroup` - [04-STACK-ROOT](#04-stack-root) - - [ ] Implicit `App` for root `Stack` under a feature flag and modify all - [relevant - tests](https://github.com/aws/aws-cdk/pull/9056/commits/69e221ab5e2fcf564283f337ab8111196608520e) - to run behind this flag. + - [ ] Introduce `node.relocate()` in constructs 3.x + - [ ] Implement implicit `App` for root `Stack`s, relocated to "". - [05-METADATA-TRACES](#05-metadata-traces) - N/A - [06-NO-PREPARE](#06-no-prepare) @@ -861,7 +866,8 @@ for constructs 4.x. - [x] Return only local dependencies in `node.dependencies` - [ ] Migrate [DependencyGraph](https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s/src/dependency.ts) from cdk8s into `constructs`. - [04-STACK-ROOT](#04-stack-root) - - [x] Do not skip root construct when creating `path` and `uniqueId`. + - [x] Introduce `node.relocate()` to allow root stacks to be relocated to + `""`. - [05-METADATA-TRACES](#05-metadata-traces) - [x] Do not emit stack traces in `addMetadata` (`{ stackTrace: true }`). - [06-NO-PREPARE](#06-no-prepare) From 99b2a0627bc872eb121b43010c74d0febca34d87 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 20 Jul 2020 17:50:24 +0300 Subject: [PATCH 11/29] another revision --- text/0192-remove-constructs-compat.md | 199 +++++++++++++++++++------- 1 file changed, 149 insertions(+), 50 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index dcddf4d93..d24ebd415 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -30,6 +30,7 @@ This RFC describes the motivation, implications and plan for this project. - [Summary](#summary) - [Table of Contents](#table-of-contents) - [Release Notes](#release-notes) + - [00-DEPENDENCY: Declare a dependency on "constructs"](#00-dependency-declare-a-dependency-on-constructs) - [01-BASE-TYPES: Removal of base types](#01-base-types-removal-of-base-types) - [02-ASPECTS: Changes in Aspects API](#02-aspects-changes-in-aspects-api) - [03-DEPENDABLE: Changes to IDependable implementation](#03-dependable-changes-to-idependable-implementation) @@ -45,6 +46,7 @@ This RFC describes the motivation, implications and plan for this project. - [3. Composability with other domains](#3-composability-with-other-domains) - [4. Non-intuitive dependency requirement](#4-non-intuitive-dependency-requirement) - [Design](#design) + - [00-DEPENDENCY](#00-dependency) - [01-BASE-TYPES](#01-base-types) - [02-ASPECTS](#02-aspects) - [03-DEPENDABLE](#03-dependable) @@ -86,7 +88,7 @@ import { Construct } from '@aws-cdk/core'; With this: ```ts -import { Cosntruct } from 'constructs'; +import { Construct } from 'constructs'; ``` The following table summarizes the API changes between 1.x and 2.x: @@ -100,20 +102,61 @@ The following table summarizes the API changes between 1.x and 2.x: `@aws-cdk/core.ConstructOrder` | `constructs.ConstructOrder` `@aws-cdk/core.ConstructNode` | `constructs.Node` `Construct.isConstruct(x)` | `x instanceof Construct` -`construct.node.applyAspect(aspect)` | `Aspects.of(construct).apply(aspect)` +`construct.node.applyAspect(aspect)` | `Aspects.of(construct).add(aspect)` `@aws-cdk/core.IDependable` | `constructs.IDependable` `@aws-cdk/core.DependencyTrait` | `constructs.Dependable` `@aws-cdk.core.DependencyTrait.get(x)` | `constructs.Dependable.of(x)` `construct.node.dependencies` | Is now non-transitive `construct.addMetadata()` | Stack trace not attached by default -`ConstructNode.prepare(node)`, `onPrepare()`, `prepare()` | Not supported -`ConstructNode.synthesize(node)`, `onSynthesize()`, `synthesize()` | Not supported +`ConstructNode.prepareTree()`, `node.prepare()`, `onPrepare()`, `prepare()` | Not supported, use aspects instead +`ConstructNode.synthesizeTree()`, `node.synthesize()`, `onSynthesize()`, `synthesize()` | Not supported `construct.onValidate()`, `construct.validate()` hooks | Implement `constructs.IValidation` and call `node.addValidation()` `ConstructNode.validate(node)` | `construct.node.validate()` The following sections describe all the related breaking changes and details migration strategies for each change. +## 00-DEPENDENCY: Declare a dependency on "constructs" + +As part of migrating your code to AWS CDK 2.0, you will need to declare a +dependency on the `constructs` library (in addition to the `aws-cdk-lib` library +which now includes the entire AWS CDK). + +For libraries, this should be a peer dependency, similarly to your dependency on +the AWS CDK. You will likely also want to declare those as `devDependencies` in +order to be able to run tests in your build environment. + +To increase interoperability of your library, the recommendation is to use the +lowest possible __caret__ version: + +```json +{ + "peerDependencies": { + "aws-cdk-lib": "^2.0.0", + "constructs": "^10.0.0" + }, + "devDependencies": { + "aws-cdk-lib": "^2.0.0", + "constructs": "^10.0.0" + } +} +``` + +For apps, you should declare these as direct dependencies, and you would +normally want to use the highest version available: + +```json +{ + "dependencies": { + "aws-cdk-lib": "^2.44.0", + "constructs": "^10.787.0" + } +} +``` + +NOTE: Due to it's foundational nature, the `constructs` library is committed to +never introduce breaking changes. Therefore, it's version will be `10.x`. + ## 01-BASE-TYPES: Removal of base types The following `@aws-cdk/core` types have stand-in replacements in `constructs`: @@ -128,7 +171,7 @@ See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b ## 02-ASPECTS: Changes in Aspects API Aspects are not part of the "constructs" library, and therefore instead of -`construct.node.applyAspect(aspect)` use `Aspects.of(construct).apply(aspect)`. +`construct.node.applyAspect(aspect)` use `Aspects.of(construct).add(aspect)`. The `Tag.add(scope, name, value)` API has been removed. To apply AWS tags to a scope, use: @@ -172,17 +215,22 @@ const myConstruct = new MyConstruct(stack, 'MyConstruct'); This is still a supported idiom, but in 2.x these root stacks will have an implicit `App` parent. This means that `stack.node.scope` will be an `App` instance, while previously it was `undefined`. The "root" stack will have a -construct ID of `Stack` unless otherwise specified. +construct ID of `Default` unless otherwise specified. -> Technically, this change should have resulted in the `node.path` and -> `node.uniqueId` of all constructs defined within such trees to change, but we -> are utilizing `node.relicate()` to maintain the current behavior. +Please note that this also means that the value of `node.path` for all +constructs in the tree would now have a `Default/` prefix (if it was `Foo/Bar` +it will now be `Default/Foo/Bar`). + +> In contrast, the value of `node.uniqueId` will _not_ change because `Default` +> is a special ID that is ignored when calculating unique IDs (this feature +> already exists in 1.x). ## 05-METADATA-TRACES: Stack traces no longer attached to metadata by default For performance reasons, the `construct.node.addMetadata()` method will *not* -attach stack traces to metadata entries. You can explicitly request to attach -stack traces to a metadata entry using the `stackTrace` option: +attach stack traces to metadata entries. Stack traces will still be associated +with all `CfnResource` constructs and can also be added to custom metadata using +the `stackTrace` option: ```ts construct.node.addMetadata(key, value, { stackTrace: true }) @@ -205,7 +253,7 @@ Although we recommend that you rethink the use of "prepare", you can use this idiom to implement "prepare" using aspects: ```ts -Aspects.of(this).apply({ visit: () => this.prepare() }); +Aspects.of(this).add({ visit: () => this.prepare() }); ``` The `ConstructNode.prepare(node)` method no longer exists. To realize references @@ -316,19 +364,29 @@ constructs with constructs from other domains. It is currently impossible to define AWS CDK constructs within a non-AWS-CDK construct scope. -For example, consider the -[Terrastack](https://github.com/TerraStackIO/terrastack) project or a similar -one, which uses CDK constructs to define stacks through Terraform. Say we want to -use an AWS CDK L2 inside a Terraform stack construct: +For example, consider the [CDK for +Terraform](https://github.com/hashicorp/terraform-cdk) or a similar project, +which uses constructs to define stacks through Terraform. + +We are currently working with HashiCorp to enable the following use case in the +Terraform CDK: ```ts -const stack = new terrastack.Stack(...); +import * as cdktf from 'cdktf'; // <=== Terraform CDK +import * as s3 from '@aws-cdk/aws-s3'; // <=== AWS CDK -// COMPILATION ERROR: `this` is not a `cdk.Construct`. +const stack = new cdktf.TerraformStack(...); + +// COMPILATION ERROR: `this` is of type `constructs.Construct` and not a `@aws-cdk/core.Construct`. new s3.Bucket(this, 'my-bucket'); ``` -Being able to create construct compositions from multiple domains is a powerful +In order to enable this usage, we will need `s3.Bucket` to accept any object +that implements `constructs.Construct` as its `scope`. At the moment, this will +fail compilation with the error above, because the `scope` in `s3.Bucket` is +`core.Construct`. + +Being able to create compositions from multiple CDK domains is a powerful direction for the CDK ecosystem, and this change is required in order to enable these use case. @@ -341,12 +399,17 @@ to take a _peer dependency_ on both the CDK library (`aws-cdk-lib`) and > Currently, the AWS CDK also takes `constructs` as a normal dependency (similar > to all dependencies), but this is about to change with mono-cdk. -The reason `constructs` will also be required (whether we leave the -compatibility layer or not) is due to the fact that all CDK constructs -eventually extend the base `constructs.Construct` class. This means that this -type is part of their public API and therefore a peer dependency is required -(otherwise, there could be incompatible copies of `Construct` in the node -runtime). +The reason `constructs` will have to be defined as a peer-dependency of the AWS +CDK, whether we leave the compatibility layer or not, is due to the fact that +all AWS CDK constructs eventually extend the base `constructs.Construct` class. +This means that this type is part of their public API, and therefore must be +defined as a peer dependency (otherwise, there could be incompatible copies of +`Construct` in the dependency closure). + +The removal of the compatibility layer means that now anyone who uses the AWS +CDK will need to explicitly use the `constructs.Construct` type (even for +trivial apps), and therefore it would "make sense" for them to take a dependency +on the `constructs` library. See the RFC for [monolithic packaging] for more details. @@ -363,6 +426,25 @@ costs and/or alert users of the upcoming deprecation. This design is based on this [proof of concept](https://github.com/aws/aws-cdk/pull/9056). +## 00-DEPENDENCY + +In order to enable composability with other CDK domains (Terraform, Kubernetes), +all constructs must use the same version of the `Construct` base class. + +As long as all libraries in a closure take a __peer dependency__ on a compatible +version of `constructs`, the npm package manger will include a single copy of +the library, and therefore all constructs will derive from the same `Construct` +(and more importantly, accept the same `Construct` for `scope`). + +Practically this means that we can never introduce a major version of +`constructs` because any major version will require a new major version of all +CDKs, and that is impossible to require or coordinate given the decentralized +nature of the ecosystem. + +We propose to take a commitment to __never introduce breaking changes in +"constructs"__. This implies that we will never introduce another major version. +To symbolize that to users, we will use the major version `10.x`. + ## 01-BASE-TYPES Once `construct-compat.ts` is removed from `@aws-cdk/core`, all CDK code (both @@ -424,7 +506,7 @@ apply aspects to scopes for AWS CDK apps. To do that, we propose to use the scopes: ```ts -Aspects.of(scope).apply(aspect); +Aspects.of(scope).add(aspect); ``` The major downside of this change is discoverability, but @@ -443,7 +525,7 @@ pattern: `Tags.of(x).add(name, value)`. ## 03-DEPENDABLE The `constructs` library supports dependencies through `node.addDependency` like -in 1.x, but the API to implement `IDependable` has been slightly changed. +in 1.x, but the API to implement `IDependable` has been changed. The `constructs` library also introduces `DependencyGroup` which is a mix between `CompositeDependable` and `ConcreteDependable`. @@ -519,25 +601,28 @@ synthesized output if called twice, we will also need to introduce a `stage.synth({ force: true })` option. This will be the default behavior when using `expect(stack)` or `SynthUtils.synth()`. -### Preserving paths and unique IDs using `node.relocate()` +### Preserving unique IDs using an ID of `Default` -A side effect of adding a `App` parent to "root" stacks is that construct paths -and unique IDs in unit tests will now change. In the above example, -`foo.node.path` will change from `MyFoo` to `Stack/MyFoo`. Additionally, tests -for resources that utilized `node.uniqueId` to generate names will also change -given `uniqueId` is based on the path. +A side effect of adding a `App` parent to "root" stacks is that we now have an +additional parent scope for all constructs in the tree. The location of the +construct in the tree is taken into account when calculating `node.path` and +`node.uniqueId`. +Since `uniqueId` is used in several places throughout the AWS Construct Library +to allocate names for resources, and we have multiple unit tests that expect +these values, we will use the ID `Default` for the root stack. -To address this, we propose to introduce a new capability in `constructs` to -"**relocate**" a scope to a different path. This capability will also be useful -generally when refactoring code (for example, if I want to add a level of -encapsulation to my app without breaking production deployments). +The `uniqueId` algorithm in the constructs library (see [reference]()) ignores +any node with the ID `Default` for the purpose of calculating the unique ID, +which allows us to perform this change without breaking unique IDs. -Using this capability, we will relocate the "root" stack to the `""` path so that -all paths and unique IDs will be preserved. +We will accept the fact that `node.path` is going to change for this specific +use case (only relevant in tests). ### Alternative considered +#### Alternative 1: Breaking unique IDs + We explored the option of fixing all these test expectations throughout the CDK code base and back port this change over the 1.x behind a feature flag in order to reduce the potential merge conflicts between 1.x and 2.x. @@ -550,10 +635,21 @@ The downsides of this approach are: 1. We currently don't have a way to implicitly run all our unit tests behind a feature flag, and it is not a trivial capability to add. +#### Alternative 2: `node.relocate()` + +We also explored the option of introducing an additional capability to +constructs called `node.relocate(newPath)` which allows modifying the path of a +scope such that all child scopes will automatically be "relocated" to a new +path. This would have allowed avoiding the breakage in `node.path` but would +have also introduced several other idiosyncrasies and potential violations of +invariants such as the fact that a path is unique within the tree. + ### What can we do on 1.x? -We will add `node.relocate()` to `constructs` 3.x and implement this change over -1.x. +We will introduce this change over the 1.x branch as-is, acknowledging that we +are technically breaking the behavior of `node.path` in unit tests which use +`Stack` as the root. Since we are not breaking `uniqueId`, we expect this to be +tolerable over the 1.x branch. ## 05-METADATA-TRACES @@ -615,7 +711,7 @@ dependencies](https://github.com/awslabs/cdk8s/blob/ef95b9ffce8a39200e028c2fe8ac required that the name of the output manifest will be determined based on the topologic order at the app level. Here again, the generic approach failed. -In leu of those failures, we decided that there is no additional value in +In lieu of those failures, we decided that there is no additional value in actually offering a synthesis mechanism at the `constructs` level. Each CDK-domain implements synthesis at the "right" level. This does not mean that specific domains can't offer a decentralized approach (i.e. call a method called @@ -644,19 +740,20 @@ this as soon as the CDK app is created). Therefore, the solution for ### Implications on end-users -Participation in synthesis is an "advanced" feature of the CDK framework and se +Participation in synthesis is an "advanced" feature of the CDK framework and we assume most end-users don't use this directly. -If they need "last minute processing", they would likely use `prepare()` (which -is also being [removed](#06-no-prepare-the-prepare-hook-is-no-longer-supported)) -but not `synthesize()`. +If they need "last minute processing", they can add an aspect to the node which +will be applied before synthesis (the alternative to "prepare"). The use case of emitting arbitrary files into the cloud assembly directory is weak. The cloud assembly is a well-defined format, and is currently "closed". -There are no extension points that tools can identify. To that end, just writing -files to the cloud assembly output directory does not make tons of sense. Having -said that, it is still possible to do in the same way we plan for -`AssetStaging`. +There are no extension points that tools can identify. + +To that end, just writing files to the cloud assembly output directory does not +make tons of sense. Yet, if there is still a use case for writing files during +initialization, it is possible to find out the output directory through +`Stage.of(scope).outdir`. This is how asset staging will be implemented. ### What can we do on 1.x? @@ -886,3 +983,5 @@ for constructs 4.x. from the POC branch into it. - [ ] Write a migration guide with guidance on `synthesize` and `prepare` in +- [ ] Updates to Developer Guide (add 2.x section) +- [ ] Updates to READMEs across the library (add 2.x section) From bae1619b219e61704e9764408d554ac666880b1f Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 20 Jul 2020 17:56:12 +0300 Subject: [PATCH 12/29] add some tasks related to constructs --- text/0192-remove-constructs-compat.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index d24ebd415..b3a8a8ac9 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -67,7 +67,7 @@ This RFC describes the motivation, implications and plan for this project. - [Future Possibilities](#future-possibilities) - [Implementation Plan](#implementation-plan) - [Preparation of 1.x](#preparation-of-1x) - - [constructs 4.x](#constructs-4x) + - [constructs 10.x](#constructs-10x) - [2.x Work](#2x-work) # Release Notes @@ -943,11 +943,13 @@ created. - [09-LOGGING](#09-logging) - [ ] Introduce `Logging.of()` deprecate `node.addWarning/error/info`. -## constructs 4.x +## constructs 10.x [This branch](https://github.com/aws/constructs/pull/133) is the staging branch -for constructs 4.x. +for constructs 10.x. +- [00-DEPENDENCY](#00-dependency) + - [ ] Document API compatibility assurance and the 10.x version number. - [01-BASE-TYPES](#01-base-types) - [x] Reintroduce `construct.node` instead of `Node.of(construct)` - [ ] Reintroduce `Construct.isConstruct()`. From e031e1c9b89d35df128d5c532a4b7f00b054ad09 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Thu, 9 Jul 2020 17:23:19 +0100 Subject: [PATCH 13/29] RFC: 171 CloudFront Redesign (#174) * RFC: 0171 CloudFront Redesign - initial rough ideas * Iteration #2 of the API, sans the Lambda@Edge functionality. Still in draft state, but hopefully getting closer. * Lambda@Edge API v1, plus some of the open questions and other sections filled in. * Added detailed design section, including before/after examples * Proper namespacing in the examples, and updated the open questions * Minor update to force pull request refresh * Minor changes per feedback; removing the 'draft' tag * Distribution.for* syntactic sugar * Removing Distribution.behavior, keeping origin->behavior nesting consistent * Minor API updates per feedback and lessons learned from implementation * Added implementation note for capturing behavior ordering --- text/0171-cloudfront-redesign.md | 495 +++++++++++++++++++++++++++++++ 1 file changed, 495 insertions(+) create mode 100644 text/0171-cloudfront-redesign.md diff --git a/text/0171-cloudfront-redesign.md b/text/0171-cloudfront-redesign.md new file mode 100644 index 000000000..a5692ccd2 --- /dev/null +++ b/text/0171-cloudfront-redesign.md @@ -0,0 +1,495 @@ +--- +feature name: CloudFront redesign +start date: 2020-06-15 +rfc pr: +related issue: https://github.com/aws/aws-cdk-rfcs/issues/171 +--- + +# Summary + +Proposal to redesign the @aws-cdk/aws-cloudfront module. + +The current module does not adhere to the best practice naming conventions or ease-of-use patterns +that are present in the other CDK modules. A redesign of the API will allow for friendly, easier +access to common patterns and usages. + +This RFC does not attempt to lay out the entire API; rather, it focuses on a complete re-write of +the module README with a focus on the most common use cases and how they work with the new design. +More detailed designs and incremental API improvements will be tracked as part of GitHub Project Board +once the RFC is approved. + +--- + +# README + +Amazon CloudFront is a web service that speeds up distribution of your static and dynamic web content, such as .html, .css, .js, and image files, to +your users. CloudFront delivers your content through a worldwide network of data centers called edge locations. When a user requests content that +you're serving with CloudFront, the user is routed to the edge location that provides the lowest latency, so that content is delivered with the best +possible performance. + +## Creating a distribution + +CloudFront distributions deliver your content from one or more origins; an origin is the location where you store the original version of your +content. Origins can be created from S3 buckets or a custom origin (HTTP server). + +### From an S3 Bucket + +An S3 bucket can be added as an origin. If the bucket is configured as a website endpoint, the distribution can use S3 redirects and S3 custom error +documents. + +```ts +import * as cloudfront from '@aws-cdk/aws-cloudfront'; + +// Creates a distribution for a S3 bucket. +const myBucket = new s3.Bucket(...); +new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromBucket(myBucket), +}); + +// Equivalent to the above +const myBucket = new s3.Bucket(...); +cloudfront.Distribution.forBucket(this, 'myDist', myBucket); + +// Creates a distribution for a S3 bucket that has been configured for website hosting. +const myWebsiteBucket = new s3.Bucket(...); +new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromWebsiteBucket(myBucket), +}); + +// Equivalent to the above +const myBucket = new s3.Bucket(...); +cloudfront.Distribution.forWebsiteBucket(this, 'myDist', myBucket); +``` + +Both of the S3 Origin options will automatically create an origin access identity and grant it access to the underlying bucket. This can be used in +conjunction with a bucket that is not public to require that your users access your content using CloudFront URLs and not S3 URLs directly. + +### From an HTTP endpoint + +Origins can also be created from other resources (e.g., load balancers, API gateways), or from any accessible HTTP server. + +```ts +// Creates a distribution for an application load balancer. +const myLoadBalancer = new elbv2.ApplicationLoadBalancer(...); +new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromLoadBalancerV2(myLoadBalancer), +}); + +// Creates a distribution for an HTTP server. +new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromHttpServer({ + domainName: 'www.example.com', + protocolPolicy: OriginProtocolPolicy.HTTPS_ONLY, + }) +}); +``` + +## Domain Names and Certificates + +When you create a distribution, CloudFront returns a domain name for the distribution, for example: `d111111abcdef8.cloudfront.net`. If you want to +use your own domain name, such as `www.example.com`, you can add an alternate domain name to your distribution. + +```ts +new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromBucket(myBucket), + aliases: ['www.example.com'], +}) +``` + +CloudFront distributions use a default certificate (`*.cloudfront.net`) to support HTTPS by default. If you want to support HTTPS with your own domain +name, you must associate a certificate with your distribution that contains your domain name. The certificate must be present in the AWS Certificate +Manager (ACM) service in the US East (N. Virginia) region; the certificate may either be created by ACM, or created elsewhere and imported into ACM. + +```ts +const myCertificate = new certmgr.DnsValidatedCertificate(this, 'mySiteCert', { + domainName: 'www.example.com', + hostedZone, +}); +new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromBucket(myBucket), + certificate: myCertificate, +}); +``` + +Note that in the above example the aliases are inferred from the certificate and do not need to be explicitly provided. + +## Behaviors + +Each distribution has a default behavior which applies to all requests to that distribution; additional behaviors may be specified for a +given URL path pattern. Behaviors allow routing with multiple origins, controlling which HTTP methods to support, whether to require users to +use HTTPS, and what query strings or cookies to forward to your origin, among others. + +The properties of the default behavior can be adjusted as part of the distribution creation. The following example shows configuring the HTTP +methods and viewer protocol policy of the cache. + +```ts +const myWebDistribution = new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromLoadBalancerV2(myLoadBalancer, { + allowedMethods: AllowedMethods.ALL, + viewerProtocolPolicy: ViewerProtocolPolicy.REDIRECT_TO_HTTPS, + }), +}); +``` + +Additional cache behaviors can be specified at creation, or added to the origin(s) after the initial creation. These additional cache behaviors enable +customization for a specific set of resources based on a URL path pattern. For example, we can add a behavior to `myWebDistribution` to override the +default time-to-live (TTL) for all of the images. + +```ts +myWebDistribution.origin.addBehavior('/images/*.jpg', { + viewerProtocolPolicy: ViewerProtocolPolicy.REDIRECT_TO_HTTPS, + defaultTtl: cdk.Duration.days(7), +}); +``` + +## Multiple Origins + +A distribution may have multiple origins in addition to the default origin; each additional origin must have (at least) one behavior to route requests +to that origin. A common pattern might be to serve all static assets from an S3 bucket, but all dynamic content served from a web server. The +following example shows how such a setup might be created: + +```ts +const myWebsiteBucket = new s3.Bucket(...); +const myMultiOriginDistribution = new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromWebsiteBucket(myBucket), + additionalOrigins: [ + cloudfront.Origin.fromLoadBalancerV2(myLoadBalancer, { + pathPattern: '/api/*', + allowedMethods: AllowedMethods.ALL, + forwardQueryString: true, + }), + ], +}); +``` + +## Origin Groups + +You can specify an origin group for your CloudFront origin if, for example, you want to configure origin failover for scenarios when you need high +availability. Use origin failover to designate a primary origin for CloudFront plus a second origin that CloudFront automatically switches to when the +primary origin returns specific HTTP status code failure responses. An origin group can be created and specified as the primary (or additional) origin +for the distribution. + +```ts +const myOriginGroup = cloudfront.Origin.fromOriginGroup({ + primaryOrigin: cloudfront.Origin.fromLoadBalancerV2(myLoadBalancer), + fallbackOrigin: cloudfront.Origin.fromBucket(myBucket), + fallbackStatusCodes: [500, 503], +}); +new cloudfront.Distribution(this, 'myDist', { origin: myOriginGroup }); +``` + +The above will create both origins and a single origin group with the load balancer origin falling back to the S3 bucket in case of 500 or 503 errors. + +## Lambda@Edge + +Lambda@Edge is an extension of AWS Lambda, a compute service that lets you execute functions that customize the content that CloudFront delivers. You +can author Node.js or Python functions in the US East (N. Virginia) region, and then execute them in AWS locations globally that are closer to the +viewer, without provisioning or managing servers. Lambda@Edge functions are associated with a specific behavior and event type. Lambda@Edge can be +used rewrite URLs, alter responses based on headers or cookies, or authorize requests based on headers or authorization tokens. + +The following shows a Lambda@Edge function added to the default behavior and triggered on every request. + +```ts +const myFunc = new lambda.Function(...); +new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromBucket(myBucket, { + edgeFunctions: [{ + functionVersion: myFunc.currentVersion, + eventType: EventType.VIEWER_REQUEST, + }], + }), +}); +``` + +Lambda@Edge functions can also be associated with additional behaviors, either at behavior creation (associated with the origin) or after behavior +creation. + +```ts +// Assigning at behavior creation. +myOrigin.addBehavior('/images/*.jpg', { + viewerProtocolPolicy: ViewerProtocolPolicy.REDIRECT_TO_HTTPS, + defaultTtl: cdk.Duration.days(7), + edgeFunctions: [{ + functionVersion: myFunc.currentVersion, + eventType: EventType.VIEWER_REQUEST, + }] +}); + +// Assigning after creation. +const myImagesBehavior = myOrigin.addBehavior('/images/*.jpg', ...); +myImagesBehavior.addEdgeFunction({ + functionVersion: myFunc.currentVersion, + eventType: EventType.VIEWER_REQUEST, +}); +``` + +--- + +# Motivation + +The existing aws-cloudfront module doesn't adhere to standard naming convention, lacks convenience methods for more easily interacting with +distributions, origins, and behaviors, and has been in an "experimental" state for years. This proposal aims to bring a friendlier, more ergonomic +interface to the module, and advance the module to a GA-ready state. + +# Design Summary + +The approach will create a new top-level Construct (`Distribution`) to replace the existing `CloudFrontWebDistribution`, as well as new constructs to +represent the other logical resources for a distribution (i.e., `Origin`, `Behavior`). The new construct is optimized for the most common use cases of +creating a distribution with a single origin and behavior. The new L2s will be created in the same aws-cloudfront module and no changes will be made +to the existing L2s to preserve the existing experience. Unlike the existing L2, the new L2s will feature a variety of convenience methods (e.g., +`addBehavior`) to aid in the creation of the distribution, and provide several out-of-the-box defaults for building distributions off of other +resources (e.g., buckets, load balanced services). + +# Detailed Design + +The design creates one new resource (`Distribution`) and two new classes (`Origin` and `Behavior`) to replace the current +`CloudFrontWebDistribution` construct. Each of these new classes comes with helper methods (e.g., `addBehavior`) to make assembling more complex +distributions easier, as well as factory constructors to make it easier to build common Origin and Behavior patterns (e.g., `Origin.fromBucket`). + +The following is an incomplete, but representative, listing of the API: + +```ts +class Distribution extends BaseDistribution { + static fromDistributionAttributes(scope: Construct, id: string, attributes: DistributionAttributes): IDistribution; + + static forBucket(scope: Construct, id: string, bucket: IBucket): Distribution; + static forWebsiteBucket(scope: Construct, id: string, bucket: IBucket): Distribution; + + constructor(scope: Construct, id: string, props: DistributionProps) {} + + addOrigin(options: OriginOptions): Origin +} + +class Origin { + static fromBucket(bucket: s3.IBucket, behaviorOptions?: BehaviorProps): Origin + static fromWebsiteBucket(bucket: s3.IBucket, behaviorOptions?: BehaviorProps): Origin + static fromLoadBalancerV2(loadBalancer: elbv2.ApplicationLoadBalancer, behaviorOptions?: BehaviorProps): Origin + static fromHttpServer(options: ServerOriginOptions, behaviorOptions?: BehaviorProps): Origin + static fromOriginGroup(options: OriginGroupOptions): Origin + + constructor(props: OriginProps) {} + + addBehavior(pathPattern: string, options: BehaviorOptions): Behavior +} + +class Behavior { + constructor(props: BehaviorProps) {} + + addEdgeFunction(options: EdgeFunctionOptions): EdgeFunction +} +``` + +## Nested structure and relationships + +The `Distribution` has one top-level origin and behavior, which aligns to how the vast majority of customers use CloudFront today (based on public +CDK examples). Customers can add additional origins (and origin groups) and behaviors, which are modeled as `additionalOrigins` and +`additionalBehaviors`, respectively. Each origin may have multiple behaviors associated with it, so the relationship is modeled such that behaviors +are added to origins. However, an authoritative list of behaviors is kept on the distribution to preserve ordering. This is done similarly to how +ECS clusters keep track of Fargate profiles as they are created and associated with the cluster. In the below example, the '/api/\*' behavior for +the load balancer origin will be ordered first, then the '/api/errors/\*' behavior on the bucket. + +```ts +const myWebsiteBucket = new s3.Bucket(...); +const myMultiOriginDistribution = new cloudfront.Distribution(this, 'myDist', { + origin: cloudfront.Origin.fromWebsiteBucket(this, 'myOrigin', myBucket), + additionalOrigins: [ + cloudfront.Origin.fromLoadBalancerV2(this, 'myOrigin', myLoadBalancer, { + pathPattern: '/api/*', + allowedMethods: AllowedMethods.ALL, + forwardQueryString: true, + }), + ]; +}); +myMultiOriginDistribution.origin.addBehavior('/api/errors/*', ...); +``` + +This approach was chosen as the simplest pattern to work with for the majority of customers that don't add multiple origins and behaviors, while +still giving those power users control over behavior ordering, albeit implicitly. See the Rationale and Alternatives section for a discussion of +other ways this was considered. + +**Implementation Note:** The relationships as-is are modeled with one-way connections; Distributions know about Origins, but Origins have no +references to Distributions, for example. This design makes the initial creation much simpler for users, but makes having a per-Distribution +ordered list of Behaviors impossible. To correct this, the Origin will need some form of reference to the Distribution, either at creation or +when being associated with the Distribution. The current (inelegant) proposal is for the Origin to expose a method (`_attachDistribution`) which +is called by the Distribution to create the relationship. Feedback on this approach (or more elegent proposals) are welcome. + +## Interaction with other L2s + +The existing @aws-cdk/aws-cloudfront module is used in three other modules of the CDK: (1) aws-route53-patterns, (2) aws-route53-targets, and +(3) aws-s3-deployment. + +1. In aws-route53-patterns, the CloudFrontWebDistribution is used internally to the HttpsRedirect class; no CloudFront +properties or classes are exposed to the consumer. This usage can be swapped out when the new L2s are ready without impact to customers. +2. In aws-route53-targets, the CloudFrontTarget constructor takes a CloudFrontWebDistribution as the sole parameter. This usage could actually +be replaced with an IDistribution, and then work for both Distribution and CloudFrontWebDistribution. I _believe_ this is a backwards-compatible +change; please correct me if I'm wrong. +3. In aws-s3-deployment, BucketDeploymentProps has an optional IDistribution member, which does not need to be changed. + +## Comparison with the existing API + +The primary drawback of this work is that an existing CloudFront L2 already exists and is in wide use. To justify the creation of a new API, this +section provides examples of what the user experience of common (and some uncommon) use cases will be before and after the redesign. + +### Use Case #1 - S3 bucket origin + +The simplest use case is to have a single S3 bucket origin, and no customized behaviors. + +**Before:** + +```ts +new CloudFrontWebDistribution(this, 'MyDistribution', { + originConfigs: [{ + s3OriginSource: { s3BucketSource: sourceBucket }, + behaviors : [ { isDefaultBehavior: true }], + }] +}); +``` + +**After:** + +```ts +new cloudfront.Distribution(this, 'MyDistribution', { + origin: cloudfront.Origin.fromBucket(sourceBucket), +}); +``` + +### Use Case #2 - S3 bucket origin with certificate and custom behavior + +This example keeps the same bucket, but adds one custom behavior and a certificate. + +**Before:** + +```ts +new CloudFrontWebDistribution(this, 'MyDistribution', { + originConfigs: [{ + s3OriginSource: { s3BucketSource: sourceBucket }, + behaviors : [ + { isDefaultBehavior: true }, + { + pathPattern: 'images/*', + defaultTtl: cdk.Duration.days(7), + } + ] + }], + viewerCertificate: ViewerCertificate.fromAcmCertificate(myCertificate), +}); +``` + +**After:** + +```ts +const dist = new cloudfront.Distribution(this, 'MyDistribution', { + origin: cloudfront.Origin.fromBucket(sourceBucket), + certificate: myCertificate, +}); +dist.origin.addBehavior('images/*', { defaultTtl: cdk.Duration.days(7) }); +``` + +### Use Case #3 - Multi-origin, multi-behavior distribution with a Lambda@Edge function + +Both S3 and LoadBalancedFargateService origins, custom behaviors, and a Lambda function to top it all off. + +**Before:** + +```ts +new CloudFrontWebDistribution(this, 'dist', { + originConfigs: [ + { + customOriginSource: { + domainName: lbFargateService.loadBalancer.loadBalancerDnsName, + protocolPolicy: OriginProtocolPolicy.HTTPS_ONLY, + }, + behaviors: [{ + isDefaultBehavior: true, + allowedMethods: CloudFrontAllowedMethods.ALL, + forwardedValues: { queryString: true }, + lambdaFunctionAssociations: [{ + lambdaFunction: myFunctionVersion, + eventType: LambdaEdgeEventType.ORIGIN_RESPONSE, + }] + }] + }, + { + s3OriginSource: { + s3BucketSource: sourceBucket, + originAccessIdentity: OriginAccessIdentity.fromOriginAccessIdentityName(this, 'oai', 'distOAI'), + }, + behaviors: [ { pathPattern: 'static/*' } ], + }, + ], +}); +``` + +**After:** + +```ts +const dist = new cloudfront.Distribution(this, 'MyDistribution', { + origin: cloudfront.Origin.fromLoadBalancerV2(lbFargateService.loadBalancer, { + allowedMethods: AllowedMethods.ALL, + forwardQueryString: true, + edgeFunctions: [{ + function: myFunctionVersion, + eventType: EventType.ORIGIN_RESPONSE, + }], + }), +}); +dist.addOrigin(cloudfront.Origin.fromBucket(sourceBucket), { pathPattern: 'static/*' }); +``` + +# Drawbacks + +The primary drawback to this work is one of adoption. The aws-cloudfront module is one of the oldest in the CDK, and is used by many customers. +These changes won't break any of the existing customers, but all existing customers would need to rewrite their CloudFront constructs to take +advantage of the functionality and ease-of-use benefits of the new L2s. For building an entirely new set of L2s to be worth the return on investment, +the improvements to functionality and ergonomics needs to be substantial. Feedback is welcome on how to best capitalize on this opportunity and make +the friendliest interface possible. + +# Rationale and Alternatives + +This RFC aims to take one of the older modules in the CDK and update it to the current set of design standards. By introducing secondary resource +creation methods, factories for common L2-based origins, and flattening some of the top-level nested properties, we can offer an easier-to-use +experience. + +The interface aims to make it easiest for the 90%+ use cases of having a single origin based on an S3 bucket (or other CDK construct), with a single +default behavior, optionally slightly customized; it accomplishes this by exposing a single top-level `origin` and `behavior` and making us of +factory methods to construct origins from buckets, load balancers, and other common origins. Beyond that straightforward use-case, the decision was +made to represent the behaviors as members of origins, as each behavior must be associated with an origin. This introduces one area of cognitive +complexity in terms of behavior ordering. + +Behaviors are ordered and precedence is used to determine how to route requests to origin(s). For example, origin #1 could have custom behaviors with +path patterns of 'images/\*.jpg' and '\*.gif', and origin #2 could have a behavior on 'images/\*'. Depending on the ordering, a request for +'images/foo.gif' may either be routed to origin #1 or #2. The approach taken ties behavior precedence to order of creation. An alternative would +be to expose a flat list of behaviors, and allow the user to manipulate that list to change precedence. Ultimately, this was discarded as an overly- +complex interface with diminishing benefits. However, feedback is welcome on a more elegant way to give users control of behavior ordering. + +# Adoption Strategy + +Once created, the new L2s can be used by existing CDK developers for new use cases, or by converting their existing CloudFrontWebDistribution usages +to the new cloudfront.Distribution resource. + +# Unresolved questions + +1. What level of breaking changes are acceptable for the existing `IDistribution` and `CloudFrontWebDistribution` resources? Notably, the +`IDistribution` interface should extend `IResource`, and `CloudFrontWebDistribution` changed from extending `Construct` to `Resource`. Is this +worth it, given the breaking changes to existing consumers? The alternative is to leave both `IDistribution` and `CloudFrontWebDistribution` as-is, +and have `Distribution` directly extend `Resource`. +2. Related to the above, should the current (CloudFrontWebDistribution) construct be marked as "stable" to indicate we won't be making future +updates to it? Any other suggestions on how we message the "v2" on the README to highlight the new option to customers? +3. Any better patterns for associating the Origin with the Distribution than something like the proposed `_attachDistribution`? + +# Future Possibilities + +One extension point for this redesign is building all-in-one "L3s" on top of the new L2s. One extremely common use case for CloudFront is +to be used as a globally-distributed front on top of a S3 bucket configured for website hosting. One potential L3 for CloudFront would wrap +this entire set of constructs into one easy-to-use construct. For example: + +```ts +// Creates the hosted zone, S3 bucket, CloudFront distribution, ACM certificate, and wires everything together. +new StaticWebsiteDistribution(this, 'SiteDistribution', { + domainName: 'www.example.com', + source: s3.Source.asset('static-website/'), +}); +``` + +This would be relatively easy to piece together from the existing constructs, and would follow the patterns of the aws-s3-deployment module +to deploy the assets. From 3f794e0c397b2ca4f27e039ceedb256ff8a4c7b2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 9 Jul 2020 16:49:14 +0000 Subject: [PATCH 14/29] Update RFC table in README (#191) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9bd7110c5..483d8e5b9 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ future state of the libraries and to discover projects for contribution. [18](https://github.com/aws/aws-cdk-rfcs/issues/18)|[Open context provider framework](https://github.com/aws/aws-cdk-rfcs/pull/167)|[@ddneilson](https://github.com/ddneilson)|✍️ review [25](https://github.com/aws/aws-cdk-rfcs/issues/25)|[Defaults & configuration policy](https://github.com/aws/aws-cdk-rfcs/issues/25)|[@njlynch](https://github.com/njlynch)|✍️ review [79](https://github.com/aws/aws-cdk-rfcs/issues/79)|[CDK v2.0](https://github.com/aws/aws-cdk-rfcs/pull/156)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review -[171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/issues/171)|[@njlynch](https://github.com/njlynch)|✍️ review +[171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0171-cloudfront-redesign.md)|[@njlynch](https://github.com/njlynch)|✍️ review [175](https://github.com/aws/aws-cdk-rfcs/issues/175)|[AppSync Mapping Template Object Model](https://github.com/aws/aws-cdk-rfcs/pull/177)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✍️ review [1](https://github.com/aws/aws-cdk-rfcs/issues/1)|[CDK Watch](https://github.com/aws/aws-cdk-rfcs/issues/1)||💡 proposed [2](https://github.com/aws/aws-cdk-rfcs/issues/2)|[Migration Paths](https://github.com/aws/aws-cdk-rfcs/issues/2)||💡 proposed @@ -72,7 +72,6 @@ future state of the libraries and to discover projects for contribution. [82](https://github.com/aws/aws-cdk-rfcs/issues/82)|[Loosely Coupled Stacks through SSM Parameters](https://github.com/aws/aws-cdk-rfcs/issues/82)||💡 proposed [83](https://github.com/aws/aws-cdk-rfcs/issues/83)|[Global Name Prefix](https://github.com/aws/aws-cdk-rfcs/issues/83)||💡 proposed [86](https://github.com/aws/aws-cdk-rfcs/issues/86)|[AWS Account Alias Resource](https://github.com/aws/aws-cdk-rfcs/issues/86)||💡 proposed -[109](https://github.com/aws/aws-cdk-rfcs/issues/109)|[Elasticache L2 Constructs](https://github.com/aws/aws-cdk-rfcs/issues/109)||💡 proposed [116](https://github.com/aws/aws-cdk-rfcs/issues/116)|[Easier identification of experimental modules](https://github.com/aws/aws-cdk-rfcs/issues/116)|[@rix0rrr](https://github.com/rix0rrr)|💡 proposed [118](https://github.com/aws/aws-cdk-rfcs/issues/118)|[New CDK Major Version](https://github.com/aws/aws-cdk-rfcs/issues/118)||💡 proposed [127](https://github.com/aws/aws-cdk-rfcs/issues/127)|[CDK to directly reference/import/update an existing stack](https://github.com/aws/aws-cdk-rfcs/issues/127)||💡 proposed @@ -80,6 +79,8 @@ future state of the libraries and to discover projects for contribution. [159](https://github.com/aws/aws-cdk-rfcs/issues/159)|[Cross-App Resource Sharing](https://github.com/aws/aws-cdk-rfcs/issues/159)||💡 proposed [161](https://github.com/aws/aws-cdk-rfcs/issues/161)|[Cross-Region/Account References](https://github.com/aws/aws-cdk-rfcs/issues/161)||💡 proposed [180](https://github.com/aws/aws-cdk-rfcs/issues/180)|[CustomResources: Allow usage across accounts](https://github.com/aws/aws-cdk-rfcs/issues/180)||💡 proposed +[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/issues/192)||💡 proposed +[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Removal of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|💡 proposed [16](https://github.com/aws/aws-cdk-rfcs/issues/16)|[RFC Process](https://github.com/aws/aws-cdk-rfcs/pull/53)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done [36](https://github.com/aws/aws-cdk-rfcs/issues/36)|[Constructs Programming Model](https://github.com/aws/aws-cdk-rfcs/issues/36)|[@eladb](https://github.com/eladb)|✅ done [37](https://github.com/aws/aws-cdk-rfcs/issues/37)|[Release from a "release" branch](https://github.com/aws/aws-cdk-rfcs/issues/37)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done From 25d46e08774194f31b901332e369929f6bcbdf9c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 9 Jul 2020 21:39:57 +0300 Subject: [PATCH 15/29] Update RFC table in README (#196) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 483d8e5b9..30b2274e1 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,8 @@ future state of the libraries and to discover projects for contribution. [79](https://github.com/aws/aws-cdk-rfcs/issues/79)|[CDK v2.0](https://github.com/aws/aws-cdk-rfcs/pull/156)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review [171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0171-cloudfront-redesign.md)|[@njlynch](https://github.com/njlynch)|✍️ review [175](https://github.com/aws/aws-cdk-rfcs/issues/175)|[AppSync Mapping Template Object Model](https://github.com/aws/aws-cdk-rfcs/pull/177)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✍️ review +[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)||✍️ review +[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Removal of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review [1](https://github.com/aws/aws-cdk-rfcs/issues/1)|[CDK Watch](https://github.com/aws/aws-cdk-rfcs/issues/1)||💡 proposed [2](https://github.com/aws/aws-cdk-rfcs/issues/2)|[Migration Paths](https://github.com/aws/aws-cdk-rfcs/issues/2)||💡 proposed [3](https://github.com/aws/aws-cdk-rfcs/issues/3)|[Integrate CLI into Native Modules](https://github.com/aws/aws-cdk-rfcs/issues/3)||💡 proposed @@ -79,8 +81,6 @@ future state of the libraries and to discover projects for contribution. [159](https://github.com/aws/aws-cdk-rfcs/issues/159)|[Cross-App Resource Sharing](https://github.com/aws/aws-cdk-rfcs/issues/159)||💡 proposed [161](https://github.com/aws/aws-cdk-rfcs/issues/161)|[Cross-Region/Account References](https://github.com/aws/aws-cdk-rfcs/issues/161)||💡 proposed [180](https://github.com/aws/aws-cdk-rfcs/issues/180)|[CustomResources: Allow usage across accounts](https://github.com/aws/aws-cdk-rfcs/issues/180)||💡 proposed -[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/issues/192)||💡 proposed -[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Removal of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|💡 proposed [16](https://github.com/aws/aws-cdk-rfcs/issues/16)|[RFC Process](https://github.com/aws/aws-cdk-rfcs/pull/53)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done [36](https://github.com/aws/aws-cdk-rfcs/issues/36)|[Constructs Programming Model](https://github.com/aws/aws-cdk-rfcs/issues/36)|[@eladb](https://github.com/eladb)|✅ done [37](https://github.com/aws/aws-cdk-rfcs/issues/37)|[Release from a "release" branch](https://github.com/aws/aws-cdk-rfcs/issues/37)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done From daea35f702f7e206bacabad3b3332984a9c44f5e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 12 Jul 2020 05:45:28 +0000 Subject: [PATCH 16/29] Update RFC table in README (#198) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 30b2274e1..0365d05e1 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,7 @@ future state of the libraries and to discover projects for contribution. [159](https://github.com/aws/aws-cdk-rfcs/issues/159)|[Cross-App Resource Sharing](https://github.com/aws/aws-cdk-rfcs/issues/159)||💡 proposed [161](https://github.com/aws/aws-cdk-rfcs/issues/161)|[Cross-Region/Account References](https://github.com/aws/aws-cdk-rfcs/issues/161)||💡 proposed [180](https://github.com/aws/aws-cdk-rfcs/issues/180)|[CustomResources: Allow usage across accounts](https://github.com/aws/aws-cdk-rfcs/issues/180)||💡 proposed +[197](https://github.com/aws/aws-cdk-rfcs/issues/197)|[Generating CloudFormation resources from CloudFormation Registry schemas](https://github.com/aws/aws-cdk-rfcs/issues/197)|[@RomainMuller](https://github.com/RomainMuller)|💡 proposed [16](https://github.com/aws/aws-cdk-rfcs/issues/16)|[RFC Process](https://github.com/aws/aws-cdk-rfcs/pull/53)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done [36](https://github.com/aws/aws-cdk-rfcs/issues/36)|[Constructs Programming Model](https://github.com/aws/aws-cdk-rfcs/issues/36)|[@eladb](https://github.com/eladb)|✅ done [37](https://github.com/aws/aws-cdk-rfcs/issues/37)|[Release from a "release" branch](https://github.com/aws/aws-cdk-rfcs/issues/37)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done From 15645b463451d7e33558176fa2547a8760b96d46 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 13 Jul 2020 22:05:02 +0300 Subject: [PATCH 17/29] Update RFC table in README (#200) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0365d05e1..80036bcd5 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ future state of the libraries and to discover projects for contribution. [79](https://github.com/aws/aws-cdk-rfcs/issues/79)|[CDK v2.0](https://github.com/aws/aws-cdk-rfcs/pull/156)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review [171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0171-cloudfront-redesign.md)|[@njlynch](https://github.com/njlynch)|✍️ review [175](https://github.com/aws/aws-cdk-rfcs/issues/175)|[AppSync Mapping Template Object Model](https://github.com/aws/aws-cdk-rfcs/pull/177)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✍️ review -[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)||✍️ review +[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)|[@eladb](https://github.com/eladb)|✍️ review [193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Removal of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review [1](https://github.com/aws/aws-cdk-rfcs/issues/1)|[CDK Watch](https://github.com/aws/aws-cdk-rfcs/issues/1)||💡 proposed [2](https://github.com/aws/aws-cdk-rfcs/issues/2)|[Migration Paths](https://github.com/aws/aws-cdk-rfcs/issues/2)||💡 proposed From 090d8bcf777a525c768e1b219813e9b7501e34dc Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 16 Jul 2020 14:01:33 +0000 Subject: [PATCH 18/29] Update RFC table in README (#202) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 80036bcd5..7d021de24 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ future state of the libraries and to discover projects for contribution. [161](https://github.com/aws/aws-cdk-rfcs/issues/161)|[Cross-Region/Account References](https://github.com/aws/aws-cdk-rfcs/issues/161)||💡 proposed [180](https://github.com/aws/aws-cdk-rfcs/issues/180)|[CustomResources: Allow usage across accounts](https://github.com/aws/aws-cdk-rfcs/issues/180)||💡 proposed [197](https://github.com/aws/aws-cdk-rfcs/issues/197)|[Generating CloudFormation resources from CloudFormation Registry schemas](https://github.com/aws/aws-cdk-rfcs/issues/197)|[@RomainMuller](https://github.com/RomainMuller)|💡 proposed +[201](https://github.com/aws/aws-cdk-rfcs/issues/201)|[Construct scope relocation](https://github.com/aws/aws-cdk-rfcs/issues/201)||💡 proposed [16](https://github.com/aws/aws-cdk-rfcs/issues/16)|[RFC Process](https://github.com/aws/aws-cdk-rfcs/pull/53)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done [36](https://github.com/aws/aws-cdk-rfcs/issues/36)|[Constructs Programming Model](https://github.com/aws/aws-cdk-rfcs/issues/36)|[@eladb](https://github.com/eladb)|✅ done [37](https://github.com/aws/aws-cdk-rfcs/issues/37)|[Release from a "release" branch](https://github.com/aws/aws-cdk-rfcs/issues/37)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done From fb413add072fd57d1e351c97d58825ec9b2c6112 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2020 15:28:53 +0000 Subject: [PATCH 19/29] Update RFC table in README (#203) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7d021de24..1cb797767 100644 --- a/README.md +++ b/README.md @@ -18,13 +18,13 @@ future state of the libraries and to discover projects for contribution. [6](https://github.com/aws/aws-cdk-rfcs/issues/6)|[Monolithic Packaging](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0006-monolothic-packaging.md)|[@RomainMuller](https://github.com/RomainMuller)|👷 implementing [49](https://github.com/aws/aws-cdk-rfcs/issues/49)|[CI/CD for CDK apps](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0049-continuous-delivery.md)|[@rix0rrr](https://github.com/rix0rrr)|👷 implementing [95](https://github.com/aws/aws-cdk-rfcs/issues/95)|[Cognito Construct Library](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0095-cognito-construct-library)|[@nija-at](https://github.com/nija-at)|👷 implementing +[171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0171-cloudfront-redesign.md)|[@njlynch](https://github.com/njlynch)|👷 implementing [18](https://github.com/aws/aws-cdk-rfcs/issues/18)|[Open context provider framework](https://github.com/aws/aws-cdk-rfcs/pull/167)|[@ddneilson](https://github.com/ddneilson)|✍️ review [25](https://github.com/aws/aws-cdk-rfcs/issues/25)|[Defaults & configuration policy](https://github.com/aws/aws-cdk-rfcs/issues/25)|[@njlynch](https://github.com/njlynch)|✍️ review [79](https://github.com/aws/aws-cdk-rfcs/issues/79)|[CDK v2.0](https://github.com/aws/aws-cdk-rfcs/pull/156)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review -[171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0171-cloudfront-redesign.md)|[@njlynch](https://github.com/njlynch)|✍️ review [175](https://github.com/aws/aws-cdk-rfcs/issues/175)|[AppSync Mapping Template Object Model](https://github.com/aws/aws-cdk-rfcs/pull/177)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✍️ review [192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)|[@eladb](https://github.com/eladb)|✍️ review -[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Removal of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review +[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Fixing of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review [1](https://github.com/aws/aws-cdk-rfcs/issues/1)|[CDK Watch](https://github.com/aws/aws-cdk-rfcs/issues/1)||💡 proposed [2](https://github.com/aws/aws-cdk-rfcs/issues/2)|[Migration Paths](https://github.com/aws/aws-cdk-rfcs/issues/2)||💡 proposed [3](https://github.com/aws/aws-cdk-rfcs/issues/3)|[Integrate CLI into Native Modules](https://github.com/aws/aws-cdk-rfcs/issues/3)||💡 proposed From 21ce330d1b88a3f5e8b8e9a101e9bc85081b0251 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 19 Jul 2020 14:26:04 +0300 Subject: [PATCH 20/29] Update RFC table in README (#205) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1cb797767..0e023238f 100644 --- a/README.md +++ b/README.md @@ -81,8 +81,8 @@ future state of the libraries and to discover projects for contribution. [159](https://github.com/aws/aws-cdk-rfcs/issues/159)|[Cross-App Resource Sharing](https://github.com/aws/aws-cdk-rfcs/issues/159)||💡 proposed [161](https://github.com/aws/aws-cdk-rfcs/issues/161)|[Cross-Region/Account References](https://github.com/aws/aws-cdk-rfcs/issues/161)||💡 proposed [180](https://github.com/aws/aws-cdk-rfcs/issues/180)|[CustomResources: Allow usage across accounts](https://github.com/aws/aws-cdk-rfcs/issues/180)||💡 proposed -[197](https://github.com/aws/aws-cdk-rfcs/issues/197)|[Generating CloudFormation resources from CloudFormation Registry schemas](https://github.com/aws/aws-cdk-rfcs/issues/197)|[@RomainMuller](https://github.com/RomainMuller)|💡 proposed [201](https://github.com/aws/aws-cdk-rfcs/issues/201)|[Construct scope relocation](https://github.com/aws/aws-cdk-rfcs/issues/201)||💡 proposed +[204](https://github.com/aws/aws-cdk-rfcs/issues/204)|[Go language bindings](https://github.com/aws/aws-cdk-rfcs/issues/204)||💡 proposed [16](https://github.com/aws/aws-cdk-rfcs/issues/16)|[RFC Process](https://github.com/aws/aws-cdk-rfcs/pull/53)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done [36](https://github.com/aws/aws-cdk-rfcs/issues/36)|[Constructs Programming Model](https://github.com/aws/aws-cdk-rfcs/issues/36)|[@eladb](https://github.com/eladb)|✅ done [37](https://github.com/aws/aws-cdk-rfcs/issues/37)|[Release from a "release" branch](https://github.com/aws/aws-cdk-rfcs/issues/37)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✅ done From fc4973f92826c20c20201dccfede9c1b8f1ee857 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 20 Jul 2020 08:47:24 +0300 Subject: [PATCH 21/29] Update RFC table in README (#207) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0e023238f..1a557e3eb 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ future state of the libraries and to discover projects for contribution. [74](https://github.com/aws/aws-cdk-rfcs/issues/74)|[Common API for Resources with Web Addresses](https://github.com/aws/aws-cdk-rfcs/issues/74)||💡 proposed [77](https://github.com/aws/aws-cdk-rfcs/issues/77)|[On-demand import of CloudFormation Resources from Registry](https://github.com/aws/aws-cdk-rfcs/issues/77)||💡 proposed [78](https://github.com/aws/aws-cdk-rfcs/issues/78)|[Feature proposal: Workspaces](https://github.com/aws/aws-cdk-rfcs/issues/78)||💡 proposed -[82](https://github.com/aws/aws-cdk-rfcs/issues/82)|[Loosely Coupled Stacks through SSM Parameters](https://github.com/aws/aws-cdk-rfcs/issues/82)||💡 proposed +[82](https://github.com/aws/aws-cdk-rfcs/issues/82)|[Weak references](https://github.com/aws/aws-cdk-rfcs/issues/82)||💡 proposed [83](https://github.com/aws/aws-cdk-rfcs/issues/83)|[Global Name Prefix](https://github.com/aws/aws-cdk-rfcs/issues/83)||💡 proposed [86](https://github.com/aws/aws-cdk-rfcs/issues/86)|[AWS Account Alias Resource](https://github.com/aws/aws-cdk-rfcs/issues/86)||💡 proposed [116](https://github.com/aws/aws-cdk-rfcs/issues/116)|[Easier identification of experimental modules](https://github.com/aws/aws-cdk-rfcs/issues/116)|[@rix0rrr](https://github.com/rix0rrr)|💡 proposed From 1283383511660dc695aee6bf7ddfb18bfd932e8d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 20 Jul 2020 16:33:37 +0000 Subject: [PATCH 22/29] Update RFC table in README (#208) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1a557e3eb..40498106b 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,6 @@ future state of the libraries and to discover projects for contribution. [79](https://github.com/aws/aws-cdk-rfcs/issues/79)|[CDK v2.0](https://github.com/aws/aws-cdk-rfcs/pull/156)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review [175](https://github.com/aws/aws-cdk-rfcs/issues/175)|[AppSync Mapping Template Object Model](https://github.com/aws/aws-cdk-rfcs/pull/177)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✍️ review [192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)|[@eladb](https://github.com/eladb)|✍️ review -[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Fixing of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review [1](https://github.com/aws/aws-cdk-rfcs/issues/1)|[CDK Watch](https://github.com/aws/aws-cdk-rfcs/issues/1)||💡 proposed [2](https://github.com/aws/aws-cdk-rfcs/issues/2)|[Migration Paths](https://github.com/aws/aws-cdk-rfcs/issues/2)||💡 proposed [3](https://github.com/aws/aws-cdk-rfcs/issues/3)|[Integrate CLI into Native Modules](https://github.com/aws/aws-cdk-rfcs/issues/3)||💡 proposed @@ -96,6 +95,7 @@ future state of the libraries and to discover projects for contribution. [85](https://github.com/aws/aws-cdk-rfcs/issues/85)|[Cross Account resource import and source account tracking](https://github.com/aws/aws-cdk-rfcs/issues/85)|[@skinny85](https://github.com/skinny85)|❓unknown [158](https://github.com/aws/aws-cdk-rfcs/issues/158)|[Experiment with Resource Provider in TypeScript](https://github.com/aws/aws-cdk-rfcs/pull/170)||❓unknown [162](https://github.com/aws/aws-cdk-rfcs/issues/162)|[Simplify working with logical IDs when refactoring CDK code](https://github.com/aws/aws-cdk-rfcs/issues/162)||❓unknown +[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Fixing of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|❓unknown ## What is an RFC? From 9ad430bd74126f1b1a9e9e628ea59d3f2f8dcbd9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 20 Jul 2020 19:51:04 +0300 Subject: [PATCH 23/29] Update RFC table in README (#209) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 40498106b..02424970a 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ future state of the libraries and to discover projects for contribution. [49](https://github.com/aws/aws-cdk-rfcs/issues/49)|[CI/CD for CDK apps](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0049-continuous-delivery.md)|[@rix0rrr](https://github.com/rix0rrr)|👷 implementing [95](https://github.com/aws/aws-cdk-rfcs/issues/95)|[Cognito Construct Library](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0095-cognito-construct-library)|[@nija-at](https://github.com/nija-at)|👷 implementing [171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0171-cloudfront-redesign.md)|[@njlynch](https://github.com/njlynch)|👷 implementing +[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Fixing of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|⏰ final comments [18](https://github.com/aws/aws-cdk-rfcs/issues/18)|[Open context provider framework](https://github.com/aws/aws-cdk-rfcs/pull/167)|[@ddneilson](https://github.com/ddneilson)|✍️ review [25](https://github.com/aws/aws-cdk-rfcs/issues/25)|[Defaults & configuration policy](https://github.com/aws/aws-cdk-rfcs/issues/25)|[@njlynch](https://github.com/njlynch)|✍️ review [79](https://github.com/aws/aws-cdk-rfcs/issues/79)|[CDK v2.0](https://github.com/aws/aws-cdk-rfcs/pull/156)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review From db5631e44f9f6e67b8459aaaddbf745f279c9936 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 20 Jul 2020 17:41:32 +0000 Subject: [PATCH 24/29] Update RFC table in README (#210) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 02424970a..67455bc70 100644 --- a/README.md +++ b/README.md @@ -19,12 +19,12 @@ future state of the libraries and to discover projects for contribution. [49](https://github.com/aws/aws-cdk-rfcs/issues/49)|[CI/CD for CDK apps](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0049-continuous-delivery.md)|[@rix0rrr](https://github.com/rix0rrr)|👷 implementing [95](https://github.com/aws/aws-cdk-rfcs/issues/95)|[Cognito Construct Library](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0095-cognito-construct-library)|[@nija-at](https://github.com/nija-at)|👷 implementing [171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0171-cloudfront-redesign.md)|[@njlynch](https://github.com/njlynch)|👷 implementing +[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)|[@eladb](https://github.com/eladb)|⏰ final comments [193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Fixing of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|⏰ final comments [18](https://github.com/aws/aws-cdk-rfcs/issues/18)|[Open context provider framework](https://github.com/aws/aws-cdk-rfcs/pull/167)|[@ddneilson](https://github.com/ddneilson)|✍️ review [25](https://github.com/aws/aws-cdk-rfcs/issues/25)|[Defaults & configuration policy](https://github.com/aws/aws-cdk-rfcs/issues/25)|[@njlynch](https://github.com/njlynch)|✍️ review [79](https://github.com/aws/aws-cdk-rfcs/issues/79)|[CDK v2.0](https://github.com/aws/aws-cdk-rfcs/pull/156)|[@RomainMuller](https://github.com/RomainMuller)|✍️ review [175](https://github.com/aws/aws-cdk-rfcs/issues/175)|[AppSync Mapping Template Object Model](https://github.com/aws/aws-cdk-rfcs/pull/177)|[@MrArnoldPalmer](https://github.com/MrArnoldPalmer)|✍️ review -[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)|[@eladb](https://github.com/eladb)|✍️ review [1](https://github.com/aws/aws-cdk-rfcs/issues/1)|[CDK Watch](https://github.com/aws/aws-cdk-rfcs/issues/1)||💡 proposed [2](https://github.com/aws/aws-cdk-rfcs/issues/2)|[Migration Paths](https://github.com/aws/aws-cdk-rfcs/issues/2)||💡 proposed [3](https://github.com/aws/aws-cdk-rfcs/issues/3)|[Integrate CLI into Native Modules](https://github.com/aws/aws-cdk-rfcs/issues/3)||💡 proposed @@ -96,7 +96,6 @@ future state of the libraries and to discover projects for contribution. [85](https://github.com/aws/aws-cdk-rfcs/issues/85)|[Cross Account resource import and source account tracking](https://github.com/aws/aws-cdk-rfcs/issues/85)|[@skinny85](https://github.com/skinny85)|❓unknown [158](https://github.com/aws/aws-cdk-rfcs/issues/158)|[Experiment with Resource Provider in TypeScript](https://github.com/aws/aws-cdk-rfcs/pull/170)||❓unknown [162](https://github.com/aws/aws-cdk-rfcs/issues/162)|[Simplify working with logical IDs when refactoring CDK code](https://github.com/aws/aws-cdk-rfcs/issues/162)||❓unknown -[193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Fixing of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|❓unknown ## What is an RFC? From c1aaaf5b02a4b9c97a4b423415621c89ef9cdf64 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2020 10:18:47 +0300 Subject: [PATCH 25/29] Update RFC table in README (#211) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 67455bc70..7fad80e69 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ future state of the libraries and to discover projects for contribution. [49](https://github.com/aws/aws-cdk-rfcs/issues/49)|[CI/CD for CDK apps](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0049-continuous-delivery.md)|[@rix0rrr](https://github.com/rix0rrr)|👷 implementing [95](https://github.com/aws/aws-cdk-rfcs/issues/95)|[Cognito Construct Library](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0095-cognito-construct-library)|[@nija-at](https://github.com/nija-at)|👷 implementing [171](https://github.com/aws/aws-cdk-rfcs/issues/171)|[CloudFront Module Redesign](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0171-cloudfront-redesign.md)|[@njlynch](https://github.com/njlynch)|👷 implementing -[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)|[@eladb](https://github.com/eladb)|⏰ final comments +[192](https://github.com/aws/aws-cdk-rfcs/issues/192)|[Removal of the "constructs" compatibility layer (v2.0)](https://github.com/aws/aws-cdk-rfcs/pull/195)|[@eladb](https://github.com/eladb)|👍 approved [193](https://github.com/aws/aws-cdk-rfcs/issues/193)|[Fixing of type unions](https://github.com/aws/aws-cdk-rfcs/issues/193)|[@RomainMuller](https://github.com/RomainMuller)|⏰ final comments [18](https://github.com/aws/aws-cdk-rfcs/issues/18)|[Open context provider framework](https://github.com/aws/aws-cdk-rfcs/pull/167)|[@ddneilson](https://github.com/ddneilson)|✍️ review [25](https://github.com/aws/aws-cdk-rfcs/issues/25)|[Defaults & configuration policy](https://github.com/aws/aws-cdk-rfcs/issues/25)|[@njlynch](https://github.com/njlynch)|✍️ review From edbe17c16cb4810fa8aaf078f791a8041f4b6250 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 27 Jul 2020 08:40:58 +0300 Subject: [PATCH 26/29] Rename "node" to "construct" in constructs 10.x --- text/0192-remove-constructs-compat.md | 156 +++++++++++++++++++++----- 1 file changed, 131 insertions(+), 25 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index b3a8a8ac9..516f96be8 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -32,6 +32,7 @@ This RFC describes the motivation, implications and plan for this project. - [Release Notes](#release-notes) - [00-DEPENDENCY: Declare a dependency on "constructs"](#00-dependency-declare-a-dependency-on-constructs) - [01-BASE-TYPES: Removal of base types](#01-base-types-removal-of-base-types) + - [10-CONSTRUCT-NODE: `myConstruct.node` is now `myConstrct.construct`](#10-construct-node-myconstructnode-is-now-myconstrctconstruct) - [02-ASPECTS: Changes in Aspects API](#02-aspects-changes-in-aspects-api) - [03-DEPENDABLE: Changes to IDependable implementation](#03-dependable-changes-to-idependable-implementation) - [04-STACK-ROOT: Stacks as root constructs](#04-stack-root-stacks-as-root-constructs) @@ -48,6 +49,7 @@ This RFC describes the motivation, implications and plan for this project. - [Design](#design) - [00-DEPENDENCY](#00-dependency) - [01-BASE-TYPES](#01-base-types) + - [10-CONSTRUCT-NODE](#10-construct-node) - [02-ASPECTS](#02-aspects) - [03-DEPENDABLE](#03-dependable) - [04-STACK-ROOT](#04-stack-root) @@ -91,7 +93,18 @@ With this: import { Construct } from 'constructs'; ``` -The following table summarizes the API changes between 1.x and 2.x: +Additionally, The `node` property in `Construct` is now called `construct`. This +means, for example, order to find the `path` of a construct `foo`, use: + +```ts +foo.construct.path // instead of `foo.node.path` +``` + +--- + +The following table summarizes the API changes between 1.x and 2.x. The +following sections describe all the related breaking changes and details +migration strategies for each change. 1.x|2.x ------|----- @@ -101,20 +114,18 @@ The following table summarizes the API changes between 1.x and 2.x: `@aws-cdk/core.IConstruct` | `constructs.IConstruct` `@aws-cdk/core.ConstructOrder` | `constructs.ConstructOrder` `@aws-cdk/core.ConstructNode` | `constructs.Node` -`Construct.isConstruct(x)` | `x instanceof Construct` -`construct.node.applyAspect(aspect)` | `Aspects.of(construct).add(aspect)` +`myConstruct.node` | `myConstruct.construct` +`myConstruct.node.applyAspect(aspect)` | `Aspects.of(myConstruct).add(aspect)` `@aws-cdk/core.IDependable` | `constructs.IDependable` `@aws-cdk/core.DependencyTrait` | `constructs.Dependable` `@aws-cdk.core.DependencyTrait.get(x)` | `constructs.Dependable.of(x)` -`construct.node.dependencies` | Is now non-transitive -`construct.addMetadata()` | Stack trace not attached by default +`myConstruct.`node.dependencies` | Is now non-transitive +`myConstruct.`addMetadata()` | Stack trace not attached by default `ConstructNode.prepareTree()`, `node.prepare()`, `onPrepare()`, `prepare()` | Not supported, use aspects instead `ConstructNode.synthesizeTree()`, `node.synthesize()`, `onSynthesize()`, `synthesize()` | Not supported -`construct.onValidate()`, `construct.validate()` hooks | Implement `constructs.IValidation` and call `node.addValidation()` -`ConstructNode.validate(node)` | `construct.node.validate()` +`myConstruct.`onValidate()`, `myConstruct.`validate()` hooks | Implement `constructs.IValidation` and call `myConstruct.construct.addValidation()` +`ConstructNode.validate(node)` | `myConstruct.construct.validate()` -The following sections describe all the related breaking changes and details -migration strategies for each change. ## 00-DEPENDENCY: Declare a dependency on "constructs" @@ -168,6 +179,27 @@ The following `@aws-cdk/core` types have stand-in replacements in `constructs`: See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). +## 10-CONSTRUCT-NODE: `myConstruct.node` is now `myConstrct.construct` + +The `node` property of `Construct` was removed in favor of a property named +`construct` in order to improve discoverability of the construct API and reduce +the chance for naming conflicts with generated APIs (see +[hashicorp/terraform-cdk#230](https://github.com/hashicorp/terraform-cdk/pull/230)). + +For example, to add a dependency to a construct: + +Before: + +```ts +c1.node.addDependency(c2); +``` + +After: + +```ts +c1.construct.addDependency(c2); +``` + ## 02-ASPECTS: Changes in Aspects API Aspects are not part of the "constructs" library, and therefore instead of @@ -191,13 +223,13 @@ If you need to implement `IDependable`: - The `@aws-cdk/core.DependencyTrait` class has been replaced with `constructs.Dependable` - `@aws-cdk.core.DependencyTrait.get(x)` is now `constructs.Dependable.of(x)` -- `construct.node.dependencies` is now **non-transitive** and returns only the +- `c.construct.dependencies` is now **non-transitive** and returns only the dependencies added to the current node. -The method `construct.node.addDependency(otherConstruct)` __did not change__ and +The method `c.construct.addDependency(otherConstruct)` __did not change__ and can be used as before. -> You can use the new `construct.node.dependencyGraph` to access a rich object +> You can use the new `c.construct.dependencyGraph` to access a rich object > model for reflecting on the node's dependency graph. See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). @@ -213,27 +245,27 @@ const myConstruct = new MyConstruct(stack, 'MyConstruct'); ``` This is still a supported idiom, but in 2.x these root stacks will have an -implicit `App` parent. This means that `stack.node.scope` will be an `App` +implicit `App` parent. This means that `stack.construct.scope` will be an `App` instance, while previously it was `undefined`. The "root" stack will have a construct ID of `Default` unless otherwise specified. -Please note that this also means that the value of `node.path` for all +Please note that this also means that the value of `construct.path` for all constructs in the tree would now have a `Default/` prefix (if it was `Foo/Bar` it will now be `Default/Foo/Bar`). -> In contrast, the value of `node.uniqueId` will _not_ change because `Default` +> In contrast, the value of `construct.uniqueId` will _not_ change because `Default` > is a special ID that is ignored when calculating unique IDs (this feature > already exists in 1.x). ## 05-METADATA-TRACES: Stack traces no longer attached to metadata by default -For performance reasons, the `construct.node.addMetadata()` method will *not* +For performance reasons, the `c.construct.addMetadata()` method will *not* attach stack traces to metadata entries. Stack traces will still be associated with all `CfnResource` constructs and can also be added to custom metadata using the `stackTrace` option: ```ts -construct.node.addMetadata(key, value, { stackTrace: true }) +c.construct.addMetadata(key, value, { stackTrace: true }) ``` See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). @@ -279,7 +311,7 @@ hook, please post a comment on [this GitHub issue](https://github.com/aws/aws-cd ## 08-VALIDATION: The `validate()` hook is now `node.addValidation()` -To add validation logic to a construct, use `construct.node.addValidation()` +To add validation logic to a construct, use `c.construct.addValidation()` method instead of overriding a protected `validate()` method: Before: @@ -299,13 +331,13 @@ class MyConstruct extends Construct { constructor(scope: Construct, id: string) { super(scope, id); - this.node.addValidation({ validate: () => [ 'validation-error' ] }); + this.construct.addValidation({ validate: () => [ 'validation-error' ] }); } } ``` The static method `ConstructNode.validate(node)` is no longer available. You can -use `construct.node.validate()` which only validates the _current_ construct and +use `c.construct.validate()` which only validates the _current_ construct and returns the list of all error messages returned from calling `validation.validate()` on all validations added to this node. @@ -318,7 +350,7 @@ The `construct.node.addInfo()`, `construct.node.addWarning()` and Instead of: ```ts -construct.node.addWarning('my warning'); +myConstruct.node.addWarning('my warning'); ``` Use: @@ -485,6 +517,76 @@ Alternatives considered: - **Automatic merge resolution and import organization**: requires research and development, not necessarily worth it. +## 10-CONSTRUCT-NODE + +As we realize that we won't be able to release additional major versions of the +"constructs" in order to ensure interoperability between domains is always +possible, we need to closely examine the API of this library. + +In particular, the API of the `Construct` class, which is the base of all +constructs in all CDKs, should be as small as possible in order not to "pollute" +domain-specific APIs introduced in various domains. + +In many cases (cdk8s, cdktf), constructs are *generated* on-demand from +domain-specific API specifications. In such cases, we need to ensure that the +API in `Construct` does not conflict with generated property names or methods. + +The current API of `Construct` in the base class (2.x, 3.x) only includes a +bunch of protected `onXxx` methods (`onPrepare`, `onValidate` and +`onSynthesize`). Those methods will be removed in 10.x +([prepare](#06-no-prepare) and [synthesize](#07-no-synthesize) are no longer +supported and [validate](#08-validation) will be supported through +`addValidation()`). + +In AWS CDK 1.x, to construct API, is available under `myConstruct.node`. This +API has been intentionally removed from "constructs" 3.x in order to allow the +compatibility layer in AWS CDK 1.x to use property name and expose the shim +type. The base library currently offers `Node.of(scope)` as an alternative - but +this API is cumbersome to use and not discoverable. In evidence, in CDK for +Terraform, they chose to offer +[`constructNode`](https://github.com/hashicorp/terraform-cdk/blob/5becfbc699180adfe920dec794200bbf56dda0a7/packages/cdktf/lib/terraform-element.ts#L21) +in `TerraformElement` as a sugar for `Node.of()`. + +Another downside of `Node.of()` is that it means that the `IConstruct` interface +is now an empty interface, which is a very weak type in TypeScript due to +structural typing (it's structurally identical to `any`). + +As we evaluate this use case for constructs 10.x, we would like to restore the +ability to access the construct API from a property of `Construct`, and use that +property as the single marker that represents a construct type (`IConstruct`). + +To reduce the risk of naming conflicts (e.g. see [Terraform CDK +issue](https://github.com/hashicorp/terraform-cdk/pull/230)) between `node` and +domain-specific APIs, we propose to introduce this API under the name +`construct` (of type `Node`). + +This has a few benefits: + +1. It signals that "this is the construct API" and more uniquely identifies with + it. +3. The chance for conflicts with domain-specific names is minimized. +3. We can introduce this API without breaking existing code while deprecating + `node` in the AWS CDK. + +The main downside is that is is **a breaking change** in AWS CDK 2.x. There is +likely quite a lot of code out there (a [few +hundred](https://github.com/search?q=cdk+node.addDependency++extension%3Ats&type=Code&ref=advsearch&l=&l=) +results for an approximated GitHub code search). + +> We also considered the name `constructNode` as an alternative but there is no +> additional value in the word "node" being included, especially given the type +> is `Node`. + +### What can we do in 1.x + +As mentioned above, since this is a new name for this property, we can +technically introduce it in 1.x and announce that `node` is deprecated. This +will allow users to migrate to the new API before 2.x is released and hopefully +will reduce some of the friction from the 2.x migration. + +To encourage users to migrate, we will consider introducing this deprecation +through a runtime warning message (as well as the @deprecated API annotation). + ## 02-ASPECTS Aspects are actually a form of "prepare" and as such, if they mutate the tree, @@ -913,6 +1015,10 @@ created. - [ ] Normalize reference to base types (`cdk.Construct` => `Construct`). - [ ] Use an `awslint` rule to modify the `scope` argument on all 1.x to accept `constructs.Construct` instead of `core.Construct` +- [10-CONSTRUCT-NODE](#10-construct-node) + - [ ] Introduce `c.construct` as an alias to `c.node` + - [ ] Deprecate `c.node` (with a warning message) + - [ ] Replace all `c.node` with `c.construct`. - [02-ASPECTS](#02-aspects) - [ ] Introduce `Aspects.of(x)` and deprecate `applyAspect` - [ ] Introduce `Tags.of()` and deprecate `Tag.add()` @@ -951,8 +1057,9 @@ for constructs 10.x. - [00-DEPENDENCY](#00-dependency) - [ ] Document API compatibility assurance and the 10.x version number. - [01-BASE-TYPES](#01-base-types) - - [x] Reintroduce `construct.node` instead of `Node.of(construct)` - - [ ] Reintroduce `Construct.isConstruct()`. + - [x] Reintroduce `Construct.isConstruct()`. +- [10-CONSTRUCT-NODE](#10-construct-node) + - [ ] Reintroduce `construct.construct` instead of `Node.of(construct)` - [02-ASPECTS](#02-aspects) - [x] Remove aspects (`IAspect` and `node.applyAspect`). - [03-DEPENDABLE](#03-dependable) @@ -965,8 +1072,7 @@ for constructs 10.x. - [x] Return only local dependencies in `node.dependencies` - [ ] Migrate [DependencyGraph](https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s/src/dependency.ts) from cdk8s into `constructs`. - [04-STACK-ROOT](#04-stack-root) - - [x] Introduce `node.relocate()` to allow root stacks to be relocated to - `""`. + - N/A - [05-METADATA-TRACES](#05-metadata-traces) - [x] Do not emit stack traces in `addMetadata` (`{ stackTrace: true }`). - [06-NO-PREPARE](#06-no-prepare) From 6a1dc2768e04fca92d2e785c8eb0aea9ff34552a Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 27 Jul 2020 09:05:18 +0300 Subject: [PATCH 27/29] Apply suggestions from code review --- text/0192-remove-constructs-compat.md | 34 +++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 516f96be8..18759aac4 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -519,9 +519,9 @@ Alternatives considered: ## 10-CONSTRUCT-NODE -As we realize that we won't be able to release additional major versions of the -"constructs" in order to ensure interoperability between domains is always -possible, we need to closely examine the API of this library. +Since we won't be able to release additional major versions of the +"constructs" library (in order to ensure interoperability between domains is always +possible), we need to closely examine the API of this library. In particular, the API of the `Construct` class, which is the base of all constructs in all CDKs, should be as small as possible in order not to "pollute" @@ -532,21 +532,23 @@ domain-specific API specifications. In such cases, we need to ensure that the API in `Construct` does not conflict with generated property names or methods. The current API of `Construct` in the base class (2.x, 3.x) only includes a -bunch of protected `onXxx` methods (`onPrepare`, `onValidate` and +few protected `onXxx` methods (`onPrepare`, `onValidate` and `onSynthesize`). Those methods will be removed in 10.x ([prepare](#06-no-prepare) and [synthesize](#07-no-synthesize) are no longer supported and [validate](#08-validation) will be supported through `addValidation()`). -In AWS CDK 1.x, to construct API, is available under `myConstruct.node`. This -API has been intentionally removed from "constructs" 3.x in order to allow the -compatibility layer in AWS CDK 1.x to use property name and expose the shim -type. The base library currently offers `Node.of(scope)` as an alternative - but -this API is cumbersome to use and not discoverable. In evidence, in CDK for -Terraform, they chose to offer -[`constructNode`](https://github.com/hashicorp/terraform-cdk/blob/5becfbc699180adfe920dec794200bbf56dda0a7/packages/cdktf/lib/terraform-element.ts#L21) +In AWS CDK 1.x the construct API is available under `myConstruct.node`. This +API has been intentionally removed when we extracted "constructs" from the AWS CDK +in order to allow the compatibility layer in AWS CDK 1.x to use the same property name +and expose the shim type (jsii does not allow changing the type of a property in a subclass). + +The base library currently offers `Node.of(scope)` as an alternative - but this API is cumbersome +to use and not discoverable. In evidence, in CDK for Terraform, they chose to offer [`constructNode`] in `TerraformElement` as a sugar for `Node.of()`. +[`constructNode`]: https://github.com/hashicorp/terraform-cdk/blob/5becfbc699180adfe920dec794200bbf56dda0a7/packages/cdktf/lib/terraform-element.ts#L21 + Another downside of `Node.of()` is that it means that the `IConstruct` interface is now an empty interface, which is a very weak type in TypeScript due to structural typing (it's structurally identical to `any`). @@ -562,13 +564,11 @@ domain-specific APIs, we propose to introduce this API under the name This has a few benefits: -1. It signals that "this is the construct API" and more uniquely identifies with - it. -3. The chance for conflicts with domain-specific names is minimized. -3. We can introduce this API without breaking existing code while deprecating - `node` in the AWS CDK. +1. It's semantically signals that "this is the construct API". +2. The chance for conflicts with domain-specific names is low ("construct" is not prevalent). +3. We can introduce this API while deprecating `node` in AWS CDK 1.x. -The main downside is that is is **a breaking change** in AWS CDK 2.x. There is +The main downside is that it is **a breaking change** in AWS CDK 2.x. There is likely quite a lot of code out there (a [few hundred](https://github.com/search?q=cdk+node.addDependency++extension%3Ats&type=Code&ref=advsearch&l=&l=) results for an approximated GitHub code search). From 7194f8095492ed9b952d20df29b2fb111876a5e0 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 27 Jul 2020 09:06:59 +0300 Subject: [PATCH 28/29] Update text/0192-remove-constructs-compat.md --- text/0192-remove-constructs-compat.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 18759aac4..3542d3b2b 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -1,7 +1,7 @@ --- feature name: remove-constructs-compat start date: 2020-07-05 -rfc pr: (leave this empty) +rfc pr: [#195](https://github.com/aws/aws-cdk-rfcs/pull/195) related issue: [#192](https://github.com/aws/aws-cdk-rfcs/issues/192) --- From 5f2cc5062354f5849ea2778d34545e9b2b21bca0 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 28 Jul 2020 10:16:36 +0300 Subject: [PATCH 29/29] fix lint errors --- text/0192-remove-constructs-compat.md | 135 +++++++++++++------------- 1 file changed, 67 insertions(+), 68 deletions(-) diff --git a/text/0192-remove-constructs-compat.md b/text/0192-remove-constructs-compat.md index 3542d3b2b..14be50b02 100644 --- a/text/0192-remove-constructs-compat.md +++ b/text/0192-remove-constructs-compat.md @@ -5,7 +5,9 @@ rfc pr: [#195](https://github.com/aws/aws-cdk-rfcs/pull/195) related issue: [#192](https://github.com/aws/aws-cdk-rfcs/issues/192) --- -# Summary +# Removal of Construct Compatibility Layer + +## Summary As part of our effort to broaden the applicability of the CDK's programming model to other domains such as [Kubernetes](https://cdk8s.io), we have extracted @@ -25,7 +27,7 @@ This RFC describes the motivation, implications and plan for this project. [AWS CDK v2.0]: https://github.com/aws/aws-cdk-rfcs/issues/156 [construct-compat.ts]: https://github.com/aws/aws-cdk/blob/fcca86e82235387b88371a0682cd0fc88bc1b67e/packages/%40aws-cdk/core/lib/construct-compat.ts -# Table of Contents +## Table of Contents - [Summary](#summary) - [Table of Contents](#table-of-contents) @@ -72,7 +74,7 @@ This RFC describes the motivation, implications and plan for this project. - [constructs 10.x](#constructs-10x) - [2.x Work](#2x-work) -# Release Notes +## Release Notes > This section "works backwards" from the v2.0 release notes in order to > describe the user impact of this change. @@ -121,13 +123,12 @@ migration strategies for each change. `@aws-cdk.core.DependencyTrait.get(x)` | `constructs.Dependable.of(x)` `myConstruct.`node.dependencies` | Is now non-transitive `myConstruct.`addMetadata()` | Stack trace not attached by default -`ConstructNode.prepareTree()`, `node.prepare()`, `onPrepare()`, `prepare()` | Not supported, use aspects instead -`ConstructNode.synthesizeTree()`, `node.synthesize()`, `onSynthesize()`, `synthesize()` | Not supported -`myConstruct.`onValidate()`, `myConstruct.`validate()` hooks | Implement `constructs.IValidation` and call `myConstruct.construct.addValidation()` +`ConstructNode.prepareTree()`,`node.prepare()`,`onPrepare()`,`prepare()` | Not supported, use aspects instead +`ConstructNode.synthesizeTree()`,`node.synthesize()`,`onSynthesize()`,`synthesize()` | Not supported +`myConstruct.`onValidate()`,`myConstruct.`validate()` hooks | Implement `constructs.IValidation` and call `myConstruct.construct.addValidation()` instead `ConstructNode.validate(node)` | `myConstruct.construct.validate()` - -## 00-DEPENDENCY: Declare a dependency on "constructs" +### 00-DEPENDENCY: Declare a dependency on "constructs" As part of migrating your code to AWS CDK 2.0, you will need to declare a dependency on the `constructs` library (in addition to the `aws-cdk-lib` library @@ -168,7 +169,7 @@ normally want to use the highest version available: NOTE: Due to it's foundational nature, the `constructs` library is committed to never introduce breaking changes. Therefore, it's version will be `10.x`. -## 01-BASE-TYPES: Removal of base types +### 01-BASE-TYPES: Removal of base types The following `@aws-cdk/core` types have stand-in replacements in `constructs`: @@ -179,7 +180,7 @@ The following `@aws-cdk/core` types have stand-in replacements in `constructs`: See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). -## 10-CONSTRUCT-NODE: `myConstruct.node` is now `myConstrct.construct` +### 10-CONSTRUCT-NODE: `myConstruct.node` is now `myConstrct.construct` The `node` property of `Construct` was removed in favor of a property named `construct` in order to improve discoverability of the construct API and reduce @@ -200,7 +201,7 @@ After: c1.construct.addDependency(c2); ``` -## 02-ASPECTS: Changes in Aspects API +### 02-ASPECTS: Changes in Aspects API Aspects are not part of the "constructs" library, and therefore instead of `construct.node.applyAspect(aspect)` use `Aspects.of(construct).add(aspect)`. @@ -214,7 +215,7 @@ Tags.of(scope).add(name, value); See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). -## 03-DEPENDABLE: Changes to IDependable implementation +### 03-DEPENDABLE: Changes to IDependable implementation If you need to implement `IDependable`: @@ -234,7 +235,7 @@ can be used as before. See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). -## 04-STACK-ROOT: Stacks as root constructs +### 04-STACK-ROOT: Stacks as root constructs It is common in unit tests to use `Stack` as the root of the tree: @@ -257,7 +258,7 @@ it will now be `Default/Foo/Bar`). > is a special ID that is ignored when calculating unique IDs (this feature > already exists in 1.x). -## 05-METADATA-TRACES: Stack traces no longer attached to metadata by default +### 05-METADATA-TRACES: Stack traces no longer attached to metadata by default For performance reasons, the `c.construct.addMetadata()` method will *not* attach stack traces to metadata entries. Stack traces will still be associated @@ -270,7 +271,7 @@ c.construct.addMetadata(key, value, { stackTrace: true }) See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). -## 06-NO-PREPARE: The `prepare` hook is no longer supported +### 06-NO-PREPARE: The `prepare` hook is no longer supported The **prepare** hook (`construct.onPrepare()` and `construct.prepare()`) is no longer supported as it can easily be abused and cause construct tree corruption @@ -291,7 +292,7 @@ Aspects.of(this).add({ visit: () => this.prepare() }); The `ConstructNode.prepare(node)` method no longer exists. To realize references & dependencies in a scope call `Stage.of(scope).synth()`. -## 07-NO-SYNTHESIZE: The `synthesize` hook is no longer supported +### 07-NO-SYNTHESIZE: The `synthesize` hook is no longer supported The `synthesize()` overload (or `onSynthesize()`) is no longer supported. Synthesis is now implemented only at the app level. @@ -309,7 +310,7 @@ can can synthesize a construct node through `Stage.of(scope).synth()`. For additional questions/guidance on how to implement your use case without this hook, please post a comment on [this GitHub issue](https://github.com/aws/aws-cdk/issues/8909). -## 08-VALIDATION: The `validate()` hook is now `node.addValidation()` +### 08-VALIDATION: The `validate()` hook is now `node.addValidation()` To add validation logic to a construct, use `c.construct.addValidation()` method instead of overriding a protected `validate()` method: @@ -341,7 +342,7 @@ use `c.construct.validate()` which only validates the _current_ construct and returns the list of all error messages returned from calling `validation.validate()` on all validations added to this node. -## 09-LOGGING: Logging API changes +### 09-LOGGING: Logging API changes The `construct.node.addInfo()`, `construct.node.addWarning()` and `construct.node.Error()` methods are now available under the @@ -363,7 +364,7 @@ Logging.of(construct).addWarning('my warning'); See [examples](https://github.com/aws/aws-cdk/pull/9056/commits/e4dff913d486592b1899182e9a928765553654fa). -# Motivation +## Motivation There are various motivations for this change: @@ -372,13 +373,13 @@ There are various motivations for this change: 3. Inability to compose AWS CDK constructs into other domains 4. Non-intuitive dependency requirement on `constructs` -## 1. Redundant layer +### 1. Redundant layer The current compatibility layer does not have any logic in it. It is pure glue introduced in order to avoid breaking v1.x users. As we release v2.0 we are able to clean up this layer, and with it, improve maintainability and code hygiene. -## 2. Multiple `Construct` types +### 2. Multiple `Construct` types The current situation is error-prone since we have two `Construct` classes in the type closure. For example, when a developer types `"Construct"` and uses @@ -388,7 +389,7 @@ extends this type instead of the `core.Construct` type, it won't be possible to pass an instance of this class as a scope to AWS CDK constructs such as `Stack` for example. -## 3. Composability with other domains +### 3. Composability with other domains The main motivation for this change is to enable composition of AWS CDK constructs with constructs from other domains. @@ -422,7 +423,7 @@ Being able to create compositions from multiple CDK domains is a powerful direction for the CDK ecosystem, and this change is required in order to enable these use case. -## 4. Non-intuitive dependency requirement +### 4. Non-intuitive dependency requirement As we transition to [monolithic packaging] as part of v2.x, CDK users will have to take a _peer dependency_ on both the CDK library (`aws-cdk-lib`) and @@ -447,7 +448,7 @@ See the RFC for [monolithic packaging] for more details. [monolithic packaging]: https://github.com/aws/aws-cdk-rfcs/blob/master/text/0006-monolothic-packaging.md -# Design +## Design This section analysis the required changes and discusses the implementation approach and alternatives. @@ -458,7 +459,7 @@ costs and/or alert users of the upcoming deprecation. This design is based on this [proof of concept](https://github.com/aws/aws-cdk/pull/9056). -## 00-DEPENDENCY +### 00-DEPENDENCY In order to enable composability with other CDK domains (Terraform, Kubernetes), all constructs must use the same version of the `Construct` base class. @@ -477,7 +478,7 @@ We propose to take a commitment to __never introduce breaking changes in "constructs"__. This implies that we will never introduce another major version. To symbolize that to users, we will use the major version `10.x`. -## 01-BASE-TYPES +### 01-BASE-TYPES Once `construct-compat.ts` is removed from `@aws-cdk/core`, all CDK code (both the framework itself and user code) would need to be changed to use the types @@ -486,7 +487,7 @@ from `constructs`. Since the APIs are similar in almost all cases, this is a simple mechanical change as shown in the [release notes](#release-notes). -### What can we do on 1.x? +#### What can we do on 1.x The main concern for the CDK codebase is maintaining this change alongside a 1.x branch until we switch over to 2.x. Since this change includes modifications to @@ -517,7 +518,7 @@ Alternatives considered: - **Automatic merge resolution and import organization**: requires research and development, not necessarily worth it. -## 10-CONSTRUCT-NODE +### 10-CONSTRUCT-NODE Since we won't be able to release additional major versions of the "constructs" library (in order to ensure interoperability between domains is always @@ -541,7 +542,7 @@ supported and [validate](#08-validation) will be supported through In AWS CDK 1.x the construct API is available under `myConstruct.node`. This API has been intentionally removed when we extracted "constructs" from the AWS CDK in order to allow the compatibility layer in AWS CDK 1.x to use the same property name -and expose the shim type (jsii does not allow changing the type of a property in a subclass). +and expose the shim type (jsii does not allow changing the type of a property in a subclass). The base library currently offers `Node.of(scope)` as an alternative - but this API is cumbersome to use and not discoverable. In evidence, in CDK for Terraform, they chose to offer [`constructNode`] @@ -577,7 +578,7 @@ results for an approximated GitHub code search). > additional value in the word "node" being included, especially given the type > is `Node`. -### What can we do in 1.x +#### What can we do in 1.x As mentioned above, since this is a new name for this property, we can technically introduce it in 1.x and announce that `node` is deprecated. This @@ -587,7 +588,7 @@ will reduce some of the friction from the 2.x migration. To encourage users to migrate, we will consider introducing this deprecation through a runtime warning message (as well as the @deprecated API annotation). -## 02-ASPECTS +### 02-ASPECTS Aspects are actually a form of "prepare" and as such, if they mutate the tree, their execution order becomes critical and extremely hard to get right. To that @@ -618,13 +619,13 @@ sure documentation is clear. We will use this opportunity to normalize the tags API and change it to use the same pattern: `Tags.of(x).add(name, value)`. -### What can we do on 1.x? +#### What can we do on 1.x for 02-ASPECTS - We will migrate the 1.x branch to `Aspects.of(x)` and add a deprecation warning to `this.node.applyAspect()`. - Introduce `Tags.of(x).add()` and add a deprecation warning to `Tag.add()`. -## 03-DEPENDABLE +### 03-DEPENDABLE The `constructs` library supports dependencies through `node.addDependency` like in 1.x, but the API to implement `IDependable` has been changed. @@ -632,12 +633,12 @@ in 1.x, but the API to implement `IDependable` has been changed. The `constructs` library also introduces `DependencyGroup` which is a mix between `CompositeDependable` and `ConcreteDependable`. -### What can we do on 1.x? +#### What can we do on 1.x for 03-DEPENDABLE It should be possible to migrate the 1.x codebase to use the new APIs without any breaking changes to users. -## 04-STACK-ROOT +### 04-STACK-ROOT If we move staging of assets to the initialization phase, it means we need to know at that time where is the cloud assembly output directory. As mentioned @@ -703,7 +704,7 @@ synthesized output if called twice, we will also need to introduce a `stage.synth({ force: true })` option. This will be the default behavior when using `expect(stack)` or `SynthUtils.synth()`. -### Preserving unique IDs using an ID of `Default` +#### Preserving unique IDs using an ID of `Default` A side effect of adding a `App` parent to "root" stacks is that we now have an additional parent scope for all constructs in the tree. The location of the @@ -714,16 +715,16 @@ Since `uniqueId` is used in several places throughout the AWS Construct Library to allocate names for resources, and we have multiple unit tests that expect these values, we will use the ID `Default` for the root stack. -The `uniqueId` algorithm in the constructs library (see [reference]()) ignores +The `uniqueId` algorithm in the constructs library (see [reference](https://github.com/aws/constructs/blob/d5c3ee0a89e09318838f78bfb89832c0548d846d/src/private/uniqueid.ts#L33)) ignores any node with the ID `Default` for the purpose of calculating the unique ID, which allows us to perform this change without breaking unique IDs. We will accept the fact that `node.path` is going to change for this specific use case (only relevant in tests). -### Alternative considered +#### Alternative considered -#### Alternative 1: Breaking unique IDs +##### Alternative 1: Breaking unique IDs We explored the option of fixing all these test expectations throughout the CDK code base and back port this change over the 1.x behind a feature flag in order @@ -737,7 +738,7 @@ The downsides of this approach are: 1. We currently don't have a way to implicitly run all our unit tests behind a feature flag, and it is not a trivial capability to add. -#### Alternative 2: `node.relocate()` +##### Alternative 2: `node.relocate()` We also explored the option of introducing an additional capability to constructs called `node.relocate(newPath)` which allows modifying the path of a @@ -746,14 +747,14 @@ path. This would have allowed avoiding the breakage in `node.path` but would have also introduced several other idiosyncrasies and potential violations of invariants such as the fact that a path is unique within the tree. -### What can we do on 1.x? +#### What can we do on 1.x for 04-STACK-ROOT We will introduce this change over the 1.x branch as-is, acknowledging that we are technically breaking the behavior of `node.path` in unit tests which use `Stack` as the root. Since we are not breaking `uniqueId`, we expect this to be tolerable over the 1.x branch. -## 05-METADATA-TRACES +### 05-METADATA-TRACES Since stack traces are not attached to metadata entries by default in constructs 4.x, we will need to pass `stackTrace: true` for `CfnResource`s. This will @@ -762,12 +763,12 @@ preserve the deploy-time stack traces which are very important for users. Other metadata entries will not get stack traces by default, and that's a reasonable behavioral change. -### What can we do on 1.x? +#### What can we do on 1.x for 05-METADATA-TRACES No need to introduce over 1.x as the change is very local to `CfnResource` and therefore can be applies over 2.x without risk. -## 06-NO-PREPARE +### 06-NO-PREPARE The "prepare" hook was removed from constructs since it is a very fragile API. Since the tree can be mutated during prepare, the order of `prepare` invocations @@ -784,7 +785,7 @@ The prepare hook was used in the CDK in a few cases: The first two use cases have already been addressed by centralizing the "prepare" logic at the stage level (into [prepare-app.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/core/lib/private/prepare-app.ts)). -### What can we do on 1.x? +#### What can we do on 1.x for 06-NO-PREPARE - The 3rd item can be addressed using `Lazy` tokens (see [example](https://github.com/aws/aws-cdk/pull/8962/files#diff-51d435d71a31c2607f923fc4d96cac56R140)), @@ -793,13 +794,11 @@ The first two use cases have already been addressed by centralizing the that implement "prepare" and refer users to a GitHub issue for details and consultation. -## 07-NO-SYNTHESIZE +### 07-NO-SYNTHESIZE Version 4.x of the `constructs` library does not contain a lifecycle hook for synthesis as described [above](#the-synthesize-hook-is-no-longer-supported). -### Motivation - The reason this is not available at the base class is because the abstraction did not "hold water" as the AWS CDK evolved and new CDKs emerged. In the AWS CDK, we eventually ended up with a centralized synthesis logic at the @@ -820,7 +819,7 @@ specific domains can't offer a decentralized approach (i.e. call a method called "synthesize" on all constructs in the tree), it just means that this is not provided at the base `Construct` class. -### Implications on CDK code base +#### Implications on CDK code base In the AWS CDK itself, `synthesize()` was used in three locations: @@ -840,7 +839,7 @@ this as soon as the CDK app is created). Therefore, the solution for `AssetStaging` is to move the staging logic to the constructor and use `Stage.of(this).outdir` to find the output directory of the current stage. -### Implications on end-users +#### Implications on end-users Participation in synthesis is an "advanced" feature of the CDK framework and we assume most end-users don't use this directly. @@ -857,7 +856,7 @@ make tons of sense. Yet, if there is still a use case for writing files during initialization, it is possible to find out the output directory through `Stage.of(scope).outdir`. This is how asset staging will be implemented. -### What can we do on 1.x? +#### What can we do on 1.x for 07-NO-SYNTHESIZE 1. The framework changes should be done on the 1.x branch as they are non-breaking. 2. We will also add a deprecation notice that identifies the existence of a @@ -865,7 +864,7 @@ initialization, it is possible to find out the output directory through this hook will no longer be available in 2.x, offering a GitHub issue for details consultation. -## 08-VALIDATION +### 08-VALIDATION Since construct validation is quite rare and we want to encourage users to validate in entry points, in constructs 4.x, the `validate()` protected method @@ -875,13 +874,13 @@ was removed and `node.addValidation()` can be used to add objects that implement An error will be thrown if a `validate()` method is found on constructs with instructions on how to implement validation in 2.x. -### What can we do on 1.x? +#### What can we do on 1.x for 08-VALIDATION We can introduce this change over the 1.x as long as we continue to support `validate()` alongside a deprecation warning with instructions on how to migrate to the new API. -## 09-LOGGING +### 09-LOGGING We decided that logging is not generic enough to include in `constructs`. It emits construct metadata that is very CLI specific (e.g. `aws:cdk:warning`) and @@ -889,13 +888,13 @@ currently there is no strong abstraction. To continue to enable logging, we will utilize the `Logging.of(x).addWarning()` pattern. -### What can we do on 1.x? +#### What can we do on 1.x for 09-LOGGING We can introduce this change on 1.x and add a deprecation warning. -# Drawbacks +## Drawbacks -## User migration effort +### User migration effort The main drawback from users' point of view is the introduction of the aforementioned breaking changes as part of the transition to CDK 2.0. As @@ -906,7 +905,7 @@ The removal of the "prepare" and "synthesize" hooks may require users to rethink their design in very advanced scenarios. We will create a GitHub issue to consult users on alternative designs. -## CDK codebase migration efforts +### CDK codebase migration efforts The AWS CDK codebase itself utilizes all of these APIs, and the migration effort is quite substantial. @@ -914,7 +913,7 @@ is quite substantial. Having said that, the majority of this work is already complete and a branch is being maintained with these changes as a pre-cursor to the v2.x fork. -## Staging of the 2.x fork +### Staging of the 2.x fork Since this change involves modifications to the CDK's source code, it may cause merge conflicts as during the period in which we need to forward-port or @@ -922,7 +921,7 @@ back-port code between the v1.x branch and the v2.x branches. The key would be to continuously merge between the branches. -# Rationale and Alternatives +## Rationale and Alternatives As a general rule, software layers which do not provide value to users should not exist. The constructs compatibility layer was added as solution for @@ -945,7 +944,7 @@ The [repository migration](#repository-migration-efforts) efforts and proposal suggests ways to reduce the chance for merge conflicts across these branches. -## Alternatives considered +### Alternatives considered At a high-level, we may consider to postpone this change to v3.x or to never take it, leaving this compatibility layer in place for eternity. @@ -969,11 +968,11 @@ Postponing to v3.x will leave us with the set exact set of problems, only with a The [Design](#design) section describes alternatives for various aspects of this project. -# Adoption Strategy +## Adoption Strategy See [Release Notes](#release-notes). -# Unresolved questions +## Unresolved questions - [ ] Automation of `import` conflict resolution. @@ -985,7 +984,7 @@ See [Release Notes](#release-notes). > addressed in the future independently of the solution that comes out of this > RFC? -# Future Possibilities +## Future Possibilities > Think about what the natural extension and evolution of your proposal would be > and how it would affect CDK as whole. Try to use this section as a tool to more @@ -998,9 +997,9 @@ See [Release Notes](#release-notes). > If you have tried and cannot think of any future possibilities, you may simply > state that you cannot think of anything. -# Implementation Plan +## Implementation Plan -## Preparation of 1.x +### Preparation of 1.x We will try to front load as much of this change to 1.x in order to reduce the merge conflict potential. @@ -1049,7 +1048,7 @@ created. - [09-LOGGING](#09-logging) - [ ] Introduce `Logging.of()` deprecate `node.addWarning/error/info`. -## constructs 10.x +### constructs 10.x [This branch](https://github.com/aws/constructs/pull/133) is the staging branch for constructs 10.x. @@ -1085,7 +1084,7 @@ for constructs 10.x. - [09-LOGGING](#09-logging) - [x] Remove `node.addWarning()`, `node.addError()`, ... -## 2.x Work +### 2.x Work - [ ] Once the 2.x branch will be created, we will merge the remaining changes from the POC branch into it.