fix(cli): flags command shows unnecessary flags, not all table rows have vertical lines (#796)

When a user runs `cdk flags`, a table is displayed with information
about their feature flags. This PR omits the intentionally unconfigured
feature flags from the table and makes the table more visually
appealing.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

---------

Co-authored-by: Rico Hermans <[email protected]>

Author: vivian12300

packages/@aws-cdk/cloudformation-diff/lib/format-table.ts

@@ -7,12 +7,15 @@ import * as table from 'table';
  *
  * First row is considered the table header.
  */
-export function formatTable(cells: string[][], columns: number | undefined): string {
+export function formatTable(cells: string[][], columns: number | undefined, noHorizontalLines?: boolean): string {
   return table.table(cells, {
     border: TABLE_BORDER_CHARACTERS,
     columns: buildColumnConfig(columns !== undefined ? calculateColumnWidths(cells, columns) : undefined),
     drawHorizontalLine: (line) => {
       // Numbering like this: [line 0] [header = row[0]] [line 1] [row 1] [line 2] [content 2] [line 3]
+      if (noHorizontalLines) {
+        return line < 2 || line === cells.length;
+      }
       return (line < 2 || line === cells.length) || lineBetween(cells[line - 1], cells[line]);
     },
   }).trimRight();

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

@@ -1276,29 +1276,46 @@ export class Toolkit extends CloudAssemblySourceBuilder {
   /**
    * Retrieve feature flag information from the cloud assembly
    */
-
   public async flags(cx: ICloudAssemblySource): Promise<FeatureFlag[]> {
     this.requireUnstableFeature('flags');
 
     const ioHelper = asIoHelper(this.ioHost, 'flags');
     await using assembly = await assemblyFromSource(ioHelper, cx);
-    const artifacts = assembly.cloudAssembly.manifest.artifacts;
+    const artifacts = Object.values(assembly.cloudAssembly.manifest.artifacts ?? {});
+    const featureFlagReports = artifacts.filter(a => a.type === ArtifactType.FEATURE_FLAG_REPORT);
+
+    const flags = featureFlagReports.flatMap(report => {
+      const properties = report.properties as FeatureFlagReportProperties;
+      const moduleName = properties.module;
+
+      const flagsWithUnconfiguredBehavesLike = Object.entries(properties.flags)
+        .filter(([_, flagInfo]) => flagInfo.unconfiguredBehavesLike != undefined);
 
-    return Object.values(artifacts!)
-      .filter(a => a.type === ArtifactType.FEATURE_FLAG_REPORT)
-      .flatMap(report => {
-        const properties = report.properties as FeatureFlagReportProperties;
-        const moduleName = properties.module;
+      const shouldIncludeUnconfiguredBehavesLike = flagsWithUnconfiguredBehavesLike.length > 0;
 
-        return Object.entries(properties.flags).map(([flagName, flagInfo]) => ({
+      return Object.entries(properties.flags).map(([flagName, flagInfo]) => {
+        const baseFlag = {
           module: moduleName,
           name: flagName,
           recommendedValue: flagInfo.recommendedValue,
           userValue: flagInfo.userValue ?? undefined,
           explanation: flagInfo.explanation ?? '',
-          unconfiguredBehavesLike: flagInfo.unconfiguredBehavesLike,
-        }));
+        };
+
+        if (shouldIncludeUnconfiguredBehavesLike) {
+          return {
+            ...baseFlag,
+            unconfiguredBehavesLike: {
+              v2: flagInfo.unconfiguredBehavesLike?.v2 ?? false,
+            },
+          };
+        }
+
+        return baseFlag;
       });
+    });
+
+    return flags;
   }
 
   private requireUnstableFeature(requestedFeature: UnstableFeature) {

packages/aws-cdk/lib/commands/flag-operations.ts

@@ -1,4 +1,5 @@
 import * as path from 'path';
+import { formatTable } from '@aws-cdk/cloudformation-diff';
 import type { FeatureFlag, Toolkit } from '@aws-cdk/toolkit-lib';
 import { CdkAppMultiContext, MemoryContext, DiffMethod } from '@aws-cdk/toolkit-lib';
 import * as chalk from 'chalk';
@@ -92,7 +93,7 @@ export async function handleFlags(flagData: FeatureFlag[], ioHelper: IoHelper, o
         default: true,
         unconfigured: true,
       };
-      await setMultipleFlags(params);
+      await setMultipleFlagsIfSupported(params);
     } else if (answer == FlagsMenuOptions.MODIFY_SPECIFIC_FLAG) {
       await setFlag(params, true);
     } else if (answer == FlagsMenuOptions.EXIT) {
@@ -172,8 +173,7 @@ export async function handleFlags(flagData: FeatureFlag[], ioHelper: IoHelper, o
   }
 
   if (options.set && options.all && options.default) {
-    await setMultipleFlags(params);
-    return;
+    await setMultipleFlagsIfSupported(params);
   }
 
   if (options.set && options.unconfigured && options.recommended) {
@@ -182,9 +182,20 @@ export async function handleFlags(flagData: FeatureFlag[], ioHelper: IoHelper, o
   }
 
   if (options.set && options.unconfigured && options.default) {
+    await setMultipleFlagsIfSupported(params);
+  }
+}
+
+/**
+ * Sets flag configurations to default values if `unconfiguredBehavesLike` is populated
+ */
+async function setMultipleFlagsIfSupported(params: FlagOperationsParams) {
+  const { flagData, ioHelper } = params;
+  if (flagData[0].unconfiguredBehavesLike) {
     await setMultipleFlags(params);
     return;
   }
+  await ioHelper.defaults.error('The --default options are not compatible with the AWS CDK library used by your application. Please upgrade to 2.212.0 or above.');
 }
 
 async function setFlag(params: FlagOperationsParams, interactive?: boolean) {
@@ -378,38 +389,6 @@ async function modifyValues(params: FlagOperationsParams, flagNames: string[]):
   await fs.writeFile(cdkJsonPath, JSON.stringify(cdkJson, null, 2), 'utf-8');
 }
 
-function formatTable(headers: string[], rows: string[][]): string {
-  const columnWidths = [
-    Math.max(headers[0].length, ...rows.map(row => row[0].length)),
-    Math.max(headers[1].length, ...rows.map(row => row[1].length)),
-    Math.max(headers[2].length, ...rows.map(row => row[2].length)),
-  ];
-
-  const createSeparator = () => {
-    return '+' + columnWidths.map(width => '-'.repeat(width + 2)).join('+') + '+';
-  };
-
-  const formatRow = (values: string[]) => {
-    return '|' + values.map((value, i) => ` ${value.padEnd(columnWidths[i])} `).join('|') + '|';
-  };
-
-  const separator = createSeparator();
-  let table = separator + '\n';
-  table += formatRow(headers) + '\n';
-  table += separator + '\n';
-
-  rows.forEach(row => {
-    if (row[1] === '' && row[2] === '') {
-      table += ` ${row[0].padEnd(columnWidths[0])} \n`;
-    } else {
-      table += formatRow(row) + '\n';
-    }
-  });
-
-  table += separator;
-  return table;
-}
-
 function getFlagSortOrder(flag: FeatureFlag): number {
   if (flag.userValue === undefined) {
     return 3;
@@ -421,9 +400,9 @@ function getFlagSortOrder(flag: FeatureFlag): number {
 }
 
 async function displayFlagTable(flags: FeatureFlag[], ioHelper: IoHelper): Promise<void> {
-  const headers = ['Feature Flag Name', 'Recommended Value', 'User Value'];
+  const filteredFlags = flags.filter(flag => flag.unconfiguredBehavesLike?.v2 !== flag.recommendedValue);
 
-  const sortedFlags = [...flags].sort((a, b) => {
+  const sortedFlags = [...filteredFlags].sort((a, b) => {
     const orderA = getFlagSortOrder(a);
     const orderB = getFlagSortOrder(b);
 
@@ -437,6 +416,7 @@ async function displayFlagTable(flags: FeatureFlag[], ioHelper: IoHelper): Promi
   });
 
   const rows: string[][] = [];
+  rows.push(['Feature Flag Name', 'Recommended Value', 'User Value']);
   let currentModule = '';
 
   sortedFlags.forEach((flag) => {
@@ -445,13 +425,13 @@ async function displayFlagTable(flags: FeatureFlag[], ioHelper: IoHelper): Promi
       currentModule = flag.module;
     }
     rows.push([
-      flag.name,
+      `  ${flag.name}`,
       String(flag.recommendedValue),
       flag.userValue === undefined ? '<unset>' : String(flag.userValue),
     ]);
   });
 
-  const formattedTable = formatTable(headers, rows);
+  const formattedTable = formatTable(rows, undefined, true);
   await ioHelper.defaults.info(formattedTable);
 }
 

packages/aws-cdk/test/commands/flag-operations.test.ts

@@ -42,6 +42,14 @@ const mockFlagsData: FeatureFlag[] = [
     userValue: 'true',
     explanation: 'Flag that matches recommendation',
   },
+  {
+    module: 'different-module',
+    name: '@aws-cdk/core:anotherMatchingFlag',
+    recommendedValue: 'true',
+    userValue: 'true',
+    explanation: 'Flag that matches recommendation',
+    unconfiguredBehavesLike: { v2: 'true' },
+  },
 ];
 
 function createMockToolkit(): jest.Mocked<Toolkit> {
@@ -121,8 +129,8 @@ describe('displayFlags', () => {
     await displayFlags(params);
 
     const plainTextOutput = output();
-    expect(plainTextOutput).toContain('@aws-cdk/core:testFlag');
-    expect(plainTextOutput).toContain('@aws-cdk/s3:anotherFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/core:testFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/s3:anotherFlag');
   });
 
   test('handles null user values correctly', async () => {
@@ -181,6 +189,19 @@ describe('displayFlags', () => {
     expect(plainTextOutput).toContain('different-module');
   });
 
+  test('does not display flag when unconfigured behavior is the same as recommended behavior', async () => {
+    const params = {
+      flagData: mockFlagsData,
+      toolkit: mockToolkit,
+      ioHelper,
+      all: true,
+    };
+    await displayFlags(params);
+
+    const plainTextOutput = output();
+    expect(plainTextOutput).not.toContain('  @aws-cdk/core:anotherMatchingFlag');
+  });
+
   test('displays single flag details when only one substring match is found', async () => {
     const params = {
       flagData: mockFlagsData,
@@ -221,9 +242,10 @@ describe('displayFlags', () => {
     await displayFlags(params);
 
     const plainTextOutput = output();
-    expect(plainTextOutput).toContain('@aws-cdk/core:testFlag');
-    expect(plainTextOutput).toContain('@aws-cdk/s3:anotherFlag');
-    expect(plainTextOutput).toContain('@aws-cdk/core:matchingFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/core:testFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/s3:anotherFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/core:matchingFlag');
+    expect(plainTextOutput).not.toContain('  @aws-cdk/core:anothermatchingFlag');
   });
 
   test('returns all matching flags if user enters multiple substrings', async () => {
@@ -236,9 +258,10 @@ describe('displayFlags', () => {
     await displayFlags(params);
 
     const plainTextOutput = output();
-    expect(plainTextOutput).toContain('@aws-cdk/core:testFlag');
-    expect(plainTextOutput).toContain('@aws-cdk/core:matchingFlag');
-    expect(plainTextOutput).not.toContain('@aws-cdk/s3:anotherFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/core:testFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/core:matchingFlag');
+    expect(plainTextOutput).not.toContain('  @aws-cdk/s3:anotherFlag');
+    expect(plainTextOutput).not.toContain('  @aws-cdk/core:anothermatchingFlag');
   });
 });
 
@@ -264,8 +287,8 @@ describe('handleFlags', () => {
     await handleFlags(mockFlagsData, ioHelper, options, mockToolkit);
 
     const plainTextOutput = output();
-    expect(plainTextOutput).toContain('@aws-cdk/core:testFlag');
-    expect(plainTextOutput).toContain('@aws-cdk/s3:anotherFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/core:testFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/s3:anotherFlag');
   });
 
   test('displays only differing flags when no specific options are provided', async () => {
@@ -275,9 +298,9 @@ describe('handleFlags', () => {
     await handleFlags(mockFlagsData, ioHelper, options, mockToolkit);
 
     const plainTextOutput = output();
-    expect(plainTextOutput).toContain('@aws-cdk/core:testFlag');
-    expect(plainTextOutput).toContain('@aws-cdk/s3:anotherFlag');
-    expect(plainTextOutput).not.toContain('@aws-cdk/core:matchingFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/core:testFlag');
+    expect(plainTextOutput).toContain('  @aws-cdk/s3:anotherFlag');
+    expect(plainTextOutput).not.toContain('  @aws-cdk/core:matchingFlag');
   });
 
   test('handles flag not found for specific flag query', async () => {
@@ -560,6 +583,65 @@ describe('modifyValues', () => {
   });
 });
 
+describe('checkDefaultBehavior', () => {
+  test('calls setMultipleFlags when unconfiguredBehavesLike is present', async () => {
+    const flagsWithUnconfiguredBehavior: FeatureFlag[] = [
+      {
+        module: 'aws-cdk-lib',
+        name: '@aws-cdk/core:testFlag',
+        recommendedValue: 'true',
+        userValue: undefined,
+        explanation: 'Test flag',
+        unconfiguredBehavesLike: { v2: 'true' },
+      },
+    ];
+
+    const cdkJsonPath = await createCdkJsonFile({});
+    setupMockToolkitForPrototyping(mockToolkit);
+
+    const requestResponseSpy = jest.spyOn(ioHelper, 'requestResponse');
+    requestResponseSpy.mockResolvedValue(true);
+
+    const options: FlagsOptions = {
+      set: true,
+      all: true,
+      default: true,
+    };
+
+    await handleFlags(flagsWithUnconfiguredBehavior, ioHelper, options, mockToolkit);
+
+    expect(mockToolkit.fromCdkApp).toHaveBeenCalled();
+    expect(mockToolkit.synth).toHaveBeenCalled();
+
+    await cleanupCdkJsonFile(cdkJsonPath);
+    requestResponseSpy.mockRestore();
+  });
+
+  test('shows error when unconfiguredBehavesLike is not present', async () => {
+    const flagsWithoutUnconfiguredBehavior: FeatureFlag[] = [
+      {
+        module: 'aws-cdk-lib',
+        name: '@aws-cdk/core:testFlag',
+        recommendedValue: 'true',
+        userValue: undefined,
+        explanation: 'Test flag',
+      },
+    ];
+
+    const options: FlagsOptions = {
+      set: true,
+      all: true,
+      default: true,
+    };
+
+    await handleFlags(flagsWithoutUnconfiguredBehavior, ioHelper, options, mockToolkit);
+
+    const plainTextOutput = output();
+    expect(plainTextOutput).toContain('The --default options are not compatible with the AWS CDK library used by your application.');
+    expect(mockToolkit.fromCdkApp).not.toHaveBeenCalled();
+  });
+});
+
 describe('interactive prompts lead to the correct function calls', () => {
   beforeEach(() => {
     setupMockToolkitForPrototyping(mockToolkit);
@@ -642,11 +724,30 @@ describe('interactive prompts lead to the correct function calls', () => {
     const requestResponseSpy = jest.spyOn(ioHelper, 'requestResponse');
     requestResponseSpy.mockResolvedValue(true);
 
+    const flagsWithUnconfiguredBehavior: FeatureFlag[] = [
+      {
+        module: 'aws-cdk-lib',
+        name: '@aws-cdk/core:testFlag',
+        recommendedValue: 'true',
+        userValue: 'false',
+        explanation: 'Test flag for unit tests',
+        unconfiguredBehavesLike: { v2: 'true' },
+      },
+      {
+        module: 'aws-cdk-lib',
+        name: '@aws-cdk/s3:anotherFlag',
+        recommendedValue: 'false',
+        userValue: undefined,
+        explanation: 'Another test flag',
+        unconfiguredBehavesLike: { v2: 'false' },
+      },
+    ];
+
     const options: FlagsOptions = {
       interactive: true,
     };
 
-    await handleFlags(mockFlagsData, ioHelper, options, mockToolkit);
+    await handleFlags(flagsWithUnconfiguredBehavior, ioHelper, options, mockToolkit);
 
     expect(mockToolkit.fromCdkApp).toHaveBeenCalledTimes(2);
     expect(mockToolkit.synth).toHaveBeenCalledTimes(2);