feat(cli): can match notices against Node version (#128)

This allows publishing notices that match against the version of Node
we're executing on.

Implemented by generalizing the code that matches against CLI version
and bootstrap versions: it can now match against arbitrary named
components.

Also implement more complex matching rules: currently we match the
pattern if one of them matches:

```ts
// Matches if one of A, B or C matches
components: [A, B, C]
```

Instead, generalize to Disjunctive Normal Form and treat the current
case as a special case of DNF where every conjunction has one element:

```ts
// The above gets interpreted as
components: [[A], [B], [C]]

// More complex rules: A and B together, or C
components: [[A, B], [C]]
```

This way we can write rules to say that "component X on Node version Y"
should get a notice.

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

Author: rix0rrr

packages/aws-cdk/lib/notices.ts

@@ -11,8 +11,7 @@ import type { Context } from './api/context';
 import { versionNumber } from './cli/version';
 import { debug, info, warning, error } from './logging';
 import { ToolkitError } from './toolkit/error';
-import { loadTreeFromDir, some } from './tree';
-import { flatMap } from './util';
+import { ConstructTreeNode, loadTreeFromDir } from './tree';
 import { cdkCacheDir } from './util/directories';
 import { formatErrorMessage } from './util/format-error';
 
@@ -84,116 +83,189 @@ export interface NoticesFilterFilterOptions {
   readonly bootstrappedEnvironments: BootstrappedEnvironment[];
 }
 
-export class NoticesFilter {
+export abstract class NoticesFilter {
   public static filter(options: NoticesFilterFilterOptions): FilteredNotice[] {
-    return [
-      ...this.findForCliVersion(options.data, options.cliVersion),
-      ...this.findForFrameworkVersion(options.data, options.outDir),
-      ...this.findForBootstrapVersion(options.data, options.bootstrappedEnvironments),
+    const components = [
+      ...NoticesFilter.constructTreeComponents(options.outDir),
+      ...NoticesFilter.otherComponents(options),
     ];
+
+    return NoticesFilter.findForNamedComponents(options.data, components);
   }
 
-  private static findForCliVersion(data: Notice[], cliVersion: string): FilteredNotice[] {
-    return flatMap(data, notice => {
-      const affectedComponent = notice.components.find(component => component.name === 'cli');
-      const affectedRange = affectedComponent?.version;
+  /**
+   * From a set of input options, return the notices components we are searching for
+   */
+  private static otherComponents(options: NoticesFilterFilterOptions): ActualComponent[] {
+    return [
+      // CLI
+      {
+        name: 'cli',
+        version: options.cliVersion,
+      },
+
+      // Node version
+      {
+        name: 'node',
+        version: process.version.replace(/^v/, ''), // remove the 'v' prefix.
+        dynamicName: 'node',
+      },
+
+      // Bootstrap environments
+      ...options.bootstrappedEnvironments.flatMap(env => {
+        const semverBootstrapVersion = semver.coerce(env.bootstrapStackVersion);
+        if (!semverBootstrapVersion) {
+          // we don't throw because notices should never crash the cli.
+          warning(`While filtering notices, could not coerce bootstrap version '${env.bootstrapStackVersion}' into semver`);
+          return [];
+        }
 
-      if (affectedRange == null) {
-        return [];
-      }
+        return [{
+          name: 'bootstrap',
+          version: `${semverBootstrapVersion}`,
+          dynamicName: 'ENVIRONMENTS',
+          dynamicValue: env.environment.name,
+        }];
+      }),
+    ];
+  }
 
-      if (!semver.satisfies(cliVersion, affectedRange)) {
-        return [];
+  /**
+   * Based on a set of component names, find all notices that match one of the given components
+   */
+  private static findForNamedComponents(data: Notice[], actualComponents: ActualComponent[]): FilteredNotice[] {
+    return data.flatMap(notice => {
+      const ors = this.resolveAliases(normalizeComponents(notice.components));
+
+      // Find the first set of the disjunctions of which all components match against the actual components.
+      // Return the actual components we found so that we can inject their dynamic values. A single filter
+      // component can match more than one actual component
+      for (const ands of ors) {
+        const matched = ands.map(affected => actualComponents.filter(actual =>
+          NoticesFilter.componentNameMatches(affected, actual) && semver.satisfies(actual.version, affected.version)));
+
+        // For every clause in the filter we matched one or more components
+        if (matched.every(xs => xs.length > 0)) {
+          const ret = new FilteredNotice(notice);
+          NoticesFilter.addDynamicValues(matched.flatMap(x => x), ret);
+          return [ret];
+        }
       }
 
-      return [new FilteredNotice(notice)];
+      return [];
     });
   }
 
-  private static findForFrameworkVersion(data: Notice[], outDir: string): FilteredNotice[] {
-    const tree = loadTreeFromDir(outDir);
-    return flatMap(data, notice => {
-      //  A match happens when:
-      //
-      //  1. The version of the node matches the version in the notice, interpreted
-      //  as a semver range.
-      //
-      //  AND
-      //
-      //  2. The name in the notice is a prefix of the node name when the query ends in '.',
-      //  or the two names are exactly the same, otherwise.
-
-      const matched = some(tree, node => {
-        return this.resolveAliases(notice.components).some(component =>
-          compareNames(component.name, node.constructInfo?.fqn) &&
-          compareVersions(component.version, node.constructInfo?.version));
-      });
-
-      if (!matched) {
-        return [];
-      }
-
-      return [new FilteredNotice(notice)];
+  /**
+   * Whether the given "affected component" name applies to the given actual component name.
+   *
+   * The name matches if the name is exactly the same, or the name in the notice
+   * is a prefix of the node name when the query ends in '.'.
+   */
+  private static componentNameMatches(pattern: Component, actual: ActualComponent): boolean {
+    return pattern.name.endsWith('.') ? actual.name.startsWith(pattern.name) : pattern.name === actual.name;
+  }
 
-      function compareNames(pattern: string, target: string | undefined): boolean {
-        if (target == null) {
-          return false;
-        }
-        return pattern.endsWith('.') ? target.startsWith(pattern) : pattern === target;
+  /**
+   * Adds dynamic values from the given ActualComponents
+   *
+   * If there are multiple components with the same dynamic name, they are joined
+   * by a comma.
+   */
+  private static addDynamicValues(comps: ActualComponent[], notice: FilteredNotice) {
+    const dynamicValues: Record<string, string[]> = {};
+    for (const comp of comps) {
+      if (comp.dynamicName) {
+        dynamicValues[comp.dynamicName] = dynamicValues[comp.dynamicName] ?? [];
+        dynamicValues[comp.dynamicName].push(comp.dynamicValue ?? comp.version);
       }
+    }
+    for (const [key, values] of Object.entries(dynamicValues)) {
+      notice.addDynamicValue(key, values.join(','));
+    }
+  }
 
-      function compareVersions(pattern: string, target: string | undefined): boolean {
-        return semver.satisfies(target ?? '', pattern);
+  /**
+   * Treat 'framework' as an alias for either `aws-cdk-lib.` or `@aws-cdk/core.`.
+   *
+   * Because it's EITHER `aws-cdk-lib` or `@aws-cdk/core`, we need to add multiple
+   * arrays at the top level.
+   */
+  private static resolveAliases(ors: Component[][]): Component[][] {
+    return ors.flatMap(ands => {
+      const hasFramework = ands.find(c => c.name === 'framework');
+      if (!hasFramework) {
+        return [ands];
       }
+
+      return [
+        ands.map(c => c.name === 'framework' ? { ...c, name: '@aws-cdk/core.' } : c),
+        ands.map(c => c.name === 'framework' ? { ...c, name: 'aws-cdk-lib.' } : c),
+      ];
     });
   }
 
-  private static findForBootstrapVersion(data: Notice[], bootstrappedEnvironments: BootstrappedEnvironment[]): FilteredNotice[] {
-    return flatMap(data, notice => {
-      const affectedComponent = notice.components.find(component => component.name === 'bootstrap');
-      const affectedRange = affectedComponent?.version;
+  /**
+   * Load the construct tree from the given directory and return its components
+   */
+  private static constructTreeComponents(manifestDir: string): ActualComponent[] {
+    const tree = loadTreeFromDir(manifestDir);
+    if (!tree) {
+      return [];
+    }
 
-      if (affectedRange == null) {
-        return [];
-      }
+    const ret: ActualComponent[] = [];
+    recurse(tree);
+    return ret;
 
-      const affected = bootstrappedEnvironments.filter(i => {
-        const semverBootstrapVersion = semver.coerce(i.bootstrapStackVersion);
-        if (!semverBootstrapVersion) {
-          // we don't throw because notices should never crash the cli.
-          warning(`While filtering notices, could not coerce bootstrap version '${i.bootstrapStackVersion}' into semver`);
-          return false;
-        }
-
-        return semver.satisfies(semverBootstrapVersion, affectedRange);
-      });
+    function recurse(x: ConstructTreeNode) {
+      if (x.constructInfo?.fqn && x.constructInfo?.version) {
+        ret.push({
+          name: x.constructInfo?.fqn,
+          version: x.constructInfo?.version,
+        });
+      }
 
-      if (affected.length === 0) {
-        return [];
+      for (const child of Object.values(x.children ?? {})) {
+        recurse(child);
       }
+    }
+  }
+}
 
-      const filtered = new FilteredNotice(notice);
-      filtered.addDynamicValue('ENVIRONMENTS', affected.map(s => s.environment.name).join(','));
+interface ActualComponent {
+  /**
+   * Name of the component
+   */
+  readonly name: string;
 
-      return [filtered];
-    });
-  }
+  /**
+   * Version of the component
+   */
+  readonly version: string;
 
-  private static resolveAliases(components: Component[]): Component[] {
-    return flatMap(components, component => {
-      if (component.name === 'framework') {
-        return [{
-          name: '@aws-cdk/core.',
-          version: component.version,
-        }, {
-          name: 'aws-cdk-lib.',
-          version: component.version,
-        }];
-      } else {
-        return [component];
-      }
-    });
-  }
+  /**
+   * If matched, under what name should it be added to the set of dynamic values
+   *
+   * These will be used to substitute placeholders in the message string, where
+   * placeholders look like `{resolve:XYZ}`.
+   *
+   * If there is more than one component with the same dynamic name, they are
+   * joined by ','.
+   *
+   * @default - Don't add to the set of dynamic values.
+   */
+  readonly dynamicName?: string;
+
+  /**
+   * If matched, what we should put in the set of dynamic values insstead of the version.
+   *
+   * Only used if `dynamicName` is set; by default we will add the actual version
+   * of the component.
+   *
+   * @default - The version.
+   */
+  readonly dynamicValue?: string;
 }
 
 /**
@@ -327,18 +399,44 @@ export class Notices {
 
 export interface Component {
   name: string;
+
+  /**
+   * The range of affected versions
+   */
   version: string;
 }
 
 export interface Notice {
   title: string;
   issueNumber: number;
   overview: string;
-  components: Component[];
+  /**
+   * A set of affected components
+   *
+   * The canonical form of a list of components is in Disjunctive Normal Form
+   * (i.e., an OR of ANDs). This is the form when the list of components is a
+   * doubly nested array: the notice matches if all components of at least one
+   * of the top-level array matches.
+   *
+   * If the `components` is a single-level array, it is evaluated as an OR; it
+   * matches if any of the components matches.
+   */
+  components: Array<Component | Component[]>;
   schemaVersion: string;
   severity?: string;
 }
 
+/**
+ * Normalizes the given components structure into DNF form
+ */
+function normalizeComponents(xs: Array<Component | Component[]>): Component[][] {
+  return xs.map(x => Array.isArray(x) ? x : [x]);
+}
+
+function renderConjunction(xs: Component[]): string {
+  return xs.map(c => `${c.name}: ${c.version}`).join(' AND ');
+}
+
 /**
  * Notice after passing the filter. A filter can augment a notice with
  * dynamic values as it has access to the dynamic matching data.
@@ -354,7 +452,7 @@ export class FilteredNotice {
   }
 
   public format(): string {
-    const componentsValue = this.notice.components.map(c => `${c.name}: ${c.version}`).join(', ');
+    const componentsValue = normalizeComponents(this.notice.components).map(renderConjunction).join(', ');
     return this.resolveDynamicValues([
       `${this.notice.issueNumber}\t${this.notice.title}`,
       this.formatOverview(),

packages/aws-cdk/lib/tree.ts

@@ -29,30 +29,30 @@ export interface ConstructTreeNode {
 /**
  * Whether the provided predicate is true for at least one element in the construct (sub-)tree.
  */
-export function some(node: ConstructTreeNode, predicate: (n: ConstructTreeNode) => boolean): boolean {
+export function some(node: ConstructTreeNode | undefined, predicate: (n: ConstructTreeNode) => boolean): boolean {
   return node != null && (predicate(node) || findInChildren());
 
   function findInChildren(): boolean {
-    return Object.values(node.children ?? {}).some(child => some(child, predicate));
+    return Object.values(node?.children ?? {}).some(child => some(child, predicate));
   }
 }
 
-export function loadTree(assembly: CloudAssembly) {
+export function loadTree(assembly: CloudAssembly): ConstructTreeNode | undefined {
   try {
     const outdir = assembly.directory;
     const fileName = assembly.tree()?.file;
     return fileName ? fs.readJSONSync(path.join(outdir, fileName)).tree : {};
   } catch (e) {
     trace(`Failed to get tree.json file: ${e}. Proceeding with empty tree.`);
-    return {};
+    return undefined;
   }
 }
 
-export function loadTreeFromDir(outdir: string) {
+export function loadTreeFromDir(outdir: string): ConstructTreeNode | undefined {
   try {
     return fs.readJSONSync(path.join(outdir, 'tree.json')).tree;
   } catch (e) {
     trace(`Failed to get tree.json file: ${e}. Proceeding with empty tree.`);
-    return {};
+    return undefined;
   }
 }