diff --git a/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts new file mode 100644 index 000000000..aa583a301 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts @@ -0,0 +1,41 @@ +import * as path from 'path'; +import * as fs from 'fs-extra'; +import { integTest, withSpecificFixture } from '../../lib'; + +integTest( + 'cdk validate records offlineWouldFailDeploy on the SYNTH telemetry event', + withSpecificFixture('validate-app', async (fixture) => { + const telemetryFile = path.join(fixture.integTestDir, `telemetry-validate-${Date.now()}.json`); + + // --no-online keeps the run deterministic; offline validation still runs + // during synthesis, and its deploy-blocking outcome is recorded on the + // SYNTH event as offlineWouldFailDeploy (visible for synth/deploy/validate). + await fixture.cdk( + ['--unstable=validate', 'validate', fixture.fullStackName('validate'), '--no-online', `--telemetry-file=${telemetryFile}`], + { + verboseLevel: 3, // trace mode + allowErrExit: true, // violations make validate exit non-zero + }, + ); + + // Assert against the local telemetry file: the CLI dispatches the batch to + // the endpoint from a background process, so its own output only confirms + // dispatch, not delivery. Real endpoint delivery is covered separately by + // cdk-telemetry-reaches-the-endpoint.integtest.ts. + const json = fs.readJSONSync(telemetryFile); + const synthEvent = json.find((e: any) => e.event?.eventType === 'SYNTH'); + expect(synthEvent).toBeDefined(); + + // The app's single S3 bucket makes SecurityPlugin report a fatal/error + // violation (what would have failed a deploy) and a warning-severity + // violation (counted by offlineValidationWarnings, separate from annotation warnings). + expect(synthEvent.counters).toEqual( + expect.objectContaining({ + offlineWouldFailDeploy: 1, + offlineValidationWarnings: 1, + }), + ); + + fs.unlinkSync(telemetryFile); + }), +); diff --git a/packages/@aws-cdk/toolkit-lib/docs/message-registry.md b/packages/@aws-cdk/toolkit-lib/docs/message-registry.md index 22e921d10..5f4470e69 100644 --- a/packages/@aws-cdk/toolkit-lib/docs/message-registry.md +++ b/packages/@aws-cdk/toolkit-lib/docs/message-registry.md @@ -158,6 +158,8 @@ Please let us know by [opening an issue](https://github.com/aws/aws-cdk-cli/issu | `CDK_TOOLKIT_E9600` | Policy validation failed | `error` | {@link ValidateResult} | | `CDK_TOOLKIT_I9601` | No policy validation report found | `info` | n/a | | `CDK_TOOLKIT_W9602` | Online validation could not be completed for a stack | `warn` | n/a | +| `CDK_TOOLKIT_I9603` | Online validation is starting | `trace` | {@link StackSelectionDetails} | +| `CDK_TOOLKIT_I9604` | Online validation has finished. Provides online validation timing, outcome counters, and any engine failure. | `trace` | {@link OnlineValidationResult} | | `CDK_TOOLKIT_I0100` | Notices decoration (the header or footer of a list of notices) | `info` | n/a | | `CDK_TOOLKIT_W0101` | A notice that is marked as a warning | `warn` | n/a | | `CDK_TOOLKIT_E0101` | A notice that is marked as an error | `error` | n/a | diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts b/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts index 6001cbca9..65ff77969 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts @@ -27,6 +27,7 @@ import type { ContextProviderMessageSource, Duration, ErrorPayload, + OnlineValidationResult, Operation, SingleStack, StackAndAssemblyData, @@ -563,6 +564,18 @@ export const IO = { description: 'Online validation could not be completed for a stack', }), + CDK_TOOLKIT_I9603: make.trace({ + code: 'CDK_TOOLKIT_I9603', + description: 'Online validation is starting', + interface: 'StackSelectionDetails', + }), + + CDK_TOOLKIT_I9604: make.trace({ + code: 'CDK_TOOLKIT_I9604', + description: 'Online validation has finished. Provides online validation timing, outcome counters, and any engine failure.', + interface: 'OnlineValidationResult', + }), + // Notices CDK_TOOLKIT_I0100: make.info({ code: 'CDK_TOOLKIT_I0100', @@ -734,4 +747,9 @@ export const SPAN = { start: IO.CDK_TOOLKIT_I5400, end: IO.CDK_TOOLKIT_I5410, }, + VALIDATE_ONLINE: { + name: 'Online validation', + start: IO.CDK_TOOLKIT_I9603, + end: IO.CDK_TOOLKIT_I9604, + }, } satisfies Record>; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts b/packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts index 93344d83d..f3d1d7cf8 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts @@ -1,7 +1,7 @@ import type { WorkNode, StackNode, AssetBuildNode, AssetPublishNode, MarkerNode } from './work-graph-types'; import { DeploymentState } from './work-graph-types'; import { ToolkitError } from '../../toolkit/toolkit-error'; -import { parallelPromises } from '../../util'; +import { parallelPromises, sum } from '../../util'; import type { IoHelper } from '../io/private'; export type Concurrency = number | Record; @@ -416,14 +416,6 @@ export interface WorkGraphActions { marker: (markerNode: MarkerNode) => Promise; } -function sum(xs: number[]) { - let ret = 0; - for (const x of xs) { - ret += x; - } - return ret; -} - function retainOnly(xs: A[], pred: (x: A) => boolean) { xs.splice(0, xs.length, ...xs.filter(pred)); } diff --git a/packages/@aws-cdk/toolkit-lib/lib/payloads/types.ts b/packages/@aws-cdk/toolkit-lib/lib/payloads/types.ts index 33fcd6835..074cef700 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/payloads/types.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/payloads/types.ts @@ -100,6 +100,34 @@ export interface Operation extends Duration { readonly error?: Error; } +/** + * The result of the online (CloudFormation change set) validation phase of the + * `validate` action. + * + * This payload is only emitted when online validation actually runs. `duration` + * times the online phase and `counters` describe only its outcome. Offline + * validation runs during synthesis and its counters are reported separately. + */ +export interface OnlineValidationResult extends Duration { + /** + * Counters describing the outcome of the online validation phase. + * + * @default - no counters + */ + readonly counters?: Record; + + /** + * Set when the online validation engine could not run at all. + * + * Online validation finding template problems is not an error; this is only + * set when the validation itself could not be performed (for example, when + * the CloudFormation calls failed for every selected stack). + * + * @default - online validation ran + */ + readonly error?: Error; +} + /** * Generic payload of a simple yes/no question. * diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts index fbbc5beb4..66e0e5d25 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts @@ -1,13 +1,27 @@ +import * as path from 'path'; import type * as cxapi from '@aws-cdk/cloud-assembly-api'; import { SynthesisMessageLevel } from '@aws-cdk/cloud-assembly-api'; +import type { PluginReportJson } from '@aws-cdk/cloud-assembly-schema'; +import { Manifest } from '@aws-cdk/cloud-assembly-schema'; +import * as fs from 'fs-extra'; import type { IMessageSpan } from '../../api/io/private/span'; +import { sum } from '../../util'; + +const VALIDATION_REPORT_FILE = 'validation-report.json'; export function countAssemblyResults(span: IMessageSpan, assembly: cxapi.CloudAssembly) { const stacksRecursively = assembly.stacksRecursively; + const summary = offlineValidationSummary(assembly); + const messageWarnings = sum(stacksRecursively.map(s => s.messages.filter(m => m.level === SynthesisMessageLevel.WARNING).length)); span.incCounter('stacks', stacksRecursively.length); span.incCounter('assemblies', asmCount(assembly)); span.incCounter('errorAnns', sum(stacksRecursively.map(s => s.messages.filter(m => m.level === SynthesisMessageLevel.ERROR).length))); - span.incCounter('warnings', sum(stacksRecursively.map(s => s.messages.filter(m => m.level === SynthesisMessageLevel.WARNING).length))); + // Construct annotation warnings live in stack.messages for legacy assemblies + // and in the validation report (report-only mode) for newer ones; the two are + // mutually exclusive, so summing them counts each once. + span.incCounter('warnings', messageWarnings + summary.reportAnnotationWarnings); + span.incCounter('offlineValidationWarnings', summary.offlineValidationWarnings); + span.incCounter('offlineWouldFailDeploy', summary.wouldFailDeploy ? 1 : 0); const annotationErrorCodes = stacksRecursively .flatMap(s => Object.values(s.metadata ?? {}) @@ -21,8 +35,73 @@ export function countAssemblyResults(span: IMessageSpan, assembly: cxapi.Cl } } -function sum(xs: number[]) { - return xs.reduce((a, b) => a + b, 0); +export interface OfflineValidationSummary { + /** + * Whether the offline validation results would have failed a default `cdk deploy` + * + * Mirrors `wouldFailDeploy` at the default 'error' threshold over the whole + * assembly (independent of any `--strict`/`--ignore-errors` on the current + * command): a deploy fails if there are error-level construct annotations or a + * policy plugin reported a failure. + */ + readonly wouldFailDeploy: boolean; + + /** + * The number of warning-severity violations reported by policy plugins + * + * Construct annotation warnings are excluded (they are counted by the + * `warnings` counter), so this only reflects policy validation plugins. + */ + readonly offlineValidationWarnings: number; + + /** + * The number of warning-severity construct annotations recorded in the + * validation report (report-only assemblies) + * + * These are folded into the `warnings` counter, not `offlineValidationWarnings`, + * so report-only annotation warnings are counted like message-based ones. + */ + readonly reportAnnotationWarnings: number; +} + +/** + * Summarize the offline validation outcome (policy report + construct annotations) for the whole assembly + */ +export function offlineValidationSummary(assembly: cxapi.CloudAssembly): OfflineValidationSummary { + const hasErrorAnnotations = assembly.stacksRecursively.some( + s => s.messages.some(m => m.level === SynthesisMessageLevel.ERROR), + ); + + const pluginReports = loadValidationReport(assembly); + + const warningsFor = (isAnnotation: boolean) => pluginReports + .filter(r => (r.pluginName === CONSTRUCT_ANNOTATIONS_PLUGINNAME) === isAnnotation) + .reduce((acc, r) => acc + r.violations.filter(v => v.severity === 'warning').length, 0); + + return { + wouldFailDeploy: hasErrorAnnotations || pluginReports.some(r => r.conclusion === 'failure'), + offlineValidationWarnings: warningsFor(false), + reportAnnotationWarnings: warningsFor(true), + }; +} + +/** + * Load the policy validation report, if any + * + * These counters are best-effort telemetry that run on every synth, so a + * missing or malformed report must never fail the command: on any read or + * schema error we behave as if there were no report. + */ +function loadValidationReport(assembly: cxapi.CloudAssembly): PluginReportJson[] { + const reportPath = path.join(assembly.directory, VALIDATION_REPORT_FILE); + if (!fs.existsSync(reportPath)) { + return []; + } + try { + return Manifest.loadValidationReport(reportPath).pluginReports; + } catch { + return []; + } } /** @@ -31,3 +110,11 @@ function sum(xs: number[]) { * Do not change, obviously. */ const ANNOTATION_ERROR_CODE_TYPE = 'aws:cdk:error-code'; + +/** + * The name of the plugin that emits construct annotations into the validation report. + * + * Its warnings are counted by the `warnings` counter, so they are excluded from + * the policy warning count. + */ +const CONSTRUCT_ANNOTATIONS_PLUGINNAME = 'Construct Annotations'; diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts new file mode 100644 index 000000000..b5fd6a5c5 --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts @@ -0,0 +1,17 @@ +import type { PluginReportJson } from '@aws-cdk/cloud-assembly-schema'; +import type { IMessageSpan } from '../../api/io/private/span'; +import { sum } from '../../util'; + +/** + * Add counters describing the online validation outcome to the given span + * + * `onlineViolations` counts the violations online validation reported, and + * `online:stacksIncomplete` records how many selected stacks could not be + * validated (0 on a clean run; every selected stack when the engine could not + * run at all, e.g. a setup failure before validation started). Both are always + * emitted so consumers can distinguish zero from missing data. + */ +export function countOnlineValidationResults(span: IMessageSpan, onlineReports: PluginReportJson[] | undefined, incompleteStacks: number) { + span.incCounter('onlineViolations', sum((onlineReports ?? []).map((r) => r.violations.length))); + span.incCounter('online:stacksIncomplete', incompleteStacks); +} diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts index 70a40c878..f4344116e 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts @@ -88,26 +88,36 @@ export async function throwIfValidationFailures( const result: ValidateResult = { conclusion, pluginReports }; await ioHelper.notify(hostMessageFromValidation(process.cwd(), result)); + if (!wouldFailDeploy(pluginReports, failAt)) { + return; + } + + if (failAt === 'warn') { + const error = AssemblyError.withStacks('Synthesis finished with warnings (--strict mode)', stacks.stackArtifacts); + error.attachSynthesisErrorCode('StrictAnnotationWarnings'); + throw error; + } + + const error = AssemblyError.withStacks('Synthesis finished with errors', stacks.stackArtifacts); + error.attachSynthesisErrorCode('AnnotationErrors'); + throw error; +} + +/** + * Whether the given validation reports make a deploy-like action fail at the given severity threshold + * + * This is the exact predicate applied by `throwIfValidationFailures`. + */ +export function wouldFailDeploy(pluginReports: PluginReportJson[], failAt: MinimumSeverity): boolean { switch (failAt) { case 'error': - if (conclusion === 'failure') { - const error = AssemblyError.withStacks('Synthesis finished with errors', stacks.stackArtifacts); - error.attachSynthesisErrorCode('AnnotationErrors'); - throw error; - } - break; + return combineConclusions(pluginReports) === 'failure'; case 'warn': - // if we're failing at 'warn', then both warnings and errors cause failure, so the initial conclusion is correct - if (conclusion === 'failure' || hasWarnings(pluginReports)) { - const error = AssemblyError.withStacks('Synthesis finished with warnings (--strict mode)', stacks.stackArtifacts); - error.attachSynthesisErrorCode('StrictAnnotationWarnings'); - throw error; - } - - break; + // if we're failing at 'warn', then both warnings and errors cause failure + return combineConclusions(pluginReports) === 'failure' || hasWarnings(pluginReports); case 'none': // if we're not failing at all, then the conclusion is always success - break; + return false; } } diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 93c154586..4d5104b0a 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -116,6 +116,7 @@ import { formatErrorMessage, formatExpressStabilizationWarning, formatTime, obsc import { pLimit } from '../util/concurrency'; import { createIgnoreMatcher } from '../util/glob-matcher'; import { promiseWithResolvers } from '../util/promises'; +import { countOnlineValidationResults } from './private/count-validation-results'; import { combineConclusions, obtainUnifiedValidationReport, throwIfValidationFailures } from './private/validation-report'; export interface ToolkitOptions { @@ -702,13 +703,40 @@ export class Toolkit extends CloudAssemblySourceBuilder { const reports = await obtainUnifiedValidationReport(assembly, stacks); - // Online validation: submit templates to CloudFormation for early validation + // Online validation: submit templates to CloudFormation for early validation. + // + // This is an optional phase that runs after synthesis and calls + // CloudFormation, so it is measured by its own VALIDATE_ONLINE telemetry + // event. + let onlineReports: PluginReportJson[] | undefined; if (options.online ?? true) { - const deployments = await this.deploymentsForAction('validate'); - - const onlineReport = await this.validateOnline(ioHelper, stacks, deployments); - if (onlineReport) { - reports.push(onlineReport); + const onlineSpan = await ioHelper.span(SPAN.VALIDATE_ONLINE).begin({ stacks: selectStacks }); + let onlineError: Error | undefined; + const stackCount = stacks.stackArtifacts.length; + // Assume no stack could be validated until validateOnline tells us otherwise + let incompleteStacks = stackCount; + try { + const deployments = await this.deploymentsForAction('validate'); + + const online = await this.validateOnline(ioHelper, stacks, deployments); + onlineReports = online.report ? [online.report] : []; + reports.push(...onlineReports); + incompleteStacks = online.incompleteStacks; + + // Online validation finding template problems is a successful run, not a + // failure of the validator. The engine itself failing to run is a failure + // recorded per-stack in `online:stacksIncomplete` and left non-fatal + // to the command, but if no stack could be validated at all we mark the + // phase as failed. + if (stackCount > 0 && incompleteStacks === stackCount) { + onlineError = new ToolkitError('OnlineValidationIncomplete', 'online validation could not be completed for any selected stack'); + } + } catch (e: any) { + onlineError = e; + throw e; + } finally { + countOnlineValidationResults(onlineSpan, onlineReports, incompleteStacks); + await onlineSpan.end(onlineError ? { error: onlineError } : {}); } } @@ -733,8 +761,9 @@ export class Toolkit extends CloudAssemblySourceBuilder { ioHelper: IoHelper, stacks: StackCollection, deployments: Deployments, - ): Promise { + ): Promise<{ report: PluginReportJson | undefined; incompleteStacks: number }> { const violations: PluginReportJson['violations'] = []; + let incompleteStacks = 0; for (const stack of stacks.stackArtifacts) { try { @@ -763,20 +792,30 @@ export class Toolkit extends CloudAssemblySourceBuilder { }], }); } + } else if (diagnosis.type === 'error-diagnosing') { + // The diagnosis itself failed (createValidationChangeSet resolves such + // failures into a result rather than throwing), so this stack was not + // actually validated -- count it as incomplete, like a thrown error. + incompleteStacks += 1; + await ioHelper.notify(IO.CDK_TOOLKIT_W9602.msg(`Online validation could not be completed for stack '${stack.hierarchicalId}': ${diagnosis.message}`)); } } catch (e: any) { + incompleteStacks += 1; await ioHelper.notify(IO.CDK_TOOLKIT_W9602.msg(`Online validation could not be completed for stack '${stack.hierarchicalId}': ${e.message}`)); } } if (violations.length === 0) { - return undefined; + return { report: undefined, incompleteStacks }; } return { - pluginName: 'CloudFormation', - conclusion: 'failure', - violations, + report: { + pluginName: 'CloudFormation', + conclusion: 'failure', + violations, + }, + incompleteStacks, }; } diff --git a/packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts b/packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts index 268d1c09e..a7006503a 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts @@ -12,6 +12,13 @@ export function flatten(xs: T[][]): T[] { return Array.prototype.concat.apply([], xs); } +/** + * Sum a list of numbers + */ +export function sum(xs: number[]): number { + return xs.reduce((a, b) => a + b, 0); +} + /** * Partition a collection by removing and returning all elements that match a predicate * diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/validate.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/validate.test.ts index cefbb1bc8..e50440231 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/validate.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/validate.test.ts @@ -260,4 +260,56 @@ describe('validate --online', () => { message: expect.stringContaining('Access denied'), })); }); + + test('counts a stack whose diagnosis errors out (not just throws) as incomplete', async () => { + // createValidationChangeSet resolves a failed diagnosis into an + // 'error-diagnosing' result rather than throwing, so it never hits the + // catch. The stack was still not validated and must count as incomplete. + jest.spyOn(cfnApi, 'createValidationChangeSet').mockResolvedValue({ + changeSet: { $metadata: {} } as any, + diagnosis: Diagnosis.errorDiagnosing('Access denied while describing the stack'), + }); + + // trace level so the VALIDATE_ONLINE (CDK_TOOLKIT_I9604) counters are captured + const traceIoHost = new TestIoHost('trace'); + const traceToolkit = new Toolkit({ ioHost: traceIoHost }); + const cx = await cdkOutFixture(traceToolkit, 'stack-with-bucket'); + const result = await traceToolkit.validate(cx, { online: true }); + + // Offline is clean and an incomplete online run is non-fatal, so the command succeeds. + expect(result.conclusion).toBe('success'); + expect(result.pluginReports).toHaveLength(0); + + // The un-validatable stack is surfaced as a warning and counted as incomplete. + traceIoHost.expectMessage({ level: 'warn', containing: 'Access denied while describing the stack' }); + const end = traceIoHost.messages.find(m => m.code === 'CDK_TOOLKIT_I9604'); + expect((end?.data as any).counters['online:stacksIncomplete']).toBe(1); + // Every selected stack was incomplete, so the phase is marked failed. + expect((end?.data as any).error?.name).toBe('OnlineValidationIncomplete'); + }); + + test('a partially incomplete online run is not marked failed', async () => { + // First stack cannot be diagnosed, the second validates cleanly. + jest.spyOn(cfnApi, 'createValidationChangeSet') + .mockResolvedValueOnce({ + changeSet: { $metadata: {} } as any, + diagnosis: Diagnosis.errorDiagnosing('Access denied while describing the stack'), + }) + .mockResolvedValue({ + changeSet: { $metadata: {} } as any, + diagnosis: Diagnosis.noProblem(), + }); + + const traceIoHost = new TestIoHost('trace'); + const traceToolkit = new Toolkit({ ioHost: traceIoHost }); + const cx = await cdkOutFixture(traceToolkit, 'two-empty-stacks'); + const result = await traceToolkit.validate(cx, { online: true }); + + expect(result.conclusion).toBe('success'); + + const end = traceIoHost.messages.find(m => m.code === 'CDK_TOOLKIT_I9604'); + expect((end?.data as any).counters['online:stacksIncomplete']).toBe(1); + // Not every stack was incomplete, so the phase is not failed. + expect((end?.data as any).error).toBeUndefined(); + }); }); diff --git a/packages/@aws-cdk/toolkit-lib/test/toolkit/count-assembly-results.test.ts b/packages/@aws-cdk/toolkit-lib/test/toolkit/count-assembly-results.test.ts index 09b8a417b..51aa03336 100644 --- a/packages/@aws-cdk/toolkit-lib/test/toolkit/count-assembly-results.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/toolkit/count-assembly-results.test.ts @@ -1,4 +1,19 @@ -import { countAssemblyResults } from '../../lib/toolkit/private/count-assembly-results'; +import * as os from 'os'; +import * as path from 'path'; +import type * as cxapi from '@aws-cdk/cloud-assembly-api'; +import { SynthesisMessageLevel } from '@aws-cdk/cloud-assembly-api'; +import * as fs from 'fs-extra'; +import { countAssemblyResults, offlineValidationSummary } from '../../lib/toolkit/private/count-assembly-results'; + +let dir: string; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'count-assembly-results-')); +}); + +afterEach(() => { + fs.removeSync(dir); +}); // Minimal span that records incCounter calls; only the surface // countAssemblyResults touches is implemented. @@ -19,11 +34,37 @@ function fakeSpan() { // Object.values(stack.metadata) with no null guard. function assemblyWithStack(stack: any): any { return { + directory: dir, stacksRecursively: [stack], nestedAssemblies: [], }; } +function assembly(messageLevels: SynthesisMessageLevel[]): cxapi.CloudAssembly { + return { + directory: dir, + stacksRecursively: [{ + messages: messageLevels.map((level) => ({ level, id: 'some-id', entry: { type: 'aws:cdk:error', data: 'msg' } })), + }], + } as any; +} + +function writeReport(pluginReports: Array<{ pluginName?: string; conclusion: 'success' | 'failure'; severities?: string[] }>) { + fs.writeJSONSync(path.join(dir, 'validation-report.json'), { + version: '1.0.0', + pluginReports: pluginReports.map((r, i) => ({ + pluginName: r.pluginName ?? `Plugin${i}`, + conclusion: r.conclusion, + violations: (r.severities ?? []).map((severity) => ({ + ruleName: 'some-rule', + description: 'some description', + severity, + violatingConstructs: [], + })), + })), + }); +} + describe('countAssemblyResults', () => { test('does not throw when a stack has no metadata', () => { const { span } = fakeSpan(); @@ -48,4 +89,98 @@ describe('countAssemblyResults', () => { expect(counters).toContainEqual({ name: 'errorAnn:MY_ERROR', delta: undefined }); }); + + test('warnings counter sums message warnings and report-only construct annotation warnings', () => { + const { span, counters } = fakeSpan(); + writeReport([{ pluginName: 'Construct Annotations', conclusion: 'success', severities: ['warning', 'warning'] }]); + const stack = { + messages: [{ level: SynthesisMessageLevel.WARNING }], + metadata: {}, + }; + + countAssemblyResults(span, assemblyWithStack(stack)); + + expect(counters).toContainEqual({ name: 'warnings', delta: 3 }); // 1 message + 2 report + }); +}); + +describe('offlineValidationSummary', () => { + describe('wouldFailDeploy', () => { + test('false when there are no error annotations and no validation report', () => { + expect(offlineValidationSummary(assembly([SynthesisMessageLevel.WARNING])).wouldFailDeploy).toBe(false); + }); + + test('true when a stack has an error-level annotation', () => { + expect(offlineValidationSummary(assembly([SynthesisMessageLevel.ERROR])).wouldFailDeploy).toBe(true); + }); + + test('true when the validation report has a failing plugin report', () => { + writeReport([{ conclusion: 'success' }, { conclusion: 'failure' }]); + + expect(offlineValidationSummary(assembly([])).wouldFailDeploy).toBe(true); + }); + + test('false when the validation report has only successful plugin reports', () => { + writeReport([{ conclusion: 'success' }]); + + expect(offlineValidationSummary(assembly([])).wouldFailDeploy).toBe(false); + }); + }); + + describe('offlineValidationWarnings', () => { + test('zero when there is no validation report', () => { + expect(offlineValidationSummary(assembly([])).offlineValidationWarnings).toBe(0); + }); + + test('counts warning-severity violations across policy plugin reports', () => { + writeReport([ + { conclusion: 'failure', severities: ['error', 'warning', 'warning'] }, + { conclusion: 'success', severities: ['warning', 'info'] }, + ]); + + expect(offlineValidationSummary(assembly([])).offlineValidationWarnings).toBe(3); + }); + + test('excludes construct annotation warnings (counted by the warnings counter)', () => { + writeReport([ + { pluginName: 'Construct Annotations', conclusion: 'success', severities: ['warning', 'warning'] }, + { pluginName: 'SomePolicyPlugin', conclusion: 'success', severities: ['warning'] }, + ]); + + const summary = offlineValidationSummary(assembly([])); + expect(summary.offlineValidationWarnings).toBe(1); + expect(summary.reportAnnotationWarnings).toBe(2); + }); + }); + + describe('reportAnnotationWarnings', () => { + test('zero when there is no validation report', () => { + expect(offlineValidationSummary(assembly([])).reportAnnotationWarnings).toBe(0); + }); + + test('counts only warning-severity construct annotations in the report', () => { + writeReport([ + { pluginName: 'Construct Annotations', conclusion: 'failure', severities: ['error', 'warning', 'warning'] }, + ]); + + expect(offlineValidationSummary(assembly([])).reportAnnotationWarnings).toBe(2); + }); + }); + + describe('malformed validation report', () => { + beforeEach(() => { + fs.writeFileSync(path.join(dir, 'validation-report.json'), 'this is not valid json {'); + }); + + test('does not throw and reports no policy warnings', () => { + const summary = offlineValidationSummary(assembly([])); + + expect(summary.offlineValidationWarnings).toBe(0); + expect(summary.wouldFailDeploy).toBe(false); + }); + + test('still reports wouldFailDeploy from error-level annotations', () => { + expect(offlineValidationSummary(assembly([SynthesisMessageLevel.ERROR])).wouldFailDeploy).toBe(true); + }); + }); }); diff --git a/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts b/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts new file mode 100644 index 000000000..ad450c3e4 --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts @@ -0,0 +1,50 @@ +import type { PluginReportJson } from '@aws-cdk/cloud-assembly-schema'; +import type { IMessageSpan } from '../../lib/api/io/private/span'; +import { countOnlineValidationResults } from '../../lib/toolkit/private/count-validation-results'; + +let span: IMessageSpan; +let counters: Record; + +beforeEach(() => { + counters = {}; + span = { + incCounter: (name: string, delta: number = 1) => { + counters[name] = (counters[name] ?? 0) + delta; + }, + } as IMessageSpan; +}); + +function report(pluginName: string, conclusion: 'success' | 'failure', severities: string[]): PluginReportJson { + return { + pluginName, + conclusion, + violations: severities.map((severity) => ({ + ruleName: 'some-rule', + description: 'some description', + severity: severity as any, + violatingConstructs: [], + })), + }; +} + +describe('countOnlineValidationResults', () => { + test('counts online violations and records incomplete stacks', () => { + countOnlineValidationResults(span, [ + report('CloudFormation', 'failure', ['fatal', 'fatal']), + ], 0); + + expect(counters).toEqual({ 'onlineViolations': 2, 'online:stacksIncomplete': 0 }); + }); + + test('records the number of stacks that could not be validated', () => { + countOnlineValidationResults(span, [], 3); + + expect(counters).toEqual({ 'onlineViolations': 0, 'online:stacksIncomplete': 3 }); + }); + + test('always emits both counters, even for undefined online reports', () => { + countOnlineValidationResults(span, undefined, 0); + + expect(counters).toEqual({ 'onlineViolations': 0, 'online:stacksIncomplete': 0 }); + }); +}); diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index 857a3f43b..baab1d551 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -637,6 +637,9 @@ export class CdkToolkit { return this.validateWatch(validateOptions); } + // Offline validation runs during synthesis; its outcome (offlineWouldFailDeploy) + // is recorded on the SYNTH telemetry event. Online validation timing and state + // are emitted separately by toolkit-lib as a VALIDATE_ONLINE event. const result = await this.toolkit.validate(this.props.cloudExecutable, validateOptions); return result.conclusion === 'failure' ? 1 : 0; } diff --git a/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts b/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts index 9f7da2a5d..a12d89d43 100644 --- a/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts +++ b/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts @@ -1257,6 +1257,18 @@ function eventFromMessage(msg: IoMessage): TelemetryEvent | undefined { if (CLI_PRIVATE_IO.CDK_CLI_I3003.is(msg)) { return eventResult('ASSET', msg); } + // Online validation lives in toolkit-lib, so (like hotswap) it cannot use a + // CDK_CLI code. We translate the toolkit-lib VALIDATE_ONLINE span end message + // into the telemetry event instead. Its `error` is a raw `Error`, so we + // convert it to a telemetry error name rather than passing it through. + if (IO.CDK_TOOLKIT_I9604.is(msg)) { + return { + eventType: 'VALIDATE_ONLINE', + duration: msg.data.duration, + ...(msg.data.error ? { error: { name: cdkCliErrorName(msg.data.error) } } : {}), + counters: msg.data.counters, + }; + } // Hotswap lives in the cdk-toolkit so it cannot be a CDK_CLI error code. // Instead we reuse the existing Hotswap span. if (IO.CDK_TOOLKIT_I5410.is(msg)) { diff --git a/packages/aws-cdk/lib/cli/telemetry/schema.ts b/packages/aws-cdk/lib/cli/telemetry/schema.ts index c2affc98d..dd79ae812 100644 --- a/packages/aws-cdk/lib/cli/telemetry/schema.ts +++ b/packages/aws-cdk/lib/cli/telemetry/schema.ts @@ -25,7 +25,7 @@ interface SessionEvent { readonly command: Command; } -export type EventType = 'SYNTH' | 'INVOKE' | 'DEPLOY' | 'HOTSWAP' | 'ASSET'; +export type EventType = 'SYNTH' | 'INVOKE' | 'DEPLOY' | 'HOTSWAP' | 'ASSET' | 'VALIDATE_ONLINE'; export type State = 'ABORTED' | 'FAILED' | 'SUCCEEDED'; interface Event extends SessionEvent { readonly state: State; diff --git a/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts b/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts index 087468280..4619f712b 100644 --- a/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts +++ b/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts @@ -2,6 +2,7 @@ import * as os from 'os'; import * as path from 'path'; import { PassThrough } from 'stream'; import { RequireApproval } from '@aws-cdk/cloud-assembly-schema'; +import { ToolkitError } from '@aws-cdk/toolkit-lib'; import chalk from 'chalk'; import * as fs from 'fs-extra'; import { Context } from '../../../lib/api/context'; @@ -835,6 +836,62 @@ describe('CliIoHost', () => { })); }); + test('emit telemetry on VALIDATE_ONLINE event', async () => { + // Online validation lives in toolkit-lib, so its telemetry is translated + // from the toolkit-lib span end message (CDK_TOOLKIT_I9604), not a CDK_CLI code. + const message: IoMessage = { + time: new Date(), + level: 'info', + action: 'validate', + code: 'CDK_TOOLKIT_I9604', + message: 'telemetry message', + data: { + duration: 123, + counters: { + 'onlineViolations': 2, + 'online:stacksIncomplete': 0, + }, + }, + }; + + // Send the notification + await telemetryIoHost.notify(message); + + // Verify that the emit method was called with the correct parameters + expect(telemetryEmitSpy).toHaveBeenCalledWith(expect.objectContaining({ + eventType: 'VALIDATE_ONLINE', + duration: 123, + counters: { + 'onlineViolations': 2, + 'online:stacksIncomplete': 0, + }, + })); + }); + + test('VALIDATE_ONLINE event carries an error name when the online engine could not run', async () => { + // The span end message carries a raw `Error`; the CLI converts it to a + // telemetry error name so the event state becomes FAILED. + const message: IoMessage = { + time: new Date(), + level: 'info', + action: 'validate', + code: 'CDK_TOOLKIT_I9604', + message: 'telemetry message', + data: { + duration: 123, + counters: { onlineViolations: 0 }, + error: new ToolkitError('OnlineValidationIncomplete', 'online validation could not be completed for any selected stack'), + }, + }; + + await telemetryIoHost.notify(message); + + expect(telemetryEmitSpy).toHaveBeenCalledWith(expect.objectContaining({ + eventType: 'VALIDATE_ONLINE', + error: { name: 'OnlineValidationIncomplete' }, + })); + }); + test('do not emit telemetry on non telemetry codes', async () => { // Create a message that should trigger telemetry using the actual message code const message: IoMessage = { diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_does_not_emit_a_VALIDATE_ONLINE_event_when_online_validation_is_disabled.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_does_not_emit_a_VALIDATE_ONLINE_event_when_online_validation_is_disabled.ndjson new file mode 100644 index 000000000..9388eb1c0 --- /dev/null +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_does_not_emit_a_VALIDATE_ONLINE_event_when_online_validation_is_disabled.ndjson @@ -0,0 +1,5 @@ +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} +{"seq":3,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} diff --git a/packages/aws-cdk/test/commands/validate.test.ts b/packages/aws-cdk/test/commands/validate.test.ts index 372a8f6d5..18589f7af 100644 --- a/packages/aws-cdk/test/commands/validate.test.ts +++ b/packages/aws-cdk/test/commands/validate.test.ts @@ -152,6 +152,26 @@ describe('with violations', () => { }); }); +describe('telemetry', () => { + afterEach(() => { + // Remove the spies installed by these tests; the file-level `resetAllMocks` + // would otherwise strip the passthrough implementation from `ioHost.notify` + // and break tests that run later (test order is randomized). + jest.restoreAllMocks(); + }); + + test('does not emit a VALIDATE_ONLINE event when online validation is disabled', async () => { + const notifySpy = jest.spyOn(ioHost, 'notify'); + await toolkit.validate({ + stacks: { patterns: [], strategy: StackSelectionStrategy.ALL_STACKS }, + online: false, + }); + + expect(notifySpy).not.toHaveBeenCalledWith(expect.objectContaining({ code: 'CDK_TOOLKIT_I9603' })); + expect(notifySpy).not.toHaveBeenCalledWith(expect.objectContaining({ code: 'CDK_TOOLKIT_I9604' })); + }); +}); + describe('stack selection', () => { test('validates a single selected stack', async () => { const exitCode = await toolkit.validate({