Skip to content
12 changes: 12 additions & 0 deletions packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ export interface ValidateResult {
* Reports from each validation plugin
*/
readonly pluginReports: PluginReportJson[];

/**
* The subset of `pluginReports` produced by online (CloudFormation change
* set) validation, as opposed to offline sources: policy validation plugins
* and construct annotations, both read from the cloud assembly.
*
* Contains the same object references as `pluginReports`. An empty array
* means online validation ran and found no problems.
*
* @default - online validation was skipped
*/
readonly onlineReports?: PluginReportJson[];
}

export type { PolicyValidationReportJson, PolicyValidationReportConclusion, PluginReportJson };
10 changes: 1 addition & 9 deletions packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts
Original file line number Diff line number Diff line change
@@ -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<WorkNode['type'], number>;

Expand Down Expand Up @@ -416,14 +416,6 @@ export interface WorkGraphActions {
marker: (markerNode: MarkerNode) => Promise<void>;
}

function sum(xs: number[]) {
let ret = 0;
for (const x of xs) {
ret += x;
}
return ret;
}

function retainOnly<A>(xs: A[], pred: (x: A) => boolean) {
xs.splice(0, xs.length, ...xs.filter(pred));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type * as cxapi from '@aws-cdk/cloud-assembly-api';
import { SynthesisMessageLevel } from '@aws-cdk/cloud-assembly-api';
import type { IMessageSpan } from '../../api/io/private/span';
import { sum } from '../../util';

export function countAssemblyResults(span: IMessageSpan<any>, assembly: cxapi.CloudAssembly) {
const stacksRecursively = assembly.stacksRecursively;
Expand All @@ -21,10 +22,6 @@ export function countAssemblyResults(span: IMessageSpan<any>, assembly: cxapi.Cl
}
}

function sum(xs: number[]) {
return xs.reduce((a, b) => a + b, 0);
}

/**
* Well-known and agreed-upon value between aws-cdk-lib and the toolkit
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { wouldFailDeploy } from './validation-report';
import type { ValidateResult } from '../../actions/validate';
import type { IMessageSpan } from '../../api/io/private/span';
import { sum } from '../../util';

/**
* Add counters describing the outcome of a validate run to the given span
*
* Offline violations (policy plugin reports and construct annotations read
* from the cloud assembly) are counted per severity. `offlineWouldFailDeploy`
* records whether the offline reports fail `wouldFailDeploy` at the default
* 'error' threshold; a deploy run with `--strict` or `--ignore-errors` moves
* that threshold, so this counter approximates the default deploy behavior.
*
* Online reports are identified by reference via `onlineReports`, not by
* plugin name: `pluginName` is a plugin-supplied string, so an offline
* policy plugin may carry any name.
*/
export function countValidationResults(span: IMessageSpan<any>, result: ValidateResult) {
const online = result.onlineReports ?? [];
const onlineSet = new Set(online);
const offline = result.pluginReports.filter((r) => !onlineSet.has(r));

for (const report of offline) {
for (const violation of report.violations) {
span.incCounter(`offlineViolations:${violation.severity}`);
}
}

span.incCounter('onlineViolations', sum(online.map((r) => r.violations.length)));
span.incCounter('offlineWouldFailDeploy', wouldFailDeploy(offline, 'error') ? 1 : 0);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
7 changes: 4 additions & 3 deletions packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -694,13 +694,13 @@ export class Toolkit extends CloudAssemblySourceBuilder {
const reports = await obtainUnifiedValidationReport(assembly, stacks);

// Online validation: submit templates to CloudFormation for early validation
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);
}
onlineReports = onlineReport ? [onlineReport] : [];
reports.push(...onlineReports);
}

const hasAnyViolations = reports.some(report => report.violations && report.violations.length > 0);
Expand All @@ -709,6 +709,7 @@ export class Toolkit extends CloudAssemblySourceBuilder {
conclusion: combineConclusions(reports),
title: undefined,
pluginReports: reports,
onlineReports,
};

if (!hasAnyViolations) {
Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ export function flatten<T>(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
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import type { PluginReportJson } from '@aws-cdk/cloud-assembly-schema';
import type { ValidateResult } from '../../lib/actions/validate';
import type { IMessageSpan } from '../../lib/api/io/private/span';
import { countValidationResults } from '../../lib/toolkit/private/count-validation-results';

let span: IMessageSpan<any>;
let counters: Record<string, number>;

beforeEach(() => {
counters = {};
span = {
incCounter: (name: string, delta: number = 1) => {
counters[name] = (counters[name] ?? 0) + delta;
},
} as IMessageSpan<any>;
});

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: [],
})),
};
}

function result(offlineReports: PluginReportJson[], onlineReports: PluginReportJson[] = []): ValidateResult {
const pluginReports = [...offlineReports, ...onlineReports];
return {
conclusion: pluginReports.some((r) => r.conclusion === 'failure') ? 'failure' : 'success',
pluginReports,
onlineReports,
};
}

test('counts offline violations per severity', () => {
countValidationResults(span, result([
report('SomePlugin', 'failure', ['error', 'error', 'warning']),
report('Construct Annotations', 'success', ['warning', 'info']),
]));

expect(counters).toEqual({
'offlineViolations:error': 2,
'offlineViolations:warning': 2,
'offlineViolations:info': 1,
'onlineViolations': 0,
'offlineWouldFailDeploy': 1,
});
});

test('online violations are counted separately from offline severities', () => {
countValidationResults(span, result([], [
report('CloudFormation', 'failure', ['fatal', 'fatal']),
]));

expect(counters).toEqual({
onlineViolations: 2,
offlineWouldFailDeploy: 0,
});
});

test('an offline plugin named CloudFormation is still counted as offline', () => {
countValidationResults(span, result([
report('CloudFormation', 'failure', ['error']),
]));

expect(counters).toEqual({
'offlineViolations:error': 1,
'onlineViolations': 0,
'offlineWouldFailDeploy': 1,
});
});

test('reports without onlineReports on the result are all counted as offline', () => {
countValidationResults(span, {
conclusion: 'failure',
pluginReports: [report('CloudFormation', 'failure', ['error'])],
});

expect(counters).toEqual({
'offlineViolations:error': 1,
'onlineViolations': 0,
'offlineWouldFailDeploy': 1,
});
});

test('offlineWouldFailDeploy is 0 when offline reports succeed', () => {
countValidationResults(span, result([
report('SomePlugin', 'success', ['warning']),
]));

expect(counters).toEqual({
'offlineViolations:warning': 1,
'onlineViolations': 0,
'offlineWouldFailDeploy': 0,
});
});

test('no reports produce zero counters', () => {
countValidationResults(span, result([]));

expect(counters).toEqual({
onlineViolations: 0,
offlineWouldFailDeploy: 0,
});
});
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/api-private.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ 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 * from '../../@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results';
export { throwIfValidationFailures } from '../../@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report';
24 changes: 21 additions & 3 deletions packages/aws-cdk/lib/cli/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { CliIoHost, suppressMessages } 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, formatExpressStabilizationWarning, IO, tagsForStack, throwIfValidationFailures } from '../../lib/api-private';
import { asIoHelper, cfnApi, countValidationResults, createIgnoreMatcher, formatExpressStabilizationWarning, IO, tagsForStack, throwIfValidationFailures } from '../../lib/api-private';
import type { AssetBuildNode, AssetPublishNode, Concurrency, MarkerNode, StackNode, WorkGraph, WorkGraphActions } from '../api';
import {
CloudWatchLogEventMonitor,
Expand Down Expand Up @@ -636,8 +636,26 @@ export class CdkToolkit {
return this.validateWatch(validateOptions);
}

const result = await this.toolkit.validate(this.props.cloudExecutable, validateOptions);
return result.conclusion === 'failure' ? 1 : 0;
// The VALIDATE span wraps the whole action, including the synthesis
// performed inside `toolkit.validate()`. Synthesis is also reported as
// its own SYNTH event (instrumented in CloudExecutable), so telemetry
// consumers can subtract it from the VALIDATE duration. The span is
// ended even if the app crashes during synthesis, so telemetry always
// records that a validation was started.
const validateSpan = await this.ioHost.asIoHelper().span(CLI_PRIVATE_SPAN.VALIDATE).begin({});
let error: ErrorDetails | undefined;
try {
const result = await this.toolkit.validate(this.props.cloudExecutable, validateOptions);
countValidationResults(validateSpan, result);
return result.conclusion === 'failure' ? 1 : 0;
Comment thread
iankhou marked this conversation as resolved.
Outdated
} catch (e: any) {
error = {
name: cdkCliErrorName(e),
};
throw e;
} finally {
await validateSpan.end({ error });
}
}

/**
Expand Down
3 changes: 3 additions & 0 deletions packages/aws-cdk/lib/cli/io-host/cli-io-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,9 @@ function eventFromMessage(msg: IoMessage<unknown>): TelemetryEvent | undefined {
if (CLI_PRIVATE_IO.CDK_CLI_I3003.is(msg)) {
return eventResult('ASSET', msg);
}
if (CLI_PRIVATE_IO.CDK_CLI_I4001.is(msg)) {
return eventResult('VALIDATE', msg);
}
// 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)) {
Expand Down
15 changes: 15 additions & 0 deletions packages/aws-cdk/lib/cli/telemetry/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ export const CLI_PRIVATE_IO = {
description: 'Finished asset building and publishing',
interface: 'EventResult',
}),
CDK_CLI_I4000: make.trace<EventStart>({
code: 'CDK_CLI_I4000',
description: 'Validation has started',
interface: 'EventStart',
}),
CDK_CLI_I4001: make.trace<EventResult>({
code: 'CDK_CLI_I4001',
description: 'Validation has finished',
interface: 'EventResult',
}),
};

/**
Expand All @@ -85,4 +95,9 @@ export const CLI_PRIVATE_SPAN = {
start: CLI_PRIVATE_IO.CDK_CLI_I3002,
end: CLI_PRIVATE_IO.CDK_CLI_I3003,
},
VALIDATE: {
name: 'Validation',
start: CLI_PRIVATE_IO.CDK_CLI_I4000,
end: CLI_PRIVATE_IO.CDK_CLI_I4001,
},
} satisfies Record<string, SpanDefinition<any, any>>;
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/cli/telemetry/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
export type State = 'ABORTED' | 'FAILED' | 'SUCCEEDED';
interface Event extends SessionEvent {
readonly state: State;
Expand Down
Loading
Loading