Skip to content

Commit dcdb58a

Browse files
authored
chore(cxapi): reduce merge conflicts in feature flags (#18411)
Try to reduce (future) merge conflicts when feature flags are added. It currently looks like `FUTURE_FLAGS_DEFAULTS` should contain all flags, and they're all set to `false`. On the `v2-main` branch, they're all set to `true`. When adding a new flag, you want to follow suit and add a new line with the flag default set to `false`; then you get a merge conflict when merging to `v2-main` branch because all preceding lines will have changed. The merge conflict alone is annoying, and you'll also be tempted to put in `true` there, which would be incorrect, and be a breaking change of behavior. Instead, this PR gets rid of the entire set of `FUTURE_FLAGS_DEFAULTS` set to `false` -- there's no point to having them anyway, and it gets rid of the associated merge conflicts. Also shore up the docs for these flags a little. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent d3b44ac commit dcdb58a

File tree

3 files changed

+44
-36
lines changed

3 files changed

+44
-36
lines changed

packages/@aws-cdk/core/test/feature-flags.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('feature flags', () => {
2323
test('invalid flag', () => {
2424
const stack = new Stack();
2525

26-
expect(FeatureFlags.of(stack).isEnabled('non-existent-flag')).toEqual(undefined);
26+
expect(FeatureFlags.of(stack).isEnabled('non-existent-flag')).toEqual(false);
2727

2828
});
2929
});

packages/@aws-cdk/cx-api/lib/features.ts

Lines changed: 41 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,18 @@
33
// implemented behind a flag in order to preserve backwards compatibility for
44
// existing apps. When a new app is initialized through `cdk init`, the CLI will
55
// automatically add enable these features by adding them to the generated
6-
// `cdk.json` file. In the next major release of the CDK, these feature flags
7-
// will be removed and will become the default behavior.
6+
// `cdk.json` file.
7+
//
8+
// Some of these flags only affect the behavior of the construct library --
9+
// these will be removed in the next major release and the behavior they are
10+
// gating will become the only behavior.
11+
//
12+
// Other flags also affect the generated CloudFormation templates, in a way
13+
// that prevents seamless upgrading. In the next major version, their
14+
// behavior will become the default, but the flag still exists so users can
15+
// switch it *off* in order to revert to the old behavior. These flags
16+
// are marked with with the [PERMANENT] tag below.
17+
//
818
// See https://github.com/aws/aws-cdk-rfcs/blob/master/text/0055-feature-flags.md
919
// --------------------------------------------------------------------------------
1020

@@ -31,6 +41,8 @@ export const ENABLE_DIFF_NO_FAIL = ENABLE_DIFF_NO_FAIL_CONTEXT;
3141

3242
/**
3343
* Switch to new stack synthesis method which enable CI/CD
44+
*
45+
* [PERMANENT]
3446
*/
3547
export const NEW_STYLE_STACK_SYNTHESIS_CONTEXT = '@aws-cdk/core:newStyleStackSynthesis';
3648

@@ -41,6 +53,8 @@ export const NEW_STYLE_STACK_SYNTHESIS_CONTEXT = '@aws-cdk/core:newStyleStackSyn
4153
* ensure uniqueness, and makes the export names robust against refactoring
4254
* the location of the stack in the construct tree (specifically, moving the Stack
4355
* into a Stage).
56+
*
57+
* [PERMANENT]
4458
*/
4559
export const STACK_RELATIVE_EXPORTS_CONTEXT = '@aws-cdk/core:stackRelativeExports';
4660

@@ -116,6 +130,8 @@ export const ECS_REMOVE_DEFAULT_DESIRED_COUNT = '@aws-cdk/aws-ecs-patterns:remov
116130
*
117131
* This feature flag make correct the ServerlessCluster.clusterArn when
118132
* clusterIdentifier contains a Upper case letters.
133+
*
134+
* [PERMANENT]
119135
*/
120136
export const RDS_LOWERCASE_DB_IDENTIFIER = '@aws-cdk/aws-rds:lowercaseDbIdentifier';
121137

@@ -132,6 +148,8 @@ export const RDS_LOWERCASE_DB_IDENTIFIER = '@aws-cdk/aws-rds:lowercaseDbIdentifi
132148
*
133149
* In effect, there is no way to get out of this mess in a backwards compatible way, while supporting existing stacks.
134150
* This flag changes the logical id layout of UsagePlanKey to not be sensitive to order.
151+
*
152+
* [PERMANENT]
135153
*/
136154
export const APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID = '@aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId';
137155

@@ -150,13 +168,17 @@ export const EFS_DEFAULT_ENCRYPTION_AT_REST = '@aws-cdk/aws-efs:defaultEncryptio
150168
* not constitute creating a new Version.
151169
*
152170
* See 'currentVersion' section in the aws-lambda module's README for more details.
171+
*
172+
* [PERMANENT]
153173
*/
154174
export const LAMBDA_RECOGNIZE_VERSION_PROPS = '@aws-cdk/aws-lambda:recognizeVersionProps';
155175

156176
/**
157177
* Enable this feature flag to have cloudfront distributions use the security policy TLSv1.2_2021 by default.
158178
*
159179
* The security policy can also be configured explicitly using the `minimumProtocolVersion` property.
180+
*
181+
* [PERMANENT]
160182
*/
161183
export const CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021 = '@aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021';
162184

@@ -167,6 +189,8 @@ export const CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021 = '@aws-cdk/aws-cl
167189
* of unnecessary regions included in stacks without a known region.
168190
*
169191
* The type of this value should be a list of strings.
192+
*
193+
* [PERMANENT]
170194
*/
171195
export const TARGET_PARTITIONS = '@aws-cdk/core:target-partitions';
172196

@@ -175,6 +199,8 @@ export const TARGET_PARTITIONS = '@aws-cdk/core:target-partitions';
175199
* `awslogs` log driver for the application container of the service to send the container logs to CloudWatch Logs.
176200
*
177201
* This is a feature flag as the new behavior provides a better default experience for the users.
202+
*
203+
* [PERMANENT]
178204
*/
179205
export const ECS_SERVICE_EXTENSIONS_ENABLE_DEFAULT_LOG_DRIVER = '@aws-cdk-containers/ecs-service-extensions:enableDefaultLogDriver';
180206

@@ -190,15 +216,11 @@ export const ECS_SERVICE_EXTENSIONS_ENABLE_DEFAULT_LOG_DRIVER = '@aws-cdk-contai
190216
export const EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME = '@aws-cdk/aws-ec2:uniqueImdsv2TemplateName';
191217

192218
/**
193-
* This map includes context keys and values for feature flags that enable
194-
* capabilities "from the future", which we could not introduce as the default
195-
* behavior due to backwards compatibility for existing projects.
196-
*
197-
* New projects generated through `cdk init` will include these flags in their
198-
* generated `cdk.json` file.
219+
* Flag values that should apply for new projects
199220
*
200-
* When we release the next major version of the CDK, we will flip the logic of
201-
* these features and clean up the `cdk.json` generated by `cdk init`.
221+
* Add a flag in here (typically with the value `true`), to enable
222+
* backwards-breaking behavior changes only for new projects. New projects
223+
* generated through `cdk init` will include these flags in their generated
202224
*
203225
* Tests must cover the default (disabled) case and the future (enabled) case.
204226
*/
@@ -218,9 +240,6 @@ export const FUTURE_FLAGS: { [key: string]: boolean } = {
218240
[CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021]: true,
219241
[ECS_SERVICE_EXTENSIONS_ENABLE_DEFAULT_LOG_DRIVER]: true,
220242
[EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME]: true,
221-
222-
// We will advertise this flag when the feature is complete
223-
// [NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: 'true',
224243
};
225244

226245
/**
@@ -238,28 +257,17 @@ export const FUTURE_FLAGS_EXPIRED: string[] = [
238257
];
239258

240259
/**
241-
* The set of defaults that should be applied if the feature flag is not
242-
* explicitly configured.
260+
* The default values of each of these flags.
261+
*
262+
* This is the effective value of the flag, unless it's overriden via
263+
* context.
264+
*
265+
* Adding new flags here is only allowed during the pre-release period of a new
266+
* major version!
243267
*/
244268
const FUTURE_FLAGS_DEFAULTS: { [key: string]: boolean } = {
245-
[APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID]: false,
246-
[ENABLE_STACK_NAME_DUPLICATES_CONTEXT]: false,
247-
[ENABLE_DIFF_NO_FAIL_CONTEXT]: false,
248-
[STACK_RELATIVE_EXPORTS_CONTEXT]: false,
249-
[NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: false,
250-
[DOCKER_IGNORE_SUPPORT]: false,
251-
[SECRETS_MANAGER_PARSE_OWNED_SECRET_NAME]: false,
252-
[KMS_DEFAULT_KEY_POLICIES]: false,
253-
[S3_GRANT_WRITE_WITHOUT_ACL]: false,
254-
[ECS_REMOVE_DEFAULT_DESIRED_COUNT]: false,
255-
[RDS_LOWERCASE_DB_IDENTIFIER]: false,
256-
[EFS_DEFAULT_ENCRYPTION_AT_REST]: false,
257-
[LAMBDA_RECOGNIZE_VERSION_PROPS]: false,
258-
[CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021]: false,
259-
[ECS_SERVICE_EXTENSIONS_ENABLE_DEFAULT_LOG_DRIVER]: false,
260-
[EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME]: false,
261269
};
262270

263-
export function futureFlagDefault(flag: string): boolean | undefined {
264-
return FUTURE_FLAGS_DEFAULTS[flag];
271+
export function futureFlagDefault(flag: string): boolean {
272+
return FUTURE_FLAGS_DEFAULTS[flag] ?? false;
265273
}

packages/@aws-cdk/cx-api/test/features.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ test('all future flags have defaults configured', () => {
77
});
88
});
99

10-
test('futureFlagDefault returns undefined if non existent flag was given', () => {
11-
expect(feats.futureFlagDefault('non-existent-flag')).toEqual(undefined);
10+
test('futureFlagDefault returns false if non existent flag was given', () => {
11+
expect(feats.futureFlagDefault('non-existent-flag')).toEqual(false);
1212
});
1313

1414
testLegacyBehavior('FUTURE_FLAGS_EXPIRED must be empty in CDKv1', Object, () => {

0 commit comments

Comments
 (0)