Skip to content

Commit 6693243

Browse files
authored
fix(cli): flag report is inconsistent with warnings (#967)
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. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 33464fc commit 6693243

File tree

6 files changed

+117
-587
lines changed

6 files changed

+117
-587
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
@@ -2128,12 +2128,9 @@ export async function displayFlagsMessage(ioHost: IoHelper, toolkit: InternalToo
21282128
return;
21292129
}
21302130

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

0 commit comments

Comments
 (0)