From fdfa05112669d5f5eba964a7bf1d10e019456289 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Mon, 24 Aug 2020 16:38:16 +0100 Subject: [PATCH 1/4] rfc 79: cdk v2.0 - change strategy on how deprecated APIs are handled Two changes to the RFC: - The branch name `next` has been updated to `v2` throughout the RFC. - The strategy on how deprecated APIs are handled has been revamped to be less manual and more automated. A one-time clean up towards the end of the project is included. --- text/0079-cdk-2.0.md | 69 ++++++++++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/text/0079-cdk-2.0.md b/text/0079-cdk-2.0.md index a60decef3..eaff195f0 100644 --- a/text/0079-cdk-2.0.md +++ b/text/0079-cdk-2.0.md @@ -131,7 +131,7 @@ v1 will be front-loaded. This includes all work that can be prepared without bre ## Forking v2 out of v1 -The forking point will be achieved by cutting a new `next` branch from the current tip of the `master` branch, where the +The forking point will be achieved by cutting a new `v2` branch from the current tip of the `master` branch, where the following changes will be made right away: 1. the repository-wide `version` configuration will be updated to `2.0.0-alpha` (the pre-release identifier could be @@ -168,8 +168,8 @@ validated. Releases will be automatically published, however using the `next` NP [github packages]: https://github.com/features/packages As long as the codebases have not diverged too much, it should be possible to continue forward-porting new developments -on top of the `next` branch by simply cherry-picking new commits from the `master` branch. Alternatively, it might be -possible to simply periodically merge `master` into `next`. This task can easily be automated. +on top of the `v2` branch by simply cherry-picking new commits from the `master` branch. Alternatively, it might be +possible to simply periodically merge `master` into `v2`. This task can easily be automated. A new standard operating procedure (SOP / MCM) will be made to support manually releasing a CDK `v2` release until this process is fully automated. Additionally, canaries and integration tests need to be prepared to ensure the ongoing @@ -184,9 +184,9 @@ _Thursdays_), manually resolving merge conflicts as needed. The AWS Construct Library will be released as a single artifact, instead of a collection of more than 100 libraries. The `monocdk-experiment` package has the tools necessary to re-package the existing _hypermodular_ libraries into a single -packaging. Re-using that same process will allow keeping the difference between `master` and `next` as constrained as +packaging. Re-using that same process will allow keeping the difference between `master` and `v2` as constrained as possible (reducing the need for manual intervention in forward-porting features). No additional packaging work is needed -once the `next` branch has been forked from `master`, as the renaming of `monocdk-experiment` to `aws-cdk-lib` and +once the `v2` branch has been forked from `master`, as the renaming of `monocdk-experiment` to `aws-cdk-lib` and marking all bundled packages `private` is taken care of at that time. However, since users will all use a single `aws-cdk-lib` library, the granularity of information collected via the @@ -210,15 +210,23 @@ The definition of `@deprecated` in the CDK specifies that annotated APIs will be release. Consequently, we will systematically remove all `@deprecated` APIs from the codebase upon cutting a new major version. -This task is expected to be easy to carry out by hand. Searching the codebase for `@deprecated` and deleting the -annotated code, as well as any private function that is no longer used. In the event non-`@deprecated` code is found to -depend on `@deprecated` code, the implementation in `master` will be fixed to no longer defer into the `@deprecated` -feature, and that fix will be forward-ported into the `next` branch in the exact same way that other changes are. +Tooling will enforce that no deprecated APIs methods, properties, interfaces or classes will be used within the CDK +repository. This also applies to tests, with an option for individual overrides, in the case of testing the deprecated +APIs themselves. -In order to prevent accidental introduction (or re-introduction) of `@deprecated` code in the `next` branch while -forward-porting new developments from `master`, a built-time check will be added that scans the codebase for -`@deprecated` (for example using `grep`), failing the build if occurrences are found. This is not done through -inspection of the _jsii assemblies_ as `@deprecated` code can exist outside of exported API signatures. +The `jsii` binary will accept a `--no-deprecated` flag that will skip deprecated APIs from the `.jsii` binary. In +conjunction, deprecated APIs will be stripped off the typescript files as part of producing the monolithic package, +`aws-cdk-lib`, similar to [import rewrites]. + +When CDK v2.0 become generally available, the source code will be updated to remove all use of `@deprecated`. +This will occur as part of flipping `master` to point to the `v2` branch, with a code freeze of at most 2 days +instituted. +Additionally, the `--no-deprecated` flag and the typescript transform will be dropped, now enabling API deprecations on +CDKv2. + +**Why not remove these by hand?** See [alternatives section](#removal-of-deprecated-apis---why-not-remove-these-by-hand). + +[import rewrites]: https://github.com/aws/aws-cdk/tree/master/packages/%40monocdk-experiment/rewrite-imports ### Resetting v1 feature flags @@ -241,8 +249,8 @@ retain the ability to disable as needed. > | `@aws-cdk/core:enableStackNameDuplicates` | Update to new file names under `cdk.out` | 🚮 Remove | > | `@aws-cdk/core:newStyleStackSynthesis` | Re-bootstrap environments | 🚮 Remove | -In order to reduce the codebase divergence between `master` and `next` (in order to minimize the amount of manual work -involved in forward-porting features on `next`), existing feature flags must be made version-aware, such that they are +In order to reduce the codebase divergence between `master` and `v2` (in order to minimize the amount of manual work +involved in forward-porting features on `v2`), existing feature flags must be made version-aware, such that they are enabled by default (and impossible to disable if a clean and reasonably simple migration is possible) when the current version is greater then `2.0.0-0`. @@ -288,7 +296,7 @@ prerelease announcement should at least mention the following elements: All breaking changes should be handled in the form of a deprecation in CDK v1. In the event a particular change cannot be achieved in this way, a limited amount of time will be provided when _AWS Construct Library_ owners (as well as third -party contributors) will be allowed to introduce API-breaking changes in the `next` branch. These should be limited to +party contributors) will be allowed to introduce API-breaking changes in the `v2` branch. These should be limited to items that were identified and documented as part of the [Prerelease Announcement](#prerelease-announcement). Additional changes can however be introduced if a well documented proposal achieves consensus among the core development team. All breaking changes (including those planned in the prerelease announcement) will be documented in the release notes for @@ -332,9 +340,9 @@ Once the criteria have been satisfied, the high-level procedure for officially r 1. Create a new `v1` branch from the `master` branch 1. Configure GitHub branch protection on `v1` as appropriate 1. Deploy CI/CD infrastructure on top of `v1` -1. Make `next` the new `master` branch: - 1. Force-push `next` on `master` - 1. Delete `next` and all related CI/CD infrastructure +1. Make `v2` the new `master` branch: + 1. Force-push `v2` on `master` + 1. Delete `v2` and all related CI/CD infrastructure 1. Perform a _normal_ release 1. Ceremoniously annouce CDK v2 is _Generally Available_, and repeat the v1 maintenance plan @@ -461,6 +469,29 @@ unless we stop supporting the `@aws-cdk/` packages. Since we want to eventually supplant the current module structure with the new one, semver says we have to do it with the release of a major version. +## Removal of @deprecated APIs - Why not remove these by hand? + +An alternative that was considered is to remove these by hand in the `v2` branch. As of Aug 24, 2020, there are ~250 +deprecated properties, methods and classes combined, distributed across ~80 files. + +While most of these would be properties and simple public APIs being deprecated, they also include larger sections +such as entire classes. The main challenge with this approach are the merge conflicts that will arise when merging +changes from the `master` branch onto the `v2` branch. + +Simple changes to files that contain a few deprecated APIs will pose no difficulty in `git` resolving these conflicts. +However, any changes to the implementation of a deprecated API, file refactor or any other major changes will require +non-trivial human intervention. It is unclear who owns the resolution of these conflicts, at what point they will be +resolved (i.e., during PR, during forward merge, during pipeline approval). + +We expect the forked state of the system (i.e., two active branches) to exist for several months as we work through the +changes to have 'CDK 2.0' generally available. As we get closer to 'CDK v2.0', we must expect the list of major changes +to this major release to expand. This may manifest as major or significant changes to the code and its structure. +We must also expect an increased rate of deprecated APIs across the CDK as the reality of CDKv2 gets closer, with the +team looking to use this opportunity to implement major changes. + +The continuous overhead of conflict resolution or a ban on large refactors or changes is an unreasonable expectation on +the team for such an extended period of time. + # Adoption Strategy All users will perform the following to adopt CDK v2 From af5386e5542038ec38685fbad5b6d091ef21ea24 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Wed, 26 Aug 2020 17:28:22 +0100 Subject: [PATCH 2/4] rewrite the section to be clearer --- text/0079-cdk-2.0.md | 47 +++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/text/0079-cdk-2.0.md b/text/0079-cdk-2.0.md index eaff195f0..eebca8cf0 100644 --- a/text/0079-cdk-2.0.md +++ b/text/0079-cdk-2.0.md @@ -210,21 +210,44 @@ The definition of `@deprecated` in the CDK specifies that annotated APIs will be release. Consequently, we will systematically remove all `@deprecated` APIs from the codebase upon cutting a new major version. -Tooling will enforce that no deprecated APIs methods, properties, interfaces or classes will be used within the CDK -repository. This also applies to tests, with an option for individual overrides, in the case of testing the deprecated -APIs themselves. +As described above in the [forking v2 out of v1](#forking-v2-out-of-v1) section, there will be a significant period of +time when both CDKv1 and CDKv2 will be in active development. During this time, they will be tracked in the source +code respectively by two active branches, `master` and `v2`, where changes on `master` will be forward merged onto `v2`. +Removing the deprecated APIs by hand right at the beginning is going to result in increased maintenance overhead caused +by merge conflicts. See [alternatives section](#removal-of-deprecated-apis---why-not-remove-these-by-hand) for more +details. -The `jsii` binary will accept a `--no-deprecated` flag that will skip deprecated APIs from the `.jsii` binary. In -conjunction, deprecated APIs will be stripped off the typescript files as part of producing the monolithic package, -`aws-cdk-lib`, similar to [import rewrites]. +The following strategy will be used to reduce merge conflicts while shipping CDKv2 without deprecated APIs right from +its first release - -When CDK v2.0 become generally available, the source code will be updated to remove all use of `@deprecated`. -This will occur as part of flipping `master` to point to the `v2` branch, with a code freeze of at most 2 days -instituted. -Additionally, the `--no-deprecated` flag and the typescript transform will be dropped, now enabling API deprecations on -CDKv2. +Before creating the `v2` branch, -**Why not remove these by hand?** See [alternatives section](#removal-of-deprecated-apis---why-not-remove-these-by-hand). +- The `jsii` assembler will be updated to accept a `--no-deprecated` flag that will strip out all deprecated methods, +properties, classes and interfaces. This will be achieved via a typescript transform that is run before assembly +generation. A proof of concept for this transform is available [here](https://github.com/nija-at/cdk-deprecated-api-poc). + +- `cdk-build` will be updated to recognize the environment variable `CDK_BLOCK_DEPRECATIONS` and set the +`--no-deprecated` flag on the `jsii` assembler. + +- All current usage of deprecated APIs within the CDK repository will be removed. In addition, a new check will be +added to pull requests on the `master` branch, that will run a full CDK build with the `CDK_BLOCK_DEPRECATIONS` +environment variable set. This will ensure that no usage of existing or new deprecations creep into the repository. + +- In order to support unit tests that verify the behaviour of deprecated APIs, `cdk-test` will be updated to recognize +a new pragma `!/// !cdk-test:allow-deprecations` and the `CDK_BLOCK_DEPRECATIONS` environment variable. Tests holding +this pragma will be skipped when `CDK_BLOCK_DEPRECATIONS` is set. + +For the first and every subsequent release of CDKv2, + +- the release pipeline will be configured with the +`CDK_BLOCK_DEPRECATIONS` environment variable. + +The `master` branch will remain in this state until we are ready to flip `v2` branch to be the new +`master`. At this point, + +- a two day code freeze will be instituted and a one time clean up of all deprecated APIs from the source +code will be performed. +- The PR check will be removed from the `master` branch. Let the deprecations on CDKv2 begin! [import rewrites]: https://github.com/aws/aws-cdk/tree/master/packages/%40monocdk-experiment/rewrite-imports From b23ce0f4dc48565eb88943aebff0d331dd0b4fcf Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Wed, 26 Aug 2020 17:34:51 +0100 Subject: [PATCH 3/4] some clarifications --- text/0079-cdk-2.0.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/text/0079-cdk-2.0.md b/text/0079-cdk-2.0.md index eebca8cf0..eb0d07c74 100644 --- a/text/0079-cdk-2.0.md +++ b/text/0079-cdk-2.0.md @@ -230,8 +230,9 @@ generation. A proof of concept for this transform is available [here](https://gi `--no-deprecated` flag on the `jsii` assembler. - All current usage of deprecated APIs within the CDK repository will be removed. In addition, a new check will be -added to pull requests on the `master` branch, that will run a full CDK build with the `CDK_BLOCK_DEPRECATIONS` -environment variable set. This will ensure that no usage of existing or new deprecations creep into the repository. +added to pull requests on the `master` branch, that will run a full CDK build **and test** with the +`CDK_BLOCK_DEPRECATIONS` environment variable set. The typescript compilation and execution of the CDK's comprehensive +unit test suite should highlight any usage of deprecated APIs. - In order to support unit tests that verify the behaviour of deprecated APIs, `cdk-test` will be updated to recognize a new pragma `!/// !cdk-test:allow-deprecations` and the `CDK_BLOCK_DEPRECATIONS` environment variable. Tests holding @@ -239,8 +240,8 @@ this pragma will be skipped when `CDK_BLOCK_DEPRECATIONS` is set. For the first and every subsequent release of CDKv2, -- the release pipeline will be configured with the -`CDK_BLOCK_DEPRECATIONS` environment variable. +- the release pipeline will be configured with the `CDK_BLOCK_DEPRECATIONS` environment variable that will run the +previously mentioned typescript transform as part of monolithic packaging (i.e., generation of `aws-cdk-lib`). The `master` branch will remain in this state until we are ready to flip `v2` branch to be the new `master`. At this point, From 29b3678739ebe9b4fd9e9f8075cdb3a963a04903 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Thu, 3 Sep 2020 11:05:26 +0100 Subject: [PATCH 4/4] added note around deprecations from aws --- text/0079-cdk-2.0.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/text/0079-cdk-2.0.md b/text/0079-cdk-2.0.md index eb0d07c74..33e6016d8 100644 --- a/text/0079-cdk-2.0.md +++ b/text/0079-cdk-2.0.md @@ -210,6 +210,12 @@ The definition of `@deprecated` in the CDK specifies that annotated APIs will be release. Consequently, we will systematically remove all `@deprecated` APIs from the codebase upon cutting a new major version. +*Sidenote:* There are a few APIs in the CDK that don't follow the above definition, specifically, [lambda runtimes] and +[RDS Oracle versions]. The APIs in these cases are marked deprecated because it has either been deprecated by the +underlying AWS service or the AWS service has deemed it to have fallen out of best practice. In such cases, either +these APIs will be removed in CDKv2, or if deemed necessary, `@deprecated` annotation will be removed and replaced by +'DEPRECATED:' note in its documentation. + As described above in the [forking v2 out of v1](#forking-v2-out-of-v1) section, there will be a significant period of time when both CDKv1 and CDKv2 will be in active development. During this time, they will be tracked in the source code respectively by two active branches, `master` and `v2`, where changes on `master` will be forward merged onto `v2`. @@ -248,9 +254,12 @@ The `master` branch will remain in this state until we are ready to flip `v2` br - a two day code freeze will be instituted and a one time clean up of all deprecated APIs from the source code will be performed. + - The PR check will be removed from the `master` branch. Let the deprecations on CDKv2 begin! [import rewrites]: https://github.com/aws/aws-cdk/tree/master/packages/%40monocdk-experiment/rewrite-imports +[lambda runtimes]: https://github.com/aws/aws-cdk/blob/b757d8856216152b03c1ed3ffe2f29719aff8c27/packages/%40aws-cdk/aws-lambda/lib/runtime.ts#L37-L63 +[RDS Oracle versions]: https://github.com/aws/aws-cdk/blob/34f40b9fb4b467c9c7af290400efa2c8ac8c8e10/packages/%40aws-cdk/aws-rds/lib/cluster-engine.ts#L449-L471 ### Resetting v1 feature flags