From eeb72a524ea3517c9329759f8c8a3a6412db8654 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 16 Jun 2026 17:08:43 -0400 Subject: [PATCH 1/3] chore(cli): validate via report instead of stack metadata --- .../toolkit-lib/lib/toolkit/toolkit.ts | 61 +++++---- .../toolkit-lib/test/actions/synth.test.ts | 8 +- .../toolkit-lib/test/toolkit/toolkit.test.ts | 6 +- packages/aws-cdk/lib/api-private.ts | 1 + packages/aws-cdk/lib/cli/cdk-toolkit.ts | 123 +++++++++++------- packages/aws-cdk/lib/cli/cli.ts | 6 +- packages/aws-cdk/test/commands/diff.test.ts | 12 +- 7 files changed, 139 insertions(+), 78 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 41589dea2..ec9ca72cb 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -27,7 +27,7 @@ import * as fs from 'fs-extra'; import { NonInteractiveIoHost } from './non-interactive-io-host'; import type { ToolkitServices } from './private'; import { assemblyFromSource } from './private'; -import { ToolkitError } from './toolkit-error'; +import { AssemblyError, ToolkitError } from './toolkit-error'; import type { DeployResult, DestroyResult, FeatureFlag, RollbackResult } from './types'; import type { BootstrapEnvironments, @@ -86,7 +86,7 @@ import { CloudFormationStackDiagnoser } from '../api/diagnosing/stack-diagnoser' import { DiffFormatter } from '../api/diff'; import { detectStackDrift } from '../api/drift'; import { DriftFormatter } from '../api/drift/drift-formatter'; -import type { IIoHost, IoMessageLevel, ToolkitAction } from '../api/io'; +import type { IIoHost, ToolkitAction } from '../api/io'; import type { ElapsedTime, IoHelper } from '../api/io/private'; import { asIoHelper, IO, SPAN, withoutColor, withoutEmojis, withTrimmedWhitespace } from '../api/io/private'; import { CloudWatchLogEventMonitor, findCloudWatchLogGroups } from '../api/logs-monitor'; @@ -345,7 +345,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { await using assembly = new AsyncDisposableBox(await synthAndMeasure(ioHelper, cx, stacksOpt(options))); const stacks = await assembly.value.selectStacksV2(stacksOpt(options)); const autoValidateStacks = options.validateStacks ? [assembly.value.selectStacksForValidation()] : []; - await this.validateStacksMetadata(stacks.concat(...autoValidateStacks), ioHelper); + await this.validateFromReport(assembly.value.directory, stacks.concat(...autoValidateStacks), ioHelper); // if we have a single stack, print it to STDOUT const message = `Successfully synthesized to ${chalk.blue(path.resolve(stacks.assembly.directory))}`; @@ -526,7 +526,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { await using assembly = await synthAndMeasure(ioHelper, cx, selectStacks); const stackCollection = await assembly.selectStacksV2(selectStacks); - await this.validateStacksMetadata(stackCollection, ioHelper); + await this.validateFromReport(assembly.directory, stackCollection, ioHelper); if (stackCollection.stackCount === 0) { await ioHelper.notify(IO.CDK_TOOLKIT_E5001.msg('No stacks selected')); @@ -785,7 +785,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { const ioHelper = asIoHelper(this.ioHost, action); const selectStacks = stacksOpt(options); const stackCollection = await assembly.selectStacksV2(selectStacks); - await this.validateStacksMetadata(stackCollection, ioHelper); + await this.validateFromReport(stackCollection.assembly.directory, stackCollection, ioHelper); const ret: DeployResult = { stacks: [], @@ -1301,7 +1301,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { const ioHelper = asIoHelper(this.ioHost, action); const stacks = await assembly.selectStacksV2(selectStacks); - await this.validateStacksMetadata(stacks, ioHelper); + await this.validateFromReport(stacks.assembly.directory, stacks, ioHelper); const ret: RollbackResult = { stacks: [], @@ -1669,23 +1669,40 @@ export class Toolkit extends CloudAssemblySourceBuilder { } /** - * Validate the stacks for errors and warnings according to the CLI's current settings + * Validate stacks by reading the validation report from the cloud assembly. + * Prints the report and throws according to the assemblyFailureAt setting. */ - private async validateStacksMetadata(stacks: StackCollection, ioHost: IoHelper) { - const builder = (level: IoMessageLevel) => { - switch (level) { - case 'error': - return IO.CDK_ASSEMBLY_E9999; - case 'warn': - return IO.CDK_ASSEMBLY_W9999; - default: - return IO.CDK_ASSEMBLY_I9999; - } - }; - await stacks.validateMetadata( - this.props.assemblyFailureAt, - async (level, msg) => ioHost.notify(builder(level).msg(`[${level} at ${msg.id}] ${msg.entry.data}`, msg)), + private async validateFromReport(assemblyDirectory: string, stacks: StackCollection, ioHelper: IoHelper) { + const failAt = this.props.assemblyFailureAt ?? 'error'; + const reportPath = path.join(assemblyDirectory, VALIDATION_REPORT_FILE); + if (!await fs.pathExists(reportPath)) { + return; + } + + const report = Manifest.loadValidationReport(reportPath); + const selectedStackIds = new Set(stacks.hierarchicalIds); + const filteredReports = filterReportsByStacks(report.pluginReports, selectedStackIds); + + const hasErrors = filteredReports.some((pr) => pr.conclusion === 'failure'); + const hasWarnings = filteredReports.some((pr) => + pr.violations.some((v) => v.severity === 'warning'), ); + + const conclusion: PolicyValidationReportConclusion = hasErrors ? 'failure' : 'success'; + const result: ValidateResult = { conclusion, title: report.title, pluginReports: filteredReports }; + await ioHelper.notify(hostMessageFromValidation(result)); + + if (hasErrors && failAt !== 'none') { + const error = AssemblyError.withStacks('Found errors', stacks.stackArtifacts); + error.attachSynthesisErrorCode('AnnotationErrors'); + throw error; + } + + if (hasWarnings && failAt === 'warn') { + const error = AssemblyError.withStacks('Found warnings (--strict mode)', stacks.stackArtifacts); + error.attachSynthesisErrorCode('StrictAnnotationWarnings'); + throw error; + } } /** @@ -1854,7 +1871,7 @@ function zeroTime(): ElapsedTime { return { asMs: 0, asSec: 0 }; } -function filterReportsByStacks(reports: PluginReportJson[], selectedStackIds: Set): PluginReportJson[] { +export function filterReportsByStacks(reports: PluginReportJson[], selectedStackIds: Set): PluginReportJson[] { return reports.map((report) => { const filteredViolations = report.violations.filter((violation) => { if (violation.violatingConstructs.length === 0) return true; diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/synth.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/synth.test.ts index d83ed6c76..17a641136 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/synth.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/synth.test.ts @@ -110,14 +110,18 @@ describe('synth', () => { await realDispose(); }); - test('assembly is disposed when synth fails due to error annotations', async () => { + test('synth fails when validation report contains errors', async () => { // GIVEN await using synthDir = autoCleanOutDir(); const builder: AssemblyBuilder = async (props) => { const app = new cdk.App({ outdir: props.outdir, - context: props.context, + context: { + ...props.context, + '@aws-cdk/core:annotationsInValidationReport': true, + '@aws-cdk/core:failSynthOnValidationErrors': false, + }, }); const stack = new cdk.Stack(app, 'SomeStack'); diff --git a/packages/@aws-cdk/toolkit-lib/test/toolkit/toolkit.test.ts b/packages/@aws-cdk/toolkit-lib/test/toolkit/toolkit.test.ts index 76316b4a5..b97336cae 100644 --- a/packages/@aws-cdk/toolkit-lib/test/toolkit/toolkit.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/toolkit/toolkit.test.ts @@ -79,7 +79,11 @@ test('outputs of assembly are measured', async () => { const builder = await toolkit.fromAssemblyBuilder(async (props) => { const app = new cdk.App({ outdir: props.outdir, - context: props.context, + context: { + ...props.context, + '@aws-cdk/core:annotationsInValidationReport': true, + '@aws-cdk/core:failSynthOnValidationErrors': false, + }, }); const s1 = new cdk.Stack(app, 'Stack1'); diff --git a/packages/aws-cdk/lib/api-private.ts b/packages/aws-cdk/lib/api-private.ts index 102510da0..d95993cd5 100644 --- a/packages/aws-cdk/lib/api-private.ts +++ b/packages/aws-cdk/lib/api-private.ts @@ -8,3 +8,4 @@ export * from '../../@aws-cdk/toolkit-lib/lib/api/tags/private'; export * from '../../@aws-cdk/toolkit-lib/lib/private/activity-printer'; export * from '../../@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/borrowed-assembly'; export * from '../../@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results'; +export { hostMessageFromValidation } from '../../@aws-cdk/toolkit-lib/lib/api/validate/validate-formatting'; diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index 33209a5c9..d576cd752 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -3,9 +3,10 @@ import * as path from 'node:path'; import { format } from 'node:util'; import type { IManifestEntry } from '@aws-cdk/cdk-assets-lib'; import * as cxapi from '@aws-cdk/cloud-assembly-api'; -import { RequireApproval } from '@aws-cdk/cloud-assembly-schema'; +import { Manifest, RequireApproval } from '@aws-cdk/cloud-assembly-schema'; +import type { PluginReportJson, PolicyValidationReportConclusion } from '@aws-cdk/cloud-assembly-schema'; import type { ConfirmationRequest, DeploymentMethod, DiagnoseOptions, PublishAssetsOptions, ToolkitAction, ToolkitOptions, UnstableFeature, ValidateOptions } from '@aws-cdk/toolkit-lib'; -import { PermissionChangeType, Toolkit, ToolkitError } from '@aws-cdk/toolkit-lib'; +import { AssemblyError, PermissionChangeType, Toolkit, ToolkitError } from '@aws-cdk/toolkit-lib'; import * as chalk from 'chalk'; import * as chokidar from 'chokidar'; import { type EventName, EVENTS } from 'chokidar/handler.js'; @@ -14,7 +15,7 @@ import { CliIoHost } from './io-host'; import type { Configuration } from './user-configuration'; import { PROJECT_CONFIG } from './user-configuration'; import type { ActionLessRequest, IMessageSpan, IoHelper } from '../../lib/api-private'; -import { asIoHelper, cfnApi, createIgnoreMatcher, IO, tagsForStack } from '../../lib/api-private'; +import { asIoHelper, cfnApi, createIgnoreMatcher, hostMessageFromValidation, IO, tagsForStack } from '../../lib/api-private'; import type { AssetBuildNode, AssetPublishNode, Concurrency, MarkerNode, StackNode, WorkGraph, WorkGraphActions } from '../api'; import { buildDestroyWorkGraph, @@ -216,8 +217,16 @@ export class CdkToolkit { 'validate': true, }; + let assemblyFailureAt: 'error' | 'warn' | 'none' = 'error'; + if (props.ignoreErrors) { + assemblyFailureAt = 'none'; + } + if (props.strict) { + assemblyFailureAt = 'warn'; + } + this.toolkit = new InternalToolkit(props.sdkProvider, { - assemblyFailureAt: this.validateMetadataFailAt(), + assemblyFailureAt, color: true, emojis: true, ioHost: this.ioHost, @@ -1265,7 +1274,7 @@ export class CdkToolkit { }); this.validateStacksSelected(stacks, selector.patterns); - await this.validateStacks(stacks); + await this.validateFromReport(stacks); return stacks; } @@ -1291,7 +1300,7 @@ export class CdkToolkit { : new StackCollection(assembly, []); this.validateStacksSelected(selectedForDiff.concat(autoValidateStacks), stackNames); - await this.validateStacks(selectedForDiff.concat(autoValidateStacks)); + await this.validateFromReport(selectedForDiff.concat(autoValidateStacks)); return selectedForDiff; } @@ -1309,15 +1318,24 @@ export class CdkToolkit { } /** - * Validate the stacks for errors and warnings according to the CLI's current settings + * Validate that if a user specified a stack name there exists at least 1 stack selected */ - private async validateStacks(stacks: StackCollection) { - const failAt = this.validateMetadataFailAt(); - await stacks.validateMetadata(failAt, stackMetadataLogger(this.ioHost.asIoHelper(), this.props.verbose)); + private validateStacksSelected(stacks: StackCollection, stackNames: string[]) { + if (stackNames.length != 0 && stacks.stackCount == 0) { + throw new ToolkitError('NoStacksMatched', `No stacks match the name(s) ${stackNames}`); + } } - private validateMetadataFailAt(): 'warn' | 'error' | 'none' { - let failAt: 'warn' | 'error' | 'none' = 'error'; + /** + * Validate stacks by reading the validation report from the cloud assembly. + */ + private async validateFromReport(stacks: StackCollection) { + const reportPath = path.join(stacks.assembly.directory, 'validation-report.json'); + if (!await fs.pathExists(reportPath)) { + return; + } + + let failAt: 'error' | 'warn' | 'none' = 'error'; if (this.props.ignoreErrors) { failAt = 'none'; } @@ -1325,15 +1343,30 @@ export class CdkToolkit { failAt = 'warn'; } - return failAt; - } + const report = Manifest.loadValidationReport(reportPath); + const selectedStackIds = new Set(stacks.hierarchicalIds); + const filteredReports = filterReportsByStacks(report.pluginReports, selectedStackIds); - /** - * Validate that if a user specified a stack name there exists at least 1 stack selected - */ - private validateStacksSelected(stacks: StackCollection, stackNames: string[]) { - if (stackNames.length != 0 && stacks.stackCount == 0) { - throw new ToolkitError('NoStacksMatched', `No stacks match the name(s) ${stackNames}`); + const hasErrors = filteredReports.some((pr) => pr.conclusion === 'failure'); + const hasWarnings = filteredReports.some((pr) => + pr.violations.some((v) => v.severity === 'warning'), + ); + + const conclusion: PolicyValidationReportConclusion = hasErrors ? 'failure' : 'success'; + await this.ioHost.asIoHelper().notify( + hostMessageFromValidation({ conclusion, title: report.title, pluginReports: filteredReports }), + ); + + if (hasErrors && failAt !== 'none') { + const error = AssemblyError.withStacks('Found errors', stacks.stackArtifacts); + error.attachSynthesisErrorCode('AnnotationErrors'); + throw error; + } + + if (hasWarnings && failAt === 'warn') { + const error = AssemblyError.withStacks('Found warnings (--strict mode)', stacks.stackArtifacts); + error.attachSynthesisErrorCode('StrictAnnotationWarnings'); + throw error; } } @@ -2084,31 +2117,6 @@ export async function displayFlagsMessage(ioHost: IoHelper, toolkit: InternalToo } } -/** - * Logger for processing stack metadata - */ -function stackMetadataLogger(ioHelper: IoHelper, verbose?: boolean): (level: 'info' | 'error' | 'warn', msg: cxapi.SynthesisMessage) => Promise { - const makeLogger = (level: string): [logger: (m: string) => void, prefix: string] => { - switch (level) { - case 'error': - return [(m) => ioHelper.defaults.error(m), 'Error']; - case 'warn': - return [(m) => ioHelper.defaults.warn(m), 'Warning']; - default: - return [(m) => ioHelper.defaults.info(m), 'Info']; - } - }; - - return async (level, msg) => { - const [logFn, prefix] = makeLogger(level); - await logFn(`[${prefix} at ${msg.id}] ${msg.entry.data}`); - - if (verbose && msg.entry.trace) { - logFn(` ${msg.entry.trace.join('\n ')}`); - } - }; -} - interface WorkGraphDeploymentActionsOptions { readonly roleArn?: string; readonly force?: boolean; @@ -2500,3 +2508,28 @@ function requiresApproval(requireApproval: RequireApproval, permissionChangeType requireApproval === RequireApproval.BROADENING && permissionChangeType === PermissionChangeType.BROADENING; } +function filterReportsByStacks(reports: PluginReportJson[], selectedStackIds: Set): PluginReportJson[] { + return reports.map((report) => { + const filteredViolations = report.violations.filter((violation) => { + if (violation.violatingConstructs.length === 0) return true; + return violation.violatingConstructs.some((c) => + selectedStackIds.has(c.constructPath?.split('/')[0] ?? ''), + ); + }).map((violation) => { + if (violation.violatingConstructs.length === 0) return violation; + return { + ...violation, + violatingConstructs: violation.violatingConstructs.filter((c) => + selectedStackIds.has(c.constructPath?.split('/')[0] ?? ''), + ), + }; + }); + + return { + ...report, + violations: filteredViolations, + conclusion: filteredViolations.length > 0 ? report.conclusion : ('success' as const), + }; + }); +} + diff --git a/packages/aws-cdk/lib/cli/cli.ts b/packages/aws-cdk/lib/cli/cli.ts index 4db7aa019..cef757c05 100644 --- a/packages/aws-cdk/lib/cli/cli.ts +++ b/packages/aws-cdk/lib/cli/cli.ts @@ -231,6 +231,11 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise { expect(exitCode).toBe(1); }); - test('throws an error during diffs on stack with error metadata', async () => { - // WHEN - await expect(() => - toolkit.diff({ - stackNames: ['C'], - }), - ).rejects.toThrow(/Found errors/); + test('diff succeeds even with error metadata on stack', async () => { + // WHEN - no longer throws on annotation errors + await toolkit.diff({ + stackNames: ['C'], + }); }); test('when quiet mode is enabled, stacks with no diffs should not print stack name & no differences to stdout', async () => { From 41c921daaa08944daf24790f005832085d6399ee Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 16 Jun 2026 17:13:43 -0400 Subject: [PATCH 2/3] revert bad change --- packages/aws-cdk/lib/cli/cli.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/cli/cli.ts b/packages/aws-cdk/lib/cli/cli.ts index cef757c05..4db7aa019 100644 --- a/packages/aws-cdk/lib/cli/cli.ts +++ b/packages/aws-cdk/lib/cli/cli.ts @@ -231,11 +231,6 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise Date: Tue, 16 Jun 2026 17:36:32 -0400 Subject: [PATCH 3/3] better test --- packages/aws-cdk/test/_helpers/assembly.ts | 7 +++++- packages/aws-cdk/test/commands/diff.test.ts | 25 ++++++++++++++++----- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/aws-cdk/test/_helpers/assembly.ts b/packages/aws-cdk/test/_helpers/assembly.ts index e0ad97fc8..6836597c7 100644 --- a/packages/aws-cdk/test/_helpers/assembly.ts +++ b/packages/aws-cdk/test/_helpers/assembly.ts @@ -1,7 +1,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { type CloudAssembly, CloudAssemblyBuilder, type CloudFormationStackArtifact, type StackMetadata } from '@aws-cdk/cloud-assembly-api'; -import { ArtifactMetadataEntryType, ArtifactType, type AssetManifest, type AssetMetadataEntry, type AwsCloudFormationStackProperties, type MetadataEntry, type MissingContext } from '@aws-cdk/cloud-assembly-schema'; +import { ArtifactMetadataEntryType, ArtifactType, type AssetManifest, type AssetMetadataEntry, type AwsCloudFormationStackProperties, type MetadataEntry, type MissingContext, type PolicyValidationReportJson } from '@aws-cdk/cloud-assembly-schema'; import { cxapiAssemblyWithForcedVersion } from './assembly-versions'; import { TestIoHost } from './io-host'; import { asIoHelper } from '../../lib/api-private'; @@ -37,6 +37,7 @@ export interface TestAssembly { missing?: MissingContext[]; nestedAssemblies?: TestAssembly[]; schemaVersion?: string; + validationReport?: PolicyValidationReportJson; } export class MockCloudExecutable extends CloudExecutable { @@ -149,6 +150,10 @@ export function testAssembly(assembly: TestAssembly): CloudAssembly { }); } + if (assembly.validationReport) { + fs.writeFileSync(path.join(builder.outdir, 'validation-report.json'), JSON.stringify(assembly.validationReport)); + } + const asm = builder.buildAssembly(); return cxapiAssemblyWithForcedVersion(asm, assembly.schemaVersion ?? SOME_RECENT_SCHEMA_VERSION); } diff --git a/packages/aws-cdk/test/commands/diff.test.ts b/packages/aws-cdk/test/commands/diff.test.ts index 34ab75606..83a05f593 100644 --- a/packages/aws-cdk/test/commands/diff.test.ts +++ b/packages/aws-cdk/test/commands/diff.test.ts @@ -486,6 +486,19 @@ describe('non-nested stacks', () => { template: { resource: 'D' }, }, ], + validationReport: { + version: '1.0.0', + pluginReports: [{ + pluginName: 'Construct Annotations', + conclusion: 'failure', + violations: [{ + ruleName: 'annotation-error', + description: 'this is an error', + severity: 'error', + violatingConstructs: [{ constructPath: 'C/resource' }], + }], + }], + }, }, undefined, ioHost); cloudFormation = instanceMockFrom(Deployments); @@ -616,11 +629,13 @@ describe('non-nested stacks', () => { expect(exitCode).toBe(1); }); - test('diff succeeds even with error metadata on stack', async () => { - // WHEN - no longer throws on annotation errors - await toolkit.diff({ - stackNames: ['C'], - }); + test('throws an error during diffs on stack with error metadata', async () => { + // WHEN + await expect(() => + toolkit.diff({ + stackNames: ['C'], + }), + ).rejects.toThrow(/Found errors/); }); test('when quiet mode is enabled, stacks with no diffs should not print stack name & no differences to stdout', async () => {