Skip to content

Commit 168016b

Browse files
authored
fix(cli): displays a nag message for flags that don't need configuring (#792)
The flags nag message is too pessimistic, showing 18 unconfigured flags on a newly cdk inited project. * Part of that is the CDK library emitting info for flags that don't exist anymore, fixed here: aws/aws-cdk#35227 * Part of that is we are alarming on unconfigured flags, even if the behavior of unconfigured flags is the same as recommended behavior. We shouldn't alarm on the latter, so filter them out. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 73184ac commit 168016b

File tree

3 files changed

+46
-5
lines changed

3 files changed

+46
-5
lines changed

packages/@aws-cdk/toolkit-lib/lib/toolkit/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,5 +123,5 @@ export interface FeatureFlag {
123123
readonly recommendedValue: unknown;
124124
readonly userValue?: unknown;
125125
readonly explanation?: string;
126-
readonly unconfiguredBehavesLike?: { [key: string]: any };
126+
readonly unconfiguredBehavesLike?: { v2?: any };
127127
}

packages/aws-cdk/lib/cli/cdk-toolkit.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2113,11 +2113,29 @@ async function askUserConfirmation(
21132113
});
21142114
}
21152115

2116+
/**
2117+
* Display a warning if there are flags that are different from the recommended value
2118+
*
2119+
* This happens if both of the following are true:
2120+
*
2121+
* - The user didn't configure the value
2122+
* - The default value for the flag (unconfiguredBehavesLike) is different from the recommended value
2123+
*/
21162124
export async function displayFlagsMessage(ioHost: IoHelper, toolkit: InternalToolkit, cloudExecutable: CloudExecutable): Promise<void> {
2117-
let numUnconfigured = (await toolkit.flags(cloudExecutable))
2125+
const flags = await toolkit.flags(cloudExecutable);
2126+
2127+
// The "unconfiguredBehavesLike" information got added later. If none of the flags have this information,
2128+
// we don't have enough information to reliably display this information without scaring users, so don't do anything.
2129+
if (flags.every(flag => flag.unconfiguredBehavesLike === undefined)) {
2130+
return;
2131+
}
2132+
2133+
const unconfiguredFlags = flags
21182134
.filter(flag => !OBSOLETE_FLAGS.includes(flag.name))
2119-
.filter(flag => flag.userValue === undefined).length;
2135+
.filter(flag => (flag.unconfiguredBehavesLike?.v2 ?? false) !== flag.recommendedValue)
2136+
.filter(flag => flag.userValue === undefined);
21202137

2138+
const numUnconfigured = unconfiguredFlags.length;
21212139
if (numUnconfigured > 0) {
21222140
await ioHost.defaults.warn(`${numUnconfigured} feature flags are not configured. Run 'cdk --unstable=flags flags' to learn more.`);
21232141
}

packages/aws-cdk/test/cli/display-flags-message.test.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,23 @@ describe('displayFlagsMessage', () => {
3030
recommendedValue: 'true',
3131
explanation: 'Test flag',
3232
module: 'aws-cdk-lib',
33+
unconfiguredBehavesLike: { v2: true },
3334
},
3435
{
3536
name: '@aws-cdk/s3:anotherFlag',
3637
userValue: 'false',
3738
recommendedValue: 'false',
3839
explanation: 'Another test flag',
3940
module: 'aws-cdk-lib',
41+
unconfiguredBehavesLike: { v2: true },
4042
},
4143
{
4244
name: '@aws-cdk/core:enableStackNameDuplicates',
4345
userValue: undefined,
4446
recommendedValue: 'true',
4547
explanation: 'Obsolete flag',
4648
module: 'aws-cdk-lib',
49+
unconfiguredBehavesLike: { v2: true },
4750
},
4851
];
4952

@@ -56,14 +59,16 @@ describe('displayFlagsMessage', () => {
5659
message: expect.stringContaining('1 feature flags are not configured'),
5760
}));
5861
});
62+
5963
test('does not display a message when user has no unconfigured flags', async () => {
6064
const mockFlagsData = [
6165
{
6266
name: '@aws-cdk/s3:anotherFlag',
63-
userValue: 'false',
64-
recommendedValue: 'false',
67+
userValue: false,
68+
recommendedValue: false,
6569
explanation: 'Another test flag',
6670
module: 'aws-cdk-lib',
71+
unconfiguredBehavesLike: { v2: true },
6772
},
6873
];
6974
mockToolkit.flags.mockResolvedValue(mockFlagsData);
@@ -73,5 +78,23 @@ describe('displayFlagsMessage', () => {
7378
expect(mockToolkit.flags).toHaveBeenCalledWith(mockCloudExecutable);
7479
expect(ioHost.notifySpy).not.toHaveBeenCalled();
7580
});
81+
82+
test('does not display a message when unconfigured behaviors is the same as recommended behavior', async () => {
83+
const mockFlagsData = [
84+
{
85+
name: '@aws-cdk/s3:anotherFlag',
86+
// userValue is undefined, meaning it is unconfigured
87+
recommendedValue: false,
88+
explanation: 'Another test flag',
89+
module: 'aws-cdk-lib',
90+
unconfiguredBehavesLike: { v2: false },
91+
},
92+
];
93+
mockToolkit.flags.mockResolvedValue(mockFlagsData);
94+
95+
await displayFlagsMessage(ioHost.asHelper(), mockToolkit as any, mockCloudExecutable);
96+
97+
expect(ioHost.notifySpy).not.toHaveBeenCalled();
98+
});
7699
});
77100

0 commit comments

Comments
 (0)