diff --git a/text/0055-feature-flags.md b/text/0055-feature-flags.md new file mode 100644 index 000000000..c05963bd1 --- /dev/null +++ b/text/0055-feature-flags.md @@ -0,0 +1,237 @@ +--- +feature name: feature-flags +start date: +rfc pr: https://github.com/aws/aws-cdk/pull/5017 +related issue: https://github.com/awslabs/aws-cdk-rfcs/issues/55 +--- + +# Summary + +Feature flags will allow us to introduce new breaking behavior which is disabled +by default (so existing projects will not be affected) but enabled automatically +for new projects created through `cdk init`. + +# Motivation + +Sometimes (hopefully rarely) we want to introduce new breaking behavior because +we believe this is the correct default behavior for the CDK. The problem, of +course, is that breaking changes are only allowed in major versions and those +are rare. + +# Basic Example + +For example, the AWS CDK had a bug +([#4925](https://github.com/aws/aws-cdk/pull/4925)) where it was impossible to +use the same physical stack name for multiple stacks, even if they were +targeting different regions despite the fact that this is technically possible +(and sometimes desired). + +If multiple stacks can have the same physical name, we need another way to +identify stacks uniquely (e.g. when selecting stacks in various CLI commands +like `cdk deploy X`). To that end, we introduced the concept of an **artifact +ID**. In most cases, artifact IDs will be the same as stack names, so the common +behavior will stay the same. However, in some cases, the current behavior would +break. + +If we introduced this fix without a feature flag, it means that current users +may break. Therefore, we introduced this feature under a feature flag named +`@aws-cdk/core:enableStackNameDuplicates`. + +This feature is disabled by default, which means that the behavior for existing +projects would remain the same (along with the limitation of course), but new +projects would have the flag automatically enabled in their `cdk.json` file. + +# Design Summary + +The basic idea is that new breaking behavior will always be disabled by default +and only enabled when a certain CDK context parameter is set. If not enabled, +the system will continue to behave exactly like it used to without breaking any +existing projects. + +When we release a new major version of the AWS CDK, we will flip this behavior +or completely remove the legacy behavior. + +In order for new projects to pick up this new behavior automatically, we will +modify `cdk init` to inject the set of feature flags into the generated +`cdk.json` file. This means that the new project will have the latest behavior, +but projects that were created prior to the introduction of this feature will +have the same legacy behavior based on the set of capabilities that were +available at the time of the project's creation. This list will be cleaned up +every time we release a major version of course. + +Using fine-grained flags will allow users of old projects to pick up specific +new behaviors by manually adding the specific keys to their `cdk.json` file, +without risking breakage in other unexpected areas. + +# Detailed Design + + +Context keys for feature flags will be listed in `cx-api/lib/features.ts` and +will take the form: `:`. + +For example: + +- `@aws-cdk/core:enableStackNameDuplicates` +- `@aws-cdk/aws-cloudformation:doNotCapitalizeCustomResourcePropertyNames`. + +Using the module name will allow easy tracing of the code that consumes this +flag. + +The configuration for which feature flags should be enabled for new projects +will be under `cx-api/lib/future.ts` and will be encoded as a simple context +hash that will be injected by `cdk init` to all `cdk.json` files generated for +new projects. + +We will mandate that when a feature or bug fix is introduced under a feature +flag, the CHANGELOG will include: + +- The suffix `(under feature flag)` in the title. +- A `BREAKING CHANGES` paragraph will be added which describes the *new* + behavior but disclaims that it will only apply to new projects created through + `cdk init`. It will also indicate the context key this flag uses for users who + wish to enable it manually in their project. + +Since feature flags can have implications on framework behavior, we need to ask +users to include the list of enabled features in bug reports. At a minimum, we +can request that they paste a copy of their `cdk.json` and `cdk.context.json`, +but a better experience would be to include this information in the output of +`cdk doctor` and request users to include this output in bug reports. + +# Drawbacks + +There are a few concerns this capability raises. These concerns are mostly +emphasized in the situation where we have a proliferation of feature flags. If +we end up with less than 5 feature flags before we bump a major version (and +eradicate all existing flags), then I believe these drawbacks are not +substantial. If we end up with dozens of flags, all of these will become an +issue. + +Therefore, the main mitigation is to make sure we don't abuse this capability +and only introduce feature flags when all creative and genuine attempts to avoid +a breaking change were exhausted. + +## Discoverability + +If users wish to enable a flag in an existing project, they need a way to find +out which flag to enable in their `cdk.json` and how. + +This drawback will be mitigated by: + +- [ ] Adding a documentation section about feature flags in the developer guide, + pointing to the `cx-api/lib/features.ts` file as an index of feature flags. +- [x] Announce the feature flag ID in our release notes under `BREAKING CHANGE: + (under feature flag)`. + +## Testing + +A feature flag is a system-level degree of freedom. Theoretically, every flag +introduces another dimension in our entire test matrix. Without extensive +tooling, it will be impossible to actually run our entire test suite against all +permutations of feature flags. + +Unit tests across the framework will normally continue to define apps with all +feature flags disabled (this is the default). In the case where a test depends +on a feature flag being enabled, it will explicitly enable it when the `App` +construct is defined through it's `context` option. + +The feature itself will be tested in both enabled and disabled mode. + +In the meantime, we will not introduce tooling for matrix coverage due to it's +complexity and impact on build times and contributor experience. As long as we +don't have a proliferation of flags, I believe this is a reasonable trade-off. + +## Support + +When an issue is raised, we need to be able to reproduce it. Since feature flags +can implicitly change how the CDK behaves, we need to know which features are +enabled. + +To mitigate this risk we will: + +- [ ] Add feature flags to `cdk doctor` and update bug report template + accordingly to request users to run `cdk doctor`. + +# Rationale and Alternatives + +We considered an alternative of "bundling" new capabilities under a single flag +that specifies the CDK version which created the project, but this means that +users won't have the ability to pick and choose which capabilities they want to +enable in case they need them but don't want to take the risk of unexpected +changes. + +The downside of the fine-grained approach is that it could result in a "blowing +up" new `cdk.json` files in case there will be many new breaking capabilities +between major releases. But this is hypothetical and even if this list ends up +with 20 features before we release the next major version, I still think the +benefits outweigh the risks of the alternative approach. + +# Adoption Strategy + +Most CDK users will likely not need to know about feature flags. Projects +created before a feature flag was introduced will continue to behave in the same +way and new projects created through `cdk init` will automatically get all new +behaviors. + +The contributor experience for using feature flags will be documented in the +contribution guide and will involve the following steps: + +1. Seek the approval of a core team member that a feature flag can be used. + * If the feature in question is being planned via an RFC, and the feature + flag is contained in the proposal, core team member approval should include + the feature flag. + * If the feature is being tracked in a single issue without an RFC, approval + should be indicated in this issue. +2. Define a new const under + [cx-api/lib/features.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/cx-api/lib/features.ts) + with the name of the context key that **enables** this new feature (for + example, `ENABLE_STACK_NAME_DUPLICATES`). The context key should be in the + form `module.Type:feature` (e.g. `@aws-cdk/core:enableStackNameDuplicates`). +3. Use `node.tryGetContext(cxapi.ENABLE_XXX)` to check if this feature is + enabled in your code. If it is not defined, revert to the legacy behavior. +4. Add your feature flag to + [cx-api/lib/future.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/cx-api/lib/future.ts). + This map is inserted to generated `cdk.json` files for new projects created + through `cdk init`. +5. In your PR title (which goes into CHANGELOG), add a `(behind feature flag)` + suffix. e.g: + ``` + fix(core): impossible to use the same physical stack name for two stacks (under feature flag) + ``` +5. Under `BREAKING CHANGES`, add a prefix `(under feature flag)` and the name of + the flag in the postfix. For example: + + ``` + BREAKING CHANGE: (under feature flag) template file names for new projects created + through "cdk init" will use the template artifact ID instead of the physical stack + name to enable multiple stacks to use the same name (feature flag: @aws-cdk/core:enableStackNameDuplicates) + ``` + +# Unresolved questions + +I believe the biggest unresolved question is how many feature flags we will end +up with until the next major version bump. We introduced a bit of process to +require that feature flags will be approved by two core team members, and we +will closely monitor this and reevaluate if we see a proliferation of flags. + +# Future Possibilities + +As a general rule, using a feature flag should be last resort in the case where +it is impossible to implement backwards compatibility. A feature flag is likely +to get less usage and therefore mature slower, so it's important to make sure we +don't abuse this pattern. + +Still, a valid concern is that we end up with too many feature flags between +major releases (I would say >20 is too many), in which case it might be required +to offer additional tools to manage and discover them. + +Here are a few ideas that came up as we designed this. All of these can be +implemented on top of the proposed mechanism, and should be considered if needed +in the future (as well as any other idea of course): + +- Introduce a CLI command to list all flags and enable/disable them in your + `cdk.json`. +- Aggregate all flags in groups so it will be easier to enable many of them. +- Define a flag that will allow users to say "I want all feature up until a + certain CDK version" (basically enables all features that were available when + the version was releases). +