fix(core): stack-related validation has an empty construct path (#38350)

CloudFormation Validate violations that trace to the stack, or to a non-resource construct, have empty construct information. Instead, trace them to the containing stack.


----

*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/core/lib/private/synthesis-validation.ts

@@ -329,6 +329,7 @@ function doInvokeValidationPlugins(
       try {
         const report = makeTemplatePathsRelative(plugin.validate({
           templatePaths: stacks.map(s => s.templateFullPath),
+          stackTemplates: stacks.map(s => ({ stackConstructPath: s.hierarchicalId, templatePath: s.templateFullPath })),
           appConstruct: root,
           accountId: accountId !== cxapi.UNKNOWN_ACCOUNT ? accountId : undefined,
           region: region !== cxapi.UNKNOWN_REGION ? region : undefined,

packages/aws-cdk-lib/core/lib/stage.ts

@@ -183,6 +183,7 @@ export class Stage extends Construct {
    * synthesis will be interrupted and the report displayed to the user.
    *
    * @default - no validation plugins are used
+   * @deprecated Do not use this function to look up validation plugins. Use `Validations.of(stage).plugins` instead.
    */
   public get policyValidationBeta1(): IPolicyValidationPluginBeta1[] {
     return this._policyValidation.map(_toBeta1Plugin);

packages/aws-cdk-lib/core/lib/validation/cloudformation-validate-plugin.ts

@@ -102,12 +102,9 @@ export class CloudFormationValidatePlugin implements IPolicyValidationPlugin {
   public validate(context: IPolicyValidationContext): PolicyValidationPluginReport {
     const violations: MutableViolation[] = [];
 
-    for (const templatePath of context.templatePaths) {
+    for (const { stackConstructPath, templatePath } of context.stackTemplates) {
       const templateFile = new TemplateFile(templatePath);
-      const report = this.engine.validateStandard(templateFile, {
-        // Environment-agnostic stacks use these cx-api sentinels in the cloud assembly. Omitting
-        // them lets the engine model the pseudo-parameters symbolically instead of validating the
-        // sentinel text as if it were a real account or region.
+      const report = this.engine.validateDetailed(templateFile, {
         pseudoParameterOverrides: {
           accountId: context.accountId,
           region: context.region,
@@ -132,6 +129,8 @@ export class CloudFormationValidatePlugin implements IPolicyValidationPlugin {
 
         const violatingResource: PolicyViolatingResource = {
           resourceLogicalId: diagnostic.resourceId,
+          // If this is not about any resources, best we can do is point it to the stack
+          constructPath: !diagnostic.resourceId ? stackConstructPath : undefined,
           templatePath,
           locations: diagnostic.propertyPath ? [diagnostic.propertyPath] : [],
         };

packages/aws-cdk-lib/core/lib/validation/validation.ts

@@ -72,6 +72,11 @@ export interface IPolicyValidationContext {
    */
   readonly templatePaths: string[];
 
+  /**
+   * The absolute path of all templates to be processed, along with the stack construct path for each template.
+   */
+  readonly stackTemplates: PolicyValidationStack[];
+
   /**
    * The account ID for these templates, if known
    */
@@ -93,6 +98,21 @@ export interface IPolicyValidationContext {
   readonly appConstruct: IConstruct;
 }
 
+/**
+ * Information about a single stack that is being validated.
+ */
+export interface PolicyValidationStack {
+  /**
+   * The Stack's construct path
+   */
+  readonly stackConstructPath: string;
+
+  /**
+   * The path to the template file on disk
+   */
+  readonly templatePath: string;
+}
+
 /**
  * Represents a validation plugin that will be executed during synthesis
  *
@@ -171,6 +191,8 @@ export function _toBeta1Plugin(plugin: IPolicyValidationPlugin): IPolicyValidati
     validate(context: IPolicyValidationContextBeta1): PolicyValidationPluginReportBeta1 {
       const report = plugin.validate({
         ...context,
+        // This is incorrect information -- it doesn't matter, this function shouldn't be used regardless.
+        stackTemplates: [],
         accountId: undefined,
         region: undefined,
       });

packages/aws-cdk-lib/core/test/validation/cloudformation-validate-plugin.test.ts

@@ -1,6 +1,7 @@
 import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
+import type { PolicyValidationReportJson } from '@aws-cdk/cloud-assembly-schema';
 import { Construct } from 'constructs';
 import * as cxapi from '../../../cx-api';
 import * as core from '../../lib';
@@ -94,9 +95,9 @@ describe('CloudFormationValidatePlugin', () => {
     const asm = app.synth();
 
     // THEN
-    const validationReport = loadJson(path.join(asm.directory, 'validation-report.json'));
+    const validationReport = loadValidationReport(asm);
     const report = validationReport.pluginReports.find((r: any) => r.pluginName === 'CloudFormation Validate');
-    expect(report.violations).toEqual([
+    expect(report?.violations).toEqual([
       expect.objectContaining({
         ruleName: 'F3002',
         violatingConstructs: [
@@ -134,6 +135,78 @@ describe('CloudFormationValidatePlugin', () => {
     expect(process.exitCode).toBeUndefined();
   });
 
+  test('correctly reports errors at stack level instead of resource level', () => {
+    const app = new core.App({
+      context: {
+        [cxapi.VALIDATE_AGAINST_DEFAULT_RULES]: true,
+        [cxapi.FAIL_SYNTH_ON_VALIDATION_ERRORS_CONTEXT]: false,
+      },
+    });
+    // REmove any acknowledgements for this test, since we want to see the errors
+    (app.node as any)._metadata = [];
+
+    // F0001 missing resources
+    new core.Stack(app, 'TestStack');
+
+    const report = loadValidationReport(app.synth());
+    expect(report).toEqual(expect.objectContaining({
+      pluginReports: expect.arrayContaining([
+        expect.objectContaining({
+          pluginName: 'CloudFormation Validate',
+          violations: expect.arrayContaining([
+            expect.objectContaining({
+              ruleName: 'F0001',
+              violatingConstructs: expect.arrayContaining([
+                expect.objectContaining({
+                  constructPath: 'TestStack',
+                }),
+              ]),
+            }),
+          ]),
+        }),
+      ]),
+
+    }));
+  });
+
+  test('correctly reports errors for non-resources (e.g. Parameters) instead of resource level', () => {
+    const app = new core.App({
+      context: {
+        [cxapi.VALIDATE_AGAINST_DEFAULT_RULES]: true,
+        [cxapi.FAIL_SYNTH_ON_VALIDATION_ERRORS_CONTEXT]: false,
+      },
+    });
+    // REmove any acknowledgements for this test, since we want to see the errors
+    (app.node as any)._metadata = [];
+
+    const stack = new core.Stack(app, 'TestStack');
+    new core.CfnParameter(stack, 'MyParam', {
+      type: 'Blimp',
+    });
+
+    const report = loadValidationReport(app.synth());
+    expect(report).toEqual(expect.objectContaining({
+      pluginReports: expect.arrayContaining([
+        expect.objectContaining({
+          pluginName: 'CloudFormation Validate',
+          violations: expect.arrayContaining([
+            expect.objectContaining({
+              ruleName: 'F0001',
+              violatingConstructs: expect.arrayContaining([
+                expect.objectContaining({
+                  // TODO: Currently this references the stack, in the future perhaps we have more information
+                  // to reference the actual Parameter construct: <https://github.com/aws-cloudformation/cloudformation-validate/issues/201>
+                  constructPath: 'TestStack',
+                }),
+              ]),
+            }),
+          ]),
+        }),
+      ]),
+
+    }));
+  });
+
   test('plugin can be instantiated directly with custom rules', () => {
     const plugin = new core.CloudFormationValidatePlugin({
       regoRules: [{ name: 'my-rule', content: 'package main' }],
@@ -214,6 +287,7 @@ describe('CloudFormationValidatePlugin', () => {
     const plugin = new core.CloudFormationValidatePlugin();
     const report = plugin.validate({
       templatePaths: [templatePath],
+      stackTemplates: [{ stackConstructPath: 'TestStack', templatePath }],
       appConstruct: new Construct(undefined as any, ''),
       accountId: undefined,
       region: undefined,
@@ -228,6 +302,7 @@ describe('CloudFormationValidatePlugin', () => {
   });
 });
 
-function loadJson(filePath: string): any {
-  return JSON.parse(fs.readFileSync(filePath, 'utf-8'));
+function loadValidationReport(asm: cxapi.CloudAssembly) {
+  const p = path.join(asm.directory, 'validation-report.json');
+  return JSON.parse(fs.readFileSync(p, { encoding: 'utf-8' })) as PolicyValidationReportJson;
 }

packages/awslint/bin/awslint.ts

@@ -163,6 +163,8 @@ async function main() {
 
       if (color) {
         console.error(color(`${DiagnosticLevel[diag.level].toLowerCase()}: [${chalk.bold(`awslint:${diag.rule.code}`)}:${chalk.bold(diag.scope)}] ${diag.message}`));
+      } else {
+        console.error(`${DiagnosticLevel[diag.level].toLowerCase()}: [${`awslint:${diag.rule.code}`}:${diag.scope}] ${diag.message}`);
       }
     }