Skip to content

Commit 315d5e4

Browse files
committed
fix(cli): flag report is inconsistent with warnings
The default flag report (showing the unconfigured flags) was basing its entries on a different filter than the message that says "N flags are unconfigured". This is confusing. Currently, init-ing a project and then upgrading to the latest library shows: ``` 2 feature flags are not configured. Run 'cdk flags --unstable=flags' to learn more. ``` But then running that commands shows 15 flags: ``` $ cdk flags --unstable=flags ┌─────────────────────────────────────────────────────────────────────────────────────┬─────────────┬─────────┬───────────┐ │ Feature Flag Name │ Recommended │ User │ Effective │ ├─────────────────────────────────────────────────────────────────────────────────────┼─────────────┼─────────┼───────────┤ │ Module: aws-cdk-lib │ │ │ │ │ @aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId │ true │ <unset> │ true │ │ @aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021 │ true │ <unset> │ true │ │ @aws-cdk/aws-ec2-alpha:useResourceIdForVpcV2Migration │ false │ <unset> │ false │ │ @aws-cdk/aws-ecs-patterns:uniqueTargetGroupId │ true │ <unset> │ false │ │ @aws-cdk/aws-elasticloadbalancingv2:networkLoadBalancerWithSecurityGroupByDefault │ true │ <unset> │ false │ │ @aws-cdk/aws-lambda:recognizeVersionProps │ true │ <unset> │ true │ │ @aws-cdk/aws-rds:lowercaseDbIdentifier │ true │ <unset> │ true │ │ @aws-cdk/aws-stepfunctions-tasks:httpInvokeDynamicJsonPathEndpoint │ true │ <unset> │ true │ ...etc... ``` The reason is those 2 bits of code used different filtering. Specifically, the warning excluded flags for which the default value is equal to the recommended value (i.e., flags that don't need to be configured at all because the default value is good enough), while the table was looking strictly for "unconfigured" flags. The `default == recommended` pattern crops up in a couple of situations: - Security-related fixes that we want to force on people, but want to give them a flag to back out of the changes if they really need to. - Flags that changed their default value in the most recent major version. - Flags that we've introduced at some point in the past, but have gone back on. In this PR, make both bits of code use the same logic so discrepancies like these can't happen anymore. Also add a header to the table so it's clear what you're currently looking at. Also in this PR: remove a dead `flags.ts` code, that had been fully refactored to other files but had accidentally been left in.
1 parent 94cce1e commit 315d5e4

File tree

5 files changed

+65
-531
lines changed

5 files changed

+65
-531
lines changed

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ import {
5454
} from '../commands/migrate';
5555
import type { CloudAssembly, CloudExecutable, StackSelector } from '../cxapp';
5656
import { DefaultSelection, environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob } from '../cxapp';
57-
import { OBSOLETE_FLAGS } from '../obsolete-flags';
5857
import {
5958
deserializeStructure,
6059
formatErrorMessage,
@@ -68,6 +67,7 @@ import { canCollectTelemetry } from './telemetry/collect-telemetry';
6867
import { cdkCliErrorName } from './telemetry/error';
6968
import { CLI_PRIVATE_SPAN } from './telemetry/messages';
7069
import type { ErrorDetails } from './telemetry/schema';
70+
import { FlagOperations } from '../commands/flags/operations';
7171

7272
// Must use a require() otherwise esbuild complains about calling a namespace
7373
// eslint-disable-next-line @typescript-eslint/no-require-imports,@typescript-eslint/consistent-type-imports
@@ -2130,12 +2130,9 @@ export async function displayFlagsMessage(ioHost: IoHelper, toolkit: InternalToo
21302130
return;
21312131
}
21322132

2133-
const unconfiguredFlags = flags
2134-
.filter(flag => !OBSOLETE_FLAGS.includes(flag.name))
2135-
.filter(flag => (flag.unconfiguredBehavesLike?.v2 ?? false) !== flag.recommendedValue)
2136-
.filter(flag => flag.userValue === undefined);
2137-
2133+
const unconfiguredFlags = FlagOperations.filterNeedsAttention(flags);
21382134
const numUnconfigured = unconfiguredFlags.length;
2135+
21392136
if (numUnconfigured > 0) {
21402137
await ioHost.defaults.warn(`${numUnconfigured} feature flags are not configured. Run 'cdk flags --unstable=flags' to learn more.`);
21412138
}

0 commit comments

Comments
 (0)