diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/validate-online-app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/validate-online-app/app.js new file mode 100644 index 000000000..fc6fc15ce --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/validate-online-app/app.js @@ -0,0 +1,94 @@ +const fs = require('fs'); +const cdk = require('aws-cdk-lib/core'); +const s3 = require('aws-cdk-lib/aws-s3'); + +const stackPrefix = process.env.STACK_NAME_PREFIX; +if (!stackPrefix) { + throw new Error('the STACK_NAME_PREFIX environment variable is required'); +} + +const SHARED_BUCKET_NAME = `${stackPrefix}-validate-online-shared-bucket`; + +class SecurityPlugin { + constructor() { + this.name = 'SecurityPlugin'; + this.version = '1.0.0'; + } + + validate(context) { + const violations = []; + for (const templatePath of context.templatePaths) { + const template = JSON.parse(fs.readFileSync(templatePath, 'utf-8')); + for (const [logicalId, resource] of Object.entries(template.Resources || {})) { + if (resource.Type === 'AWS::S3::Bucket') { + violations.push({ + ruleName: 'no-public-buckets', + description: 'S3 Buckets must not be publicly accessible', + fix: 'Set PublicAccessBlockConfiguration on the bucket', + severity: 'error', + violatingResources: [{ + resourceLogicalId: logicalId, + templatePath, + locations: [`/Resources/${logicalId}/Properties/PublicAccessBlockConfiguration`], + }], + }); + } + } + } + return { success: violations.length === 0, violations }; + } +} + +const app = new cdk.App(); +cdk.Validations.of(app).addPlugins(new SecurityPlugin()); + +// Valid stack — no offline or online errors +class ValidStack extends cdk.Stack { + constructor(scope, id, props) { + super(scope, id, props); + new cdk.CfnResource(this, 'WaitHandle', { + type: 'AWS::CloudFormation::WaitConditionHandle', + }); + } +} + +// Deployed stack — owns the bucket with the shared name +class DeployedStack extends cdk.Stack { + constructor(scope, id, props) { + super(scope, id, props); + new s3.Bucket(this, 'ExistingBucket', { + bucketName: SHARED_BUCKET_NAME, + removalPolicy: cdk.RemovalPolicy.DESTROY, + }); + } +} + +// Conflicting stack — tries to create a bucket with the same name (early validation error) +class ConflictingStack extends cdk.Stack { + constructor(scope, id, props) { + super(scope, id, props); + new s3.Bucket(this, 'ConflictBucket', { + bucketName: SHARED_BUCKET_NAME, + removalPolicy: cdk.RemovalPolicy.DESTROY, + }); + } +} + +// Combined stack — has BOTH offline (S3 bucket triggers SecurityPlugin) +// AND online errors (bucket name conflict caught by CFN early validation) +class CombinedStack extends cdk.Stack { + constructor(scope, id, props) { + super(scope, id, props); + new s3.Bucket(this, 'MyBucket', { + bucketName: SHARED_BUCKET_NAME, + removalPolicy: cdk.RemovalPolicy.DESTROY, + }); + } +} + +new ValidStack(app, `${stackPrefix}-validate-online-valid`); +new DeployedStack(app, `${stackPrefix}-validate-online-deployed`); +new ConflictingStack(app, `${stackPrefix}-validate-online-conflicting`); +new CombinedStack(app, `${stackPrefix}-validate-online-combined`); + +app.synth(); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/validate-online-app/cdk.json b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/validate-online-app/cdk.json new file mode 100644 index 000000000..791696b03 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/validate-online-app/cdk.json @@ -0,0 +1,7 @@ +{ + "app": "node app.js", + "versionReporting": false, + "context": { + "@aws-cdk/core:failSynthOnValidationErrors": false + } +} diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-no-online-skips-cfn.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-no-online-skips-cfn.integtest.ts new file mode 100644 index 000000000..4c4ca0739 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-no-online-skips-cfn.integtest.ts @@ -0,0 +1,25 @@ +import { integTest, withSpecificFixture } from '../../../lib'; + +integTest( + 'cdk validate --no-online skips CloudFormation validation', + withSpecificFixture('validate-online-app', async (fixture) => { + // Deploy a stack that owns the bucket + await fixture.cdk( + ['deploy', '--require-approval=never', fixture.fullStackName('validate-online-deployed')], + ); + + // Validate with --no-online: the bucket name conflict should NOT be caught + const output = await fixture.cdk( + ['--unstable=validate', 'validate', '--no-online', fixture.fullStackName('validate-online-conflicting')], + { + allowErrExit: true, + }, + ); + + expect(output).not.toContain('already exists'); + expect(output).not.toContain('CloudFormation'); + + // Cleanup + await fixture.cdk(['destroy', '--force', fixture.fullStackName('validate-online-deployed')]); + }), +); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-catches-invalid-resource.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-catches-invalid-resource.integtest.ts new file mode 100644 index 000000000..c55dc93a5 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-catches-invalid-resource.integtest.ts @@ -0,0 +1,25 @@ +import { integTest, withSpecificFixture } from '../../../lib'; + +integTest( + 'cdk validate --online catches bucket name conflict', + withSpecificFixture('validate-online-app', async (fixture) => { + // Deploy a stack that owns the bucket + await fixture.cdk( + ['deploy', '--require-approval=never', fixture.fullStackName('validate-online-deployed')], + ); + + // Now validate a stack that tries to create the same bucket name + const output = await fixture.cdk( + ['--unstable=validate', 'validate', '--online', fixture.fullStackName('validate-online-conflicting')], + { + allowErrExit: true, + }, + ); + + expect(output).toContain('CloudFormation'); + expect(output).toContain('already exists'); + + // Cleanup + await fixture.cdk(['destroy', '--force', fixture.fullStackName('validate-online-deployed')]); + }), +); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-combined-offline-and-online.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-combined-offline-and-online.integtest.ts new file mode 100644 index 000000000..3eb042fb4 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-combined-offline-and-online.integtest.ts @@ -0,0 +1,31 @@ +import { integTest, withSpecificFixture } from '../../../lib'; + +integTest( + 'cdk validate --online reports both offline and online errors', + withSpecificFixture('validate-online-app', async (fixture) => { + // Deploy a stack that owns the bucket + await fixture.cdk( + ['deploy', '--require-approval=never', fixture.fullStackName('validate-online-deployed')], + ); + + // Validate combined stack — has both a bucket (SecurityPlugin) and + // uses the same bucket name (CFN early validation conflict) + const output = await fixture.cdk( + ['--unstable=validate', 'validate', '--online', fixture.fullStackName('validate-online-combined')], + { + allowErrExit: true, + }, + ); + + // Offline: SecurityPlugin catches the S3 bucket + expect(output).toContain('S3 Buckets must not be publicly accessible'); + expect(output).toContain('SecurityPlugin'); + + // Online: CloudFormation catches the bucket name conflict + expect(output).toContain('already exists'); + expect(output).toContain('CloudFormation'); + + // Cleanup + await fixture.cdk(['destroy', '--force', fixture.fullStackName('validate-online-deployed')]); + }), +); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-passes-valid-template.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-passes-valid-template.integtest.ts new file mode 100644 index 000000000..fe68efe12 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/validate-online/cdk-validate-online-passes-valid-template.integtest.ts @@ -0,0 +1,12 @@ +import { integTest, withSpecificFixture } from '../../../lib'; + +integTest( + 'cdk validate --online passes for valid template', + withSpecificFixture('validate-online-app', async (fixture) => { + const output = await fixture.cdk( + ['--unstable=validate', 'validate', '--online', fixture.fullStackName('validate-online-valid')], + ); + + expect(output).toContain('No problems found'); + }), +); diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts index 93f4b2272..ee4f3a09d 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts @@ -6,6 +6,17 @@ export interface ValidateOptions { * Select the stacks to validate */ readonly stacks?: StackSelector; + + /** + * Submit templates to CloudFormation for early validation. + * + * Creates a non-executing change set per stack and reports any + * early validation errors (invalid resource types, property validation, name conflicts). + * Requires AWS credentials. + * + * @default true + */ + readonly online?: boolean; } /** diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts index 9a541db93..0f1fcad6e 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts @@ -13,6 +13,7 @@ import { } from '@aws-sdk/client-cloudformation'; import { AssetManifestBuilder } from './asset-manifest-builder'; import type { Deployments } from './deployments'; +import type { StackDiagnosis } from '../../actions/diagnose'; import { DeploymentError, ToolkitError } from '../../toolkit/toolkit-error'; import { changeSetNameFromArn, stackNameFromArn } from '../../util/cloudformation'; import type { ICloudFormationClient, SdkProvider } from '../aws-auth/private'; @@ -20,7 +21,6 @@ import type { Template, TemplateBodyParameter, TemplateParameter } from '../clou import { CloudFormationStack, makeBodyParameter } from '../cloudformation'; import { throwDeploymentErrorFromDiagnosis } from '../diagnosing/diagnosis-formatting'; import { CloudFormationStackDiagnoser } from '../diagnosing/stack-diagnoser'; -import type { TargetEnvironment } from '../environment'; import type { IoHelper } from '../io/private'; import type { ResourcesToImport } from '../resource-import'; import { StackArtifactSourceTracer } from '../source-tracing/private/stack-source-tracing'; @@ -89,26 +89,21 @@ async function waitFor( } } +export interface ChangeSetReport { + readonly description: DescribeChangeSetCommandOutput; + readonly diagnosis: StackDiagnosis; +} + /** - * Waits for a ChangeSet to be available for triggering a StackUpdate. - * - * Will return a changeset that is either ready to be executed or has no changes. - * Will throw in other cases. - * - * @param cfn - a CloudFormation client - * @param stackNameOrArn - the name or ARN of the Stack that the ChangeSet belongs to, prefer passing an ARN if available - * @param changeSetNameOrArn - the name or ARN of the ChangeSet, prefer passing an ARN if available - * @param fetchAll - if true, fetches all pages of the ChangeSet before returning. - * - * @returns the CloudFormation description of the ChangeSet + * Waits for a ChangeSet to reach a terminal state and returns the diagnosis without throwing. */ -export async function waitForChangeSet( +export async function waitForChangeSetReport( cfn: ICloudFormationClient, ioHelper: IoHelper, stackNameOrArn: string, changeSetNameOrArn: string, { fetchAll, diagnoser }: { fetchAll: boolean; diagnoser: CloudFormationStackDiagnoser }, -): Promise { +): Promise { const stackDisplayName = stackNameFromArn(stackNameOrArn); const changeSetDisplayName = changeSetNameFromArn(changeSetNameOrArn); await ioHelper.defaults.debug(format('Waiting for changeset %s on stack %s to finish creating...', changeSetDisplayName, stackDisplayName)); @@ -117,19 +112,13 @@ export async function waitForChangeSet( fetchAll, }); - // The following doesn't use a switch because tsc will not allow fall-through, UNLESS it is allows - // EVERYWHERE that uses this library directly or indirectly, which is undesirable. if (description.Status === 'CREATE_PENDING' || description.Status === 'CREATE_IN_PROGRESS') { await ioHelper.defaults.debug(format('Changeset %s on stack %s is still creating', changeSetDisplayName, stackDisplayName)); return undefined; } - const diag = await diagnoser.diagnoseChangeSet(description); - if (diag.type === 'no-problem') { - return description; - } - - throwDeploymentErrorFromDiagnosis(diag); + const diagnosis = await diagnoser.diagnoseChangeSet(description); + return { description, diagnosis }; }); if (!ret) { @@ -139,6 +128,32 @@ export async function waitForChangeSet( return ret; } +/** + * Waits for a ChangeSet to be available for triggering a StackUpdate. + * + * Will return a changeset that is either ready to be executed or has no changes. + * Will throw in other cases. + * + * @param cfn - a CloudFormation client + * @param stackNameOrArn - the name or ARN of the Stack the ChangeSet belongs to, prefer ARN + * @param changeSetNameOrArn - the name or ARN of the ChangeSet, prefer ARN + * @param fetchAll - if true, fetches all pages of the ChangeSet before returning. + * @returns the CloudFormation description of the ChangeSet + */ +export async function waitForChangeSet( + cfn: ICloudFormationClient, + ioHelper: IoHelper, + stackNameOrArn: string, + changeSetNameOrArn: string, + { fetchAll, diagnoser }: { fetchAll: boolean; diagnoser: CloudFormationStackDiagnoser }, +): Promise { + const { description, diagnosis } = await waitForChangeSetReport(cfn, ioHelper, stackNameOrArn, changeSetNameOrArn, { fetchAll, diagnoser }); + if (diagnosis.type !== 'no-problem') { + throwDeploymentErrorFromDiagnosis(diagnosis); + } + return description; +} + export async function waitForChangeSetGone( cfn: ICloudFormationClient, ioHelper: IoHelper, @@ -212,8 +227,7 @@ export async function createDiffChangeSet( options: Omit, ): Promise { try { - const env = await options.deployments.envs.accessStackForMutableStackOperations(options.stack); - return await uploadBodyParameterAndCreateChangeSet(ioHelper, env, { + return await uploadBodyParameterAndCreateChangeSet(ioHelper, { ...options, includeNestedStacks: true, }); @@ -258,14 +272,19 @@ function templatesFromAssetManifestArtifact( return [assetManifest, assets]; } -/** - * Only ever called for 'cdk diff' - */ -async function uploadBodyParameterAndCreateChangeSet( +interface PreparedChangeSetEnv { + cfn: ICloudFormationClient; + bodyParameter: TemplateBodyParameter; + exists: boolean; + executionRoleArn: string | undefined; + diagnoser: CloudFormationStackDiagnoser; +} + +async function prepareChangeSetEnv( ioHelper: IoHelper, - env: TargetEnvironment, - options: PrepareChangeSetOptions, -): Promise { + options: { stack: cxapi.CloudFormationStackArtifact; deployments: Deployments }, +): Promise { + const env = await options.deployments.envs.accessStackForMutableStackOperations(options.stack); await uploadStackTemplateAssets(options.stack, options.deployments); const bodyParameter = await makeBodyParameter( ioHelper, @@ -279,8 +298,24 @@ async function uploadBodyParameterAndCreateChangeSet( // A stack in REVIEW_IN_PROGRESS was created by a previous CREATE changeset // that was never executed. Treat it as non-existent for changeset purposes. const exists = stack.exists && stack.stackStatus.name !== 'REVIEW_IN_PROGRESS' && stack.stackStatus.name !== 'DELETE_IN_PROGRESS'; - const executionRoleArn = await env.replacePlaceholders(options.stack.cloudFormationExecutionRoleArn); + const diagnoser = new CloudFormationStackDiagnoser({ + sdk: env.sdk, + envResources: env.resources, + sourceTracer: new StackArtifactSourceTracer(options.stack), + ioHelper, + topLevelStackHierarchicalId: options.stack.hierarchicalId, + }); + + return { cfn, bodyParameter, exists, executionRoleArn, diagnoser }; +} + +async function uploadBodyParameterAndCreateChangeSet( + ioHelper: IoHelper, + options: PrepareChangeSetOptions, +): Promise { + const { cfn, bodyParameter, exists, executionRoleArn, diagnoser } = await prepareChangeSetEnv(ioHelper, options); + await ioHelper.defaults.info( 'Hold on while we create a read-only change set to get a diff with accurate replacement information (use --method=template to use a less accurate but faster template-only diff)\n', ); @@ -298,13 +333,7 @@ async function uploadBodyParameterAndCreateChangeSet( importExistingResources: options.importExistingResources, includeNestedStacks: options.includeNestedStacks, role: executionRoleArn, - diagnoser: new CloudFormationStackDiagnoser({ - sdk: env.sdk, - envResources: env.resources, - sourceTracer: new StackArtifactSourceTracer(options.stack), - ioHelper, - topLevelStackHierarchicalId: options.stack.hierarchicalId, - }), + diagnoser, }); } @@ -398,6 +427,60 @@ async function createChangeSetAndCleanup( return createdChangeSet; } +/** + * Create a change set for online validation (never executes, returns diagnosis instead of throwing). + * + * Uses the same env preparation as diff, but calls `waitForChangeSetReport` to return + * the diagnosis rather than throwing on failure. Always cleans up the change set afterwards. + */ +export async function createValidationChangeSet( + ioHelper: IoHelper, + options: Omit, +): Promise { + const { cfn, bodyParameter, exists, executionRoleArn, diagnoser } = await prepareChangeSetEnv(ioHelper, options); + const changeSetName = 'cdk-validate-change-set'; + + if (exists) { + await cleanupOldChangeset(cfn, ioHelper, changeSetName, options.stack.stackName); + } + + const templateParams = TemplateParameters.fromTemplate(options.stack.template); + const stackParams = templateParams.supplyAll(options.parameters); + + const changeSet = await cfn.createChangeSet({ + StackName: options.stack.stackName, + ChangeSetName: changeSetName, + ChangeSetType: exists ? 'UPDATE' : 'CREATE', + Description: `CDK Changeset for validation ${options.uuid}`, + ClientToken: `validate${options.uuid}`, + TemplateURL: bodyParameter.TemplateURL, + TemplateBody: bodyParameter.TemplateBody, + Parameters: stackParams.apiParameters, + IncludeNestedStacks: true, + RoleARN: executionRoleArn, + Tags: toCfnTags(options.stack.tags), + Capabilities: ['CAPABILITY_IAM', 'CAPABILITY_NAMED_IAM', 'CAPABILITY_AUTO_EXPAND'], + }); + + const report = await waitForChangeSetReport( + cfn, ioHelper, + changeSet.StackId ?? options.stack.stackName, + changeSet.Id ?? changeSetName, + { fetchAll: false, diagnoser }, + ); + + await cleanupOldChangeset(cfn, ioHelper, changeSet.Id ?? changeSetName, changeSet.StackId ?? options.stack.stackName); + + if (!exists) { + await cfn.deleteStack({ + StackName: changeSet.StackId ?? options.stack.stackName, + ClientRequestToken: randomUUID(), + }); + } + + return report; +} + function toCfnTags(tags: { [id: string]: string }): Tag[] { return Object.entries(tags).map(([k, v]) => ({ Key: k, diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/validate/validate-formatting.ts b/packages/@aws-cdk/toolkit-lib/lib/api/validate/validate-formatting.ts index f63308a88..439eb6d47 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/validate/validate-formatting.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/validate/validate-formatting.ts @@ -1,6 +1,6 @@ import * as path from 'node:path'; -import * as chalk from 'chalk'; import type { PluginReportJson, ViolatingConstructJson } from '@aws-cdk/cloud-assembly-schema'; +import * as chalk from 'chalk'; import type { ValidateResult } from '../../actions/validate'; import type { ActionLessMessage } from '../io/private'; import { IO } from '../io/private'; @@ -111,7 +111,10 @@ function formatConstructInfo(construct: ViolatingConstructJson): string { const logicalId = construct.cloudFormationResource?.logicalId; if (construct.constructPath) { - parts.push(logicalId ? `${chalk.bold(construct.constructPath)} (${logicalId})` : chalk.bold(construct.constructPath)); + parts.push(chalk.bold(construct.constructPath)); + if (logicalId) { + parts.push(logicalId); + } } else if (logicalId) { parts.push(chalk.bold(logicalId)); } @@ -132,8 +135,12 @@ function getLeafLocation(stackTraces: string[] | undefined): string | undefined const lastTrace = stackTraces[stackTraces.length - 1]; const frames = lastTrace.split('\n'); if (frames.length === 0) return undefined; - const leafFrame = frames[0].trim(); - const match = leafFrame.match(/\((.+)\)$/) || leafFrame.match(/at\s+(.+)$/); - const location = match ? match[1] : leafFrame; + + // Find the first frame that's user code (not in node_modules or aws-cdk-lib) + const userFrame = frames.find(f => !f.includes('node_modules') && !f.includes('aws-cdk-lib')); + const frame = (userFrame ?? frames[0]).trim(); + + const match = frame.match(/\((.+)\)$/) || frame.match(/at\s+(.+)$/); + const location = match ? match[1] : frame; return path.isAbsolute(location.split(':')[0]) ? path.relative(process.cwd(), location) : location; } diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 0ee0e6441..5f30a7608 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -1,7 +1,8 @@ import '../private/dispose-polyfill'; +import { randomUUID } from 'node:crypto'; import * as path from 'node:path'; import * as cxapi from '@aws-cdk/cloud-assembly-api'; -import type { FeatureFlagReportProperties, PolicyValidationReportConclusion } from '@aws-cdk/cloud-assembly-schema'; +import type { FeatureFlagReportProperties, PolicyValidationReportConclusion, PluginReportJson } from '@aws-cdk/cloud-assembly-schema'; import { ArtifactType, Manifest } from '@aws-cdk/cloud-assembly-schema'; import type { TemplateDiff } from '@aws-cdk/cloudformation-diff'; import * as chalk from 'chalk'; @@ -79,6 +80,7 @@ import { AsyncDisposableBox } from '../api/cloud-assembly/private/disposable-box import { CloudAssemblySourceBuilder } from '../api/cloud-assembly/source-builder'; import type { StackCollection } from '../api/cloud-assembly/stack-collection'; import { Deployments } from '../api/deployments'; +import { createValidationChangeSet } from '../api/deployments/cfn-api'; import { hostMessageFromDiagnosis } from '../api/diagnosing/diagnosis-formatting'; import { CloudFormationStackDiagnoser } from '../api/diagnosing/stack-diagnoser'; import { DiffFormatter } from '../api/diff'; @@ -89,7 +91,6 @@ 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'; import { ResourceOrphaner } from '../api/orphan/orphaner'; -import { hostMessageFromValidation } from '../api/validate/validate-formatting'; import { parseAndValidateConstructPaths } from '../api/orphan/private/helpers'; import { Mode, PluginHost } from '../api/plugin'; import { @@ -106,6 +107,7 @@ import { ResourceMigrator } from '../api/resource-import'; import { StackArtifactSourceTracer } from '../api/source-tracing/private/stack-source-tracing'; import { tagsForStack } from '../api/tags/private'; import { DEFAULT_TOOLKIT_STACK_NAME } from '../api/toolkit-info'; +import { hostMessageFromValidation } from '../api/validate/validate-formatting'; import type { AssetBuildNode, AssetPublishNode, Concurrency, StackNode } from '../api/work-graph'; import { WorkGraph, WorkGraphBuilder, buildDestroyWorkGraph } from '../api/work-graph'; import type { AssemblyData, RefactorResult, StackDetails, SuccessfulDeployStackResult } from '../payloads'; @@ -664,27 +666,42 @@ export class Toolkit extends CloudAssemblySourceBuilder { const selectStacks = stacksOpt(options); await using assembly = await synthAndMeasure(ioHelper, cx, selectStacks); - const reportPath = path.join(assembly.directory, VALIDATION_REPORT_FILE); + const pluginReports: PluginReportJson[] = []; + let title: string | undefined; - if (!await fs.pathExists(reportPath)) { - const result: ValidateResult = { - conclusion: 'success', - pluginReports: [], - }; + // Offline validation: read the policy validation report from the cloud assembly + const reportPath = path.join(assembly.directory, VALIDATION_REPORT_FILE); + if (await fs.pathExists(reportPath)) { + const report = Manifest.loadValidationReport(reportPath); + title = report.title; + pluginReports.push(...report.pluginReports); + } else if (options.online === false) { await ioHelper.notify(IO.CDK_TOOLKIT_I9601.msg('No validation plugins configured. Add a plugin to your CDK app to enable validation.')); - return result; } - const report = Manifest.loadValidationReport(reportPath); + // Online validation: submit templates to CloudFormation for early validation + if (options.online ?? true) { + const stacks = await assembly.selectStacksV2(selectStacks); + const deployments = await this.deploymentsForAction('validate'); - const conclusion: PolicyValidationReportConclusion = report.pluginReports.some( + const onlineReport = await this.validateOnline(ioHelper, stacks, deployments); + if (onlineReport) { + pluginReports.push(onlineReport); + } + } + + if (pluginReports.length === 0) { + await ioHelper.notify(IO.CDK_TOOLKIT_I9601.msg('No validation plugins configured. Add a plugin to your CDK app to enable validation.')); + } + + const conclusion: PolicyValidationReportConclusion = pluginReports.some( (pr) => pr.conclusion === 'failure', ) ? 'failure' : 'success'; const result: ValidateResult = { conclusion, - title: report.title, - pluginReports: report.pluginReports, + title, + pluginReports, }; await ioHelper.notify(hostMessageFromValidation(result)); @@ -692,6 +709,64 @@ export class Toolkit extends CloudAssemblySourceBuilder { return result; } + private async validateOnline( + ioHelper: IoHelper, + stacks: StackCollection, + deployments: Deployments, + ): Promise { + const violations: PluginReportJson['violations'] = []; + + for (const stack of stacks.stackArtifacts) { + try { + const report = await createValidationChangeSet(ioHelper, { + deployments, + stack, + parameters: {}, + uuid: randomUUID(), + willExecute: false, + failOnError: true, + }); + + if (report.diagnosis.type === 'problem') { + for (const problem of report.diagnosis.problems) { + violations.push({ + ruleName: problem.errorCode ?? 'CloudFormationValidation', + description: problem.message.replace(/\s*\(at\s+\/Resources\/[^)]+\)\s*$/, ''), + severity: 'fatal', + violatingConstructs: [{ + constructPath: problem.sourceTrace?.constructPath ?? (problem.logicalId ? `${stack.hierarchicalId}/${problem.logicalId}` : stack.hierarchicalId), + cloudFormationResource: problem.logicalId ? { + templatePath: `${stack.stackName}.template.json`, + logicalId: problem.logicalId, + } : undefined, + stackTraces: problem.sourceTrace?.creationStackTrace ? [problem.sourceTrace.creationStackTrace.join('\n')] : undefined, + }], + }); + } + } + } catch (e: any) { + violations.push({ + ruleName: 'CloudFormationValidation', + description: e.message, + severity: 'fatal', + violatingConstructs: [{ + constructPath: stack.hierarchicalId, + }], + }); + } + } + + if (violations.length === 0) { + return undefined; + } + + return { + pluginName: 'CloudFormation', + conclusion: 'failure', + violations, + }; + } + /** * Deploy Action * 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 86a8791cd..449e54810 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/validate.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/validate.test.ts @@ -1,11 +1,17 @@ +import * as awsauth from '../../lib/api/aws-auth/private'; import { StackSelectionStrategy } from '../../lib/api/cloud-assembly'; +import * as cfnApi from '../../lib/api/deployments/cfn-api'; import { Toolkit } from '../../lib/toolkit'; import { cdkOutFixture, TestIoHost } from '../_helpers'; +import { MockSdk, restoreSdkMocksToDefault, setDefaultSTSMocks } from '../_helpers/mock-sdk'; let ioHost: TestIoHost; let toolkit: Toolkit; beforeEach(() => { + jest.restoreAllMocks(); + restoreSdkMocksToDefault(); + setDefaultSTSMocks(); ioHost = new TestIoHost(); toolkit = new Toolkit({ ioHost }); }); @@ -13,7 +19,7 @@ beforeEach(() => { describe('validate', () => { test('returns failure when report contains failing plugin', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-validation-report'); - const result = await toolkit.validate(cx); + const result = await toolkit.validate(cx, { online: false }); expect(result.conclusion).toBe('failure'); expect(result.title).toBe('Validation Report'); @@ -27,7 +33,7 @@ describe('validate', () => { test('returns success when all plugins pass', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-passing-validation'); - const result = await toolkit.validate(cx); + const result = await toolkit.validate(cx, { online: false }); expect(result.conclusion).toBe('success'); expect(result.pluginReports).toHaveLength(1); @@ -37,7 +43,7 @@ describe('validate', () => { test('returns success with no reports when no report file exists', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-bucket'); - const result = await toolkit.validate(cx); + const result = await toolkit.validate(cx, { online: false }); expect(result.conclusion).toBe('success'); expect(result.pluginReports).toHaveLength(0); @@ -46,14 +52,14 @@ describe('validate', () => { test('emits info IO message on success', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-passing-validation'); - await toolkit.validate(cx); + await toolkit.validate(cx, { online: false }); ioHost.expectMessage({ containing: 'No problems found', level: 'info' }); }); test('can invoke without options', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-bucket'); - const result = await toolkit.validate(cx); + const result = await toolkit.validate(cx, { online: false }); expect(result.conclusion).toBe('success'); }); @@ -62,6 +68,7 @@ describe('validate', () => { const cx = await cdkOutFixture(toolkit, 'stack-with-validation-report'); const result = await toolkit.validate(cx, { stacks: { strategy: StackSelectionStrategy.ALL_STACKS }, + online: false, }); expect(result.conclusion).toBe('failure'); @@ -69,7 +76,7 @@ describe('validate', () => { test('parses violation details correctly', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-validation-report'); - const result = await toolkit.validate(cx); + const result = await toolkit.validate(cx, { online: false }); const violation = result.pluginReports[0].violations[0]; expect(violation.severity).toBe('error'); @@ -82,7 +89,7 @@ describe('validate', () => { test('includes plugin version in report', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-validation-report'); - const result = await toolkit.validate(cx); + const result = await toolkit.validate(cx, { online: false }); expect(result.pluginReports[0].pluginVersion).toBe('1.0.0'); expect(result.pluginReports[1].pluginVersion).toBeUndefined(); @@ -91,12 +98,12 @@ describe('validate', () => { test('throws on malformed report', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-malformed-validation-report'); - await expect(toolkit.validate(cx)).rejects.toThrow(); + await expect(toolkit.validate(cx, { online: false })).rejects.toThrow(); }); test('parses stack traces correctly', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-validation-report'); - const result = await toolkit.validate(cx); + const result = await toolkit.validate(cx, { online: false }); const construct = result.pluginReports[0].violations[0].violatingConstructs[0]; expect(construct.stackTraces).toBeDefined(); @@ -106,7 +113,7 @@ describe('validate', () => { test('IO message payload contains full ValidateResult', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-validation-report'); - await toolkit.validate(cx); + await toolkit.validate(cx, { online: false }); const msg = ioHost.messages.find( (m) => m.code === 'CDK_TOOLKIT_I9600', @@ -126,7 +133,7 @@ describe('validate', () => { test('handles report with missing title field', async () => { const cx = await cdkOutFixture(toolkit, 'stack-with-no-title-validation'); - const result = await toolkit.validate(cx); + const result = await toolkit.validate(cx, { online: false }); expect(result.conclusion).toBe('failure'); expect(result.title).toBeUndefined(); @@ -134,3 +141,101 @@ describe('validate', () => { expect(result.pluginReports[0].violations[0].ruleName).toBe('no-public-buckets'); }); }); + +describe('validate --online', () => { + beforeEach(() => { + jest.spyOn(awsauth.SdkProvider.prototype, '_makeSdk').mockReturnValue(new MockSdk()); + }); + + test('reports CloudFormation validation errors as a plugin report', async () => { + jest.spyOn(cfnApi, 'createValidationChangeSet').mockResolvedValue({ + description: { $metadata: {} } as any, + diagnosis: { + type: 'problem', + detectedBy: { type: 'early-validation', changeSetName: 'cdk-validate-change-set' }, + problems: [ + { + stackArn: 'arn:aws:cloudformation:us-east-1:123456789012:stack/Stack1', + topLevelStackHierarchicalId: 'Stack1', + parentStackLogicalIds: [], + logicalId: 'BadResource', + resourceType: 'AWS::Fake::DoesNotExist', + message: 'Resource type AWS::Fake::DoesNotExist does not exist', + errorCode: 'InvalidResourceType', + sourceTrace: undefined, + }, + ], + }, + }); + + const cx = await cdkOutFixture(toolkit, 'stack-with-bucket'); + const result = await toolkit.validate(cx, { online: true }); + + expect(result.conclusion).toBe('failure'); + expect(result.pluginReports).toHaveLength(1); + expect(result.pluginReports[0].pluginName).toBe('CloudFormation'); + expect(result.pluginReports[0].conclusion).toBe('failure'); + expect(result.pluginReports[0].violations).toHaveLength(1); + expect(result.pluginReports[0].violations[0].ruleName).toBe('InvalidResourceType'); + expect(result.pluginReports[0].violations[0].description).toBe('Resource type AWS::Fake::DoesNotExist does not exist'); + expect(result.pluginReports[0].violations[0].violatingConstructs[0].cloudFormationResource?.logicalId).toBe('BadResource'); + }); + + test('passes when CloudFormation finds no problems', async () => { + jest.spyOn(cfnApi, 'createValidationChangeSet').mockResolvedValue({ + description: { $metadata: {} } as any, + diagnosis: { type: 'no-problem' }, + }); + + const cx = await cdkOutFixture(toolkit, 'stack-with-bucket'); + const result = await toolkit.validate(cx, { online: true }); + + expect(result.conclusion).toBe('success'); + expect(result.pluginReports).toHaveLength(0); + }); + + test('merges offline and online results', async () => { + jest.spyOn(cfnApi, 'createValidationChangeSet').mockResolvedValue({ + description: { $metadata: {} } as any, + diagnosis: { + type: 'problem', + detectedBy: { type: 'early-validation', changeSetName: 'cdk-validate-change-set' }, + problems: [ + { + stackArn: 'arn:aws:cloudformation:us-east-1:123456789012:stack/Stack1', + topLevelStackHierarchicalId: 'Stack1', + parentStackLogicalIds: [], + logicalId: 'MyBucket', + message: 'Property validation failure', + errorCode: 'InvalidProperty', + sourceTrace: undefined, + }, + ], + }, + }); + + const cx = await cdkOutFixture(toolkit, 'stack-with-validation-report'); + const result = await toolkit.validate(cx, { online: true }); + + expect(result.conclusion).toBe('failure'); + // 2 from offline report + 1 from online + expect(result.pluginReports).toHaveLength(3); + expect(result.pluginReports[0].pluginName).toBe('TestPlugin'); + expect(result.pluginReports[1].pluginName).toBe('Construct Annotations'); + expect(result.pluginReports[2].pluginName).toBe('CloudFormation'); + }); + + test('reports thrown errors as violations', async () => { + jest.spyOn(cfnApi, 'createValidationChangeSet').mockRejectedValue( + new Error('Template format error: Unrecognized resource types: [AWS::Fake::DoesNotExist]'), + ); + + const cx = await cdkOutFixture(toolkit, 'stack-with-bucket'); + const result = await toolkit.validate(cx, { online: true }); + + expect(result.conclusion).toBe('failure'); + expect(result.pluginReports).toHaveLength(1); + expect(result.pluginReports[0].pluginName).toBe('CloudFormation'); + expect(result.pluginReports[0].violations[0].description).toContain('Unrecognized resource types'); + }); +}); diff --git a/packages/@aws-cdk/toolkit-lib/test/api/validate/validate-formatting.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/validate/validate-formatting.test.ts index cf403fb7a..5bd65f77c 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/validate/validate-formatting.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/validate/validate-formatting.test.ts @@ -1,5 +1,5 @@ -import { formatValidateResult } from '../../../lib/api/validate/validate-formatting'; import type { ValidateResult } from '../../../lib/actions/validate'; +import { formatValidateResult } from '../../../lib/api/validate/validate-formatting'; // Disable chalk for predictable assertions process.env.FORCE_COLOR = '0'; diff --git a/packages/aws-cdk/lib/cli/cli-config.ts b/packages/aws-cdk/lib/cli/cli-config.ts index 8e04bba8d..5d9ee3e8a 100644 --- a/packages/aws-cdk/lib/cli/cli-config.ts +++ b/packages/aws-cdk/lib/cli/cli-config.ts @@ -224,7 +224,9 @@ export async function makeConfig(): Promise { }, 'validate': { description: 'Validate synthesized CloudFormation templates against policy rules', - options: {}, + options: { + online: { type: 'boolean', desc: 'Submit templates to CloudFormation for early validation (requires AWS credentials)', default: true }, + }, arg: { name: 'STACKS', variadic: true, diff --git a/packages/aws-cdk/lib/cli/cli-type-registry.json b/packages/aws-cdk/lib/cli/cli-type-registry.json index 33b467b4d..86219b947 100644 --- a/packages/aws-cdk/lib/cli/cli-type-registry.json +++ b/packages/aws-cdk/lib/cli/cli-type-registry.json @@ -592,7 +592,13 @@ }, "validate": { "description": "Validate synthesized CloudFormation templates against policy rules", - "options": {}, + "options": { + "online": { + "type": "boolean", + "desc": "Submit templates to CloudFormation for early validation (requires AWS credentials)", + "default": true + } + }, "arg": { "name": "STACKS", "variadic": true diff --git a/packages/aws-cdk/lib/cli/cli.ts b/packages/aws-cdk/lib/cli/cli.ts index 446e19782..13fa3310a 100644 --- a/packages/aws-cdk/lib/cli/cli.ts +++ b/packages/aws-cdk/lib/cli/cli.ts @@ -450,6 +450,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise): any { desc: 'Create a drift-aware change set that brings actual resource states in line with template definitions', }), ) - .command('validate [STACKS..]', 'Validate synthesized CloudFormation templates against policy rules', (yargs: Argv) => yargs) + .command('validate [STACKS..]', 'Validate synthesized CloudFormation templates against policy rules', (yargs: Argv) => + yargs.option('online', { + default: true, + type: 'boolean', + desc: 'Submit templates to CloudFormation for early validation (requires AWS credentials)', + }), + ) .command('diagnose [STACKS..]', 'Find the root cause(s) of stack deployment failures', (yargs: Argv) => yargs .option('toolkit-stack-name', { diff --git a/packages/aws-cdk/lib/cli/user-input.ts b/packages/aws-cdk/lib/cli/user-input.ts index 3d1d7b981..68d850821 100644 --- a/packages/aws-cdk/lib/cli/user-input.ts +++ b/packages/aws-cdk/lib/cli/user-input.ts @@ -992,6 +992,13 @@ export interface DeployOptions { * @struct */ export interface ValidateOptions { + /** + * Submit templates to CloudFormation for early validation (requires AWS credentials) + * + * @default - true + */ + readonly online?: boolean; + /** * Positional argument for validate */ diff --git a/packages/aws-cdk/test/cli/cli-arguments.test.ts b/packages/aws-cdk/test/cli/cli-arguments.test.ts index 5f81b67c4..797c22a02 100644 --- a/packages/aws-cdk/test/cli/cli-arguments.test.ts +++ b/packages/aws-cdk/test/cli/cli-arguments.test.ts @@ -171,6 +171,7 @@ describe('config', () => { refactor: expect.anything(), cliTelemetry: expect.anything(), diagnose: expect.anything(), + validate: expect.anything(), }); }); }); diff --git a/yarn.lock b/yarn.lock index 606e75dd8..d0b388f41 100644 --- a/yarn.lock +++ b/yarn.lock @@ -234,19 +234,7 @@ __metadata: languageName: node linkType: hard -"@aws-cdk/cloud-assembly-api@npm:^2.2.3": - version: 2.2.4 - resolution: "@aws-cdk/cloud-assembly-api@npm:2.2.4" - dependencies: - jsonschema: "npm:^1.5.0" - semver: "npm:^7.8.0" - peerDependencies: - "@aws-cdk/cloud-assembly-schema": ">=53.25.0" - checksum: 10c0/49ff690c827a1f4795267a9f449d10b2098522f9c55a09d5dc42db93faf9091e7ddef81c37763e2300e4c17d0c0564586fcfc3dad02232fdcdc912d050db7c67 - languageName: node - linkType: hard - -"@aws-cdk/cloud-assembly-api@npm:^2.2.4": +"@aws-cdk/cloud-assembly-api@npm:^2.2.3, @aws-cdk/cloud-assembly-api@npm:^2.2.4": version: 2.2.4 resolution: "@aws-cdk/cloud-assembly-api@npm:2.2.4" dependencies: @@ -298,17 +286,7 @@ __metadata: languageName: unknown linkType: soft -"@aws-cdk/cloud-assembly-schema@npm:^53.21.0": - version: 53.27.0 - resolution: "@aws-cdk/cloud-assembly-schema@npm:53.27.0" - dependencies: - jsonschema: "npm:^1.5.0" - semver: "npm:^7.8.0" - checksum: 10c0/862ac533f5a261e24386fd6eff9cd18dca2be8b2799f2b8d3c3d6df57b3eba01bf78bdb265166466d5fafc6cd2d7d065f1cb0fe175c5b84ef41064fd00ae304e - languageName: node - linkType: hard - -"@aws-cdk/cloud-assembly-schema@npm:^53.25.0": +"@aws-cdk/cloud-assembly-schema@npm:^53.21.0, @aws-cdk/cloud-assembly-schema@npm:^53.25.0": version: 53.27.0 resolution: "@aws-cdk/cloud-assembly-schema@npm:53.27.0" dependencies: