Skip to content

Commit 566f6ca

Browse files
authored
fix(aws-cdk-lib): feature flag report contains unnecessary flags (#35227)
The feature flag report generated by the core library includes flags that: - Are not configurable for CDKv2 anymore - Whose default value is the same as the recommended value (i.e., they don't need to be configured at all). At the same time, the `recommended-feature-flags.json` file does NOT contain those flags, because they aren't necessary. This means that a newly `cdk init`ed application reports that 18 flags are unconfigured by the user, producing unnecessary warnings. We will address this in 2 parts. - Don't emit flags that don't apply in v2 in the library. - Don't warn about flags where the recommended value is the same as the default value in the CLI. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent f4ab336 commit 566f6ca

File tree

3 files changed

+29
-10
lines changed

3 files changed

+29
-10
lines changed

packages/aws-cdk-lib/core/lib/private/feature-flag-report.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ function parseFeatureFlagInfo(flagName: string, info: FlagInfo, root: IConstruct
3535
export function generateFeatureFlagReport(builder: CloudAssemblyBuilder, root: IConstruct): void {
3636
const featureFlags: Record<string, FeatureFlag> = {};
3737
for (const [flagName, flagInfo] of Object.entries(feats.FLAGS)) {
38+
// Skip flags that don't apply to the current version line
39+
if (feats.CURRENT_VERSION_EXPIRED_FLAGS.includes(flagName)) {
40+
continue;
41+
}
42+
3843
featureFlags[flagName] = parseFeatureFlagInfo(flagName, flagInfo, root);
3944
}
4045

packages/aws-cdk-lib/core/test/feature-flag-report.test.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ describe('generate feature flag report', () => {
1313
const app = new App({
1414
context: {
1515
'@aws-cdk/aws-ec2:bastionHostUseAmazonLinux2023ByDefault': true,
16-
'@aws-cdk/core:aspectStabilization': false,
16+
'@aws-cdk/core:newStyleStackSynthesis': false,
1717
},
1818
});
1919
const assembly = app.synth();
@@ -25,9 +25,14 @@ describe('generate feature flag report', () => {
2525
flags: expect.objectContaining({
2626
'@aws-cdk/aws-ec2:bastionHostUseAmazonLinux2023ByDefault': expect.objectContaining({
2727
userValue: true,
28+
recommendedValue: true,
2829
}),
29-
'@aws-cdk/core:aspectStabilization': expect.objectContaining({
30+
'@aws-cdk/core:newStyleStackSynthesis': expect.objectContaining({
3031
userValue: false,
32+
recommendedValue: true,
33+
unconfiguredBehavesLike: {
34+
v2: true,
35+
},
3136
}),
3237
}),
3338
}),
@@ -47,4 +52,15 @@ describe('generate feature flag report', () => {
4752
}),
4853
}));
4954
});
55+
test('expired flags are not included in report', () => {
56+
const app = new App();
57+
const builder = new cxapi.CloudAssemblyBuilder('/tmp/test');
58+
const spy = jest.spyOn(builder, 'addArtifact');
59+
60+
generateFeatureFlagReport(builder, app);
61+
62+
const flags = (spy.mock.calls[0][1].properties as FeatureFlagReportProperties).flags;
63+
// For example
64+
expect(Object.keys(flags)).not.toContain('aws-cdk:enableDiffNoFail');
65+
});
5066
});

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,18 +1696,16 @@ export const CURRENT_VERSION_EXPIRED_FLAGS: string[] = Object.entries(FLAGS)
16961696
/**
16971697
* Flag values that should apply for new projects
16981698
*
1699-
* Add a flag in here (typically with the value `true`), to enable
1700-
* backwards-breaking behavior changes only for new projects. New projects
1701-
* generated through `cdk init` will include these flags in their generated
1702-
* project config.
1699+
* This contains flags that satisfy both criteria of:
17031700
*
1704-
* Tests must cover the default (disabled) case and the future (enabled) case.
1705-
*
1706-
* Going forward, this should *NOT* be consumed directly anymore.
1701+
* - They are configurable for the current major version line.
1702+
* - The recommended value is different from the unconfigured value (i.e.,
1703+
* configuring a flag is useful)
17071704
*/
17081705
export const CURRENTLY_RECOMMENDED_FLAGS = Object.fromEntries(
17091706
Object.entries(FLAGS)
1710-
.filter(([_, flag]) => flag.recommendedValue !== flag.unconfiguredBehavesLike?.[CURRENT_MV] && flag.introducedIn[CURRENT_MV])
1707+
.filter(([_, flag]) => flag.introducedIn[CURRENT_MV] !== undefined)
1708+
.filter(([_, flag]) => flag.recommendedValue !== flag.unconfiguredBehavesLike?.[CURRENT_MV])
17111709
.map(([name, flag]) => [name, flag.recommendedValue]),
17121710
);
17131711

0 commit comments

Comments
 (0)