Skip to content

Commit

Permalink
feat(pipelines): throw ValidationError instead of untyped Errors (#…
Browse files Browse the repository at this point in the history
…33385)

### Issue 

Relates to #32569 

### Description of changes

`ValidationErrors` everywhere

### Describe any new or updated permissions being added

n/a

### Description of how you validated changes

Existing tests. Exemptions granted as this is a refactor of existing code.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
mrgrain authored Feb 11, 2025
1 parent 19cf902 commit 14b1098
Show file tree
Hide file tree
Showing 23 changed files with 103 additions and 89 deletions.
1 change: 1 addition & 0 deletions packages/aws-cdk-lib/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const enableNoThrowDefaultErrorIn = [
'aws-s3objectlambda',
'aws-s3outposts',
'aws-s3tables',
'pipelines',
];
baseConfig.overrides.push({
files: enableNoThrowDefaultErrorIn.map(m => `./${m}/lib/**`),
Expand Down
5 changes: 3 additions & 2 deletions packages/aws-cdk-lib/pipelines/lib/blueprint/file-set.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Step } from './step';
import { UnscopedValidationError } from '../../../core';

/**
* A set of files traveling through the deployment pipeline
Expand Down Expand Up @@ -26,7 +27,7 @@ export class FileSet implements IFileSetProducer {
*/
public get producer() {
if (!this._producer) {
throw new Error(`FileSet '${this.id}' doesn\'t have a producer; call 'fileSet.producedBy()'`);
throw new UnscopedValidationError(`FileSet '${this.id}' doesn\'t have a producer; call 'fileSet.producedBy()'`);
}
return this._producer;
}
Expand All @@ -38,7 +39,7 @@ export class FileSet implements IFileSetProducer {
*/
public producedBy(producer?: Step) {
if (this._producer) {
throw new Error(`FileSet '${this.id}' already has a producer (${this._producer}) while setting producer: ${producer}`);
throw new UnscopedValidationError(`FileSet '${this.id}' already has a producer (${this._producer}) while setting producer: ${producer}`);
}
this._producer = producer;
}
Expand Down
10 changes: 5 additions & 5 deletions packages/aws-cdk-lib/pipelines/lib/blueprint/shell-step.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { FileSet, IFileSetProducer } from './file-set';
import { StackDeployment } from './stack-deployment';
import { Step } from './step';
import { CfnOutput, Stack } from '../../../core';
import { CfnOutput, Stack, UnscopedValidationError } from '../../../core';
import { mapValues } from '../private/javascript';

/**
Expand Down Expand Up @@ -160,20 +160,20 @@ export class ShellStep extends Step {
if (props.input) {
const fileSet = props.input.primaryOutput;
if (!fileSet) {
throw new Error(`'${id}': primary input should be a step that has produced a file set, got ${props.input}`);
throw new UnscopedValidationError(`'${id}': primary input should be a step that has produced a file set, got ${props.input}`);
}
this.addDependencyFileSet(fileSet);
this.inputs.push({ directory: '.', fileSet });
}

for (const [directory, step] of Object.entries(props.additionalInputs ?? {})) {
if (directory === '.') {
throw new Error(`'${id}': input for directory '.' should be passed via 'input' property`);
throw new UnscopedValidationError(`'${id}': input for directory '.' should be passed via 'input' property`);
}

const fileSet = step.primaryOutput;
if (!fileSet) {
throw new Error(`'${id}': additionalInput for directory '${directory}' should be a step that has produced a file set, got ${step}`);
throw new UnscopedValidationError(`'${id}': additionalInput for directory '${directory}' should be a step that has produced a file set, got ${step}`);
}
this.addDependencyFileSet(fileSet);
this.inputs.push({ directory, fileSet });
Expand All @@ -200,7 +200,7 @@ export class ShellStep extends Step {
public primaryOutputDirectory(directory: string): FileSet {
if (this._primaryOutputDirectory !== undefined) {
if (this._primaryOutputDirectory !== directory) {
throw new Error(`${this}: primaryOutputDirectory is '${this._primaryOutputDirectory}', cannot be changed to '${directory}'`);
throw new UnscopedValidationError(`${this}: primaryOutputDirectory is '${this._primaryOutputDirectory}', cannot be changed to '${directory}'`);
}

return this.primaryOutput!;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as path from 'path';
import { AssetType } from './asset-type';
import { Step } from './step';
import { UnscopedValidationError } from '../../../core';
import * as cxapi from '../../../cx-api';
import { AssetManifestReader, DockerImageManifestEntry, FileManifestEntry } from '../private/asset-manifest';
import { isAssetManifest } from '../private/cloud-assembly-internals';
Expand Down Expand Up @@ -308,7 +309,7 @@ function extractStackAssets(stackArtifact: cxapi.CloudFormationStackArtifact): S
isTemplate = entry.source.packaging === 'file' && entry.source.path === stackArtifact.templateFile;
assetType = AssetType.FILE;
} else {
throw new Error(`Unrecognized asset type: ${entry.type}`);
throw new UnscopedValidationError(`Unrecognized asset type: ${entry.type}`);
}

ret.push({
Expand Down
11 changes: 6 additions & 5 deletions packages/aws-cdk-lib/pipelines/lib/blueprint/stage-deployment.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { StackDeployment } from './stack-deployment';
import { StackSteps, Step } from './step';
import * as cdk from '../../../core';
import { ValidationError } from '../../../core';
import { CloudFormationStackArtifact } from '../../../cx-api';
import { isStackArtifact } from '../private/cloud-assembly-internals';
import { pipelineSynth } from '../private/construct-internals';
Expand Down Expand Up @@ -56,13 +57,13 @@ export class StageDeployment {
if (assembly.stacks.length === 0) {
// If we don't check here, a more puzzling "stage contains no actions"
// error will be thrown come deployment time.
throw new Error(`The given Stage construct ('${stage.node.path}') should contain at least one Stack`);
throw new ValidationError(`The given Stage construct ('${stage.node.path}') should contain at least one Stack`, stage);
}

const stepFromArtifact = new Map<CloudFormationStackArtifact, StackDeployment>();
for (const artifact of assembly.stacks) {
if (artifact.assumeRoleAdditionalOptions?.Tags && artifact.assumeRoleArn) {
throw new Error(`Deployment of stack ${artifact.stackName} requires assuming the role ${artifact.assumeRoleArn} with session tags, but assuming roles with session tags is not supported by CodePipeline.`);
throw new ValidationError(`Deployment of stack ${artifact.stackName} requires assuming the role ${artifact.assumeRoleArn} with session tags, but assuming roles with session tags is not supported by CodePipeline.`, stage);
}
const step = StackDeployment.fromArtifact(artifact);
stepFromArtifact.set(artifact, step);
Expand All @@ -72,7 +73,7 @@ export class StageDeployment {
const stackArtifact = assembly.getStackArtifact(stackstep.stack.artifactId);
const thisStep = stepFromArtifact.get(stackArtifact);
if (!thisStep) {
throw new Error('Logic error: we just added a step for this artifact but it disappeared.');
throw new ValidationError('Logic error: we just added a step for this artifact but it disappeared.', stage);
}
thisStep.addStackSteps(stackstep.pre ?? [], stackstep.changeSet ?? [], stackstep.post ?? []);
}
Expand All @@ -81,14 +82,14 @@ export class StageDeployment {
for (const artifact of assembly.stacks) {
const thisStep = stepFromArtifact.get(artifact);
if (!thisStep) {
throw new Error('Logic error: we just added a step for this artifact but it disappeared.');
throw new ValidationError('Logic error: we just added a step for this artifact but it disappeared.', stage);
}

const stackDependencies = artifact.dependencies.filter(isStackArtifact);
for (const dep of stackDependencies) {
const depStep = stepFromArtifact.get(dep);
if (!depStep) {
throw new Error(`Stack '${artifact.id}' depends on stack not found in same Stage: '${dep.id}'`);
throw new ValidationError(`Stack '${artifact.id}' depends on stack not found in same Stage: '${dep.id}'`, stage);
}
thisStep.addStackDependency(depStep);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk-lib/pipelines/lib/blueprint/step.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FileSet, IFileSetProducer } from './file-set';
import { StackOutputReference } from './shell-step';
import { Stack, Token } from '../../../core';
import { Stack, Token, UnscopedValidationError } from '../../../core';
import { StepOutput } from '../helpers-internal/step-output';

/**
Expand Down Expand Up @@ -47,7 +47,7 @@ export abstract class Step implements IFileSetProducer {
/** Identifier for this step */
public readonly id: string) {
if (Token.isUnresolved(id)) {
throw new Error(`Step id cannot be unresolved, got '${id}'`);
throw new UnscopedValidationError(`Step id cannot be unresolved, got '${id}'`);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { makeCodePipelineOutput } from './private/outputs';
import * as codebuild from '../../../aws-codebuild';
import * as ec2 from '../../../aws-ec2';
import * as iam from '../../../aws-iam';
import { Duration } from '../../../core';
import { Duration, UnscopedValidationError } from '../../../core';
import { ShellStep, ShellStepProps } from '../blueprint';

/**
Expand Down Expand Up @@ -261,7 +261,7 @@ export class CodeBuildStep extends ShellStep {
*/
public get project(): codebuild.IProject {
if (!this._project) {
throw new Error('Call pipeline.buildPipeline() before reading this property');
throw new UnscopedValidationError('Call pipeline.buildPipeline() before reading this property');
}
return this._project;
}
Expand Down Expand Up @@ -317,7 +317,7 @@ export class CodeBuildStep extends ShellStep {
*/
public exportedVariable(variableName: string): string {
if (this.exportedVarsRendered && !this.exportedVariables.has(variableName)) {
throw new Error('exportVariable(): Pipeline has already been produced, cannot call this function anymore');
throw new UnscopedValidationError('exportVariable(): Pipeline has already been produced, cannot call this function anymore');
}

this.exportedVariables.add(variableName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Action, CodeCommitTrigger, GitHubTrigger, S3Trigger } from '../../../aw
import { IRepository } from '../../../aws-ecr';
import * as iam from '../../../aws-iam';
import { IBucket } from '../../../aws-s3';
import { Fn, SecretValue, Token } from '../../../core';
import { Fn, SecretValue, Token, UnscopedValidationError } from '../../../core';
import { FileSet, Step } from '../blueprint';

/**
Expand Down Expand Up @@ -257,7 +257,7 @@ class GitHubSource extends CodePipelineSource {

const parts = repoString.split('/');
if (Token.isUnresolved(repoString) || parts.length !== 2) {
throw new Error(`GitHub repository name should be a resolved string like '<owner>/<repo>', got '${repoString}'`);
throw new UnscopedValidationError(`GitHub repository name should be a resolved string like '<owner>/<repo>', got '${repoString}'`);
}
this.owner = parts[0];
this.repo = parts[1];
Expand Down Expand Up @@ -425,7 +425,7 @@ class CodeStarConnectionSource extends CodePipelineSource {
super(repoString);

if (!this.isValidRepoString(repoString)) {
throw new Error(`CodeStar repository name should be a resolved string like '<owner>/<repo>' or '<owner>/<group1>/<group2>/.../<repo>', got '${repoString}'`);
throw new UnscopedValidationError(`CodeStar repository name should be a resolved string like '<owner>/<repo>' or '<owner>/<group1>/<group2>/.../<repo>', got '${repoString}'`);
}

const parts = repoString.split('/');
Expand Down
38 changes: 19 additions & 19 deletions packages/aws-cdk-lib/pipelines/lib/codepipeline/codepipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as cpa from '../../../aws-codepipeline-actions';
import * as ec2 from '../../../aws-ec2';
import * as iam from '../../../aws-iam';
import * as s3 from '../../../aws-s3';
import { Aws, CfnCapabilities, Duration, PhysicalName, Stack, Names, FeatureFlags } from '../../../core';
import { Aws, CfnCapabilities, Duration, PhysicalName, Stack, Names, FeatureFlags, UnscopedValidationError, ValidationError } from '../../../core';
import * as cxapi from '../../../cx-api';
import { AssetType, FileSet, IFileSetProducer, ManualApprovalStep, ShellStep, StackAsset, StackDeployment, Step } from '../blueprint';
import { DockerCredential, dockerCredentialsInstallCommands, DockerCredentialUsage } from '../docker-credentials';
Expand Down Expand Up @@ -400,7 +400,7 @@ export class CodePipeline extends PipelineBase {
*/
public get synthProject(): cb.IProject {
if (!this._synthProject) {
throw new Error('Call pipeline.buildPipeline() before reading this property');
throw new UnscopedValidationError('Call pipeline.buildPipeline() before reading this property');
}
return this._synthProject;
}
Expand All @@ -413,10 +413,10 @@ export class CodePipeline extends PipelineBase {
*/
public get selfMutationProject(): cb.IProject {
if (!this._pipeline) {
throw new Error('Call pipeline.buildPipeline() before reading this property');
throw new UnscopedValidationError('Call pipeline.buildPipeline() before reading this property');
}
if (!this._selfMutationProject) {
throw new Error('No selfMutationProject since the selfMutation property was set to false');
throw new UnscopedValidationError('No selfMutationProject since the selfMutation property was set to false');
}
return this._selfMutationProject;
}
Expand All @@ -428,39 +428,39 @@ export class CodePipeline extends PipelineBase {
*/
public get pipeline(): cp.Pipeline {
if (!this._pipeline) {
throw new Error('Pipeline not created yet');
throw new UnscopedValidationError('Pipeline not created yet');
}
return this._pipeline;
}

protected doBuildPipeline(): void {
if (this._pipeline) {
throw new Error('Pipeline already created');
throw new ValidationError('Pipeline already created', this);
}

this._myCxAsmRoot = path.resolve(assemblyBuilderOf(appOf(this)).outdir);

if (this.props.codePipeline) {
if (this.props.pipelineName) {
throw new Error('Cannot set \'pipelineName\' if an existing CodePipeline is given using \'codePipeline\'');
throw new ValidationError('Cannot set \'pipelineName\' if an existing CodePipeline is given using \'codePipeline\'', this);
}
if (this.props.crossAccountKeys !== undefined) {
throw new Error('Cannot set \'crossAccountKeys\' if an existing CodePipeline is given using \'codePipeline\'');
throw new ValidationError('Cannot set \'crossAccountKeys\' if an existing CodePipeline is given using \'codePipeline\'', this);
}
if (this.props.enableKeyRotation !== undefined) {
throw new Error('Cannot set \'enableKeyRotation\' if an existing CodePipeline is given using \'codePipeline\'');
throw new ValidationError('Cannot set \'enableKeyRotation\' if an existing CodePipeline is given using \'codePipeline\'', this);
}
if (this.props.crossRegionReplicationBuckets !== undefined) {
throw new Error('Cannot set \'crossRegionReplicationBuckets\' if an existing CodePipeline is given using \'codePipeline\'');
throw new ValidationError('Cannot set \'crossRegionReplicationBuckets\' if an existing CodePipeline is given using \'codePipeline\'', this);
}
if (this.props.reuseCrossRegionSupportStacks !== undefined) {
throw new Error('Cannot set \'reuseCrossRegionSupportStacks\' if an existing CodePipeline is given using \'codePipeline\'');
throw new ValidationError('Cannot set \'reuseCrossRegionSupportStacks\' if an existing CodePipeline is given using \'codePipeline\'', this);
}
if (this.props.role !== undefined) {
throw new Error('Cannot set \'role\' if an existing CodePipeline is given using \'codePipeline\'');
throw new ValidationError('Cannot set \'role\' if an existing CodePipeline is given using \'codePipeline\'', this);
}
if (this.props.artifactBucket !== undefined) {
throw new Error('Cannot set \'artifactBucket\' if an existing CodePipeline is given using \'codePipeline\'');
throw new ValidationError('Cannot set \'artifactBucket\' if an existing CodePipeline is given using \'codePipeline\'', this);
}

this._pipeline = this.props.codePipeline;
Expand Down Expand Up @@ -496,7 +496,7 @@ export class CodePipeline extends PipelineBase {

private get myCxAsmRoot(): string {
if (!this._myCxAsmRoot) {
throw new Error('Can\'t read \'myCxAsmRoot\' if build deployment not called yet');
throw new ValidationError('Can\'t read \'myCxAsmRoot\' if build deployment not called yet', this);
}
return this._myCxAsmRoot;
}
Expand All @@ -515,7 +515,7 @@ export class CodePipeline extends PipelineBase {
let beforeSelfMutation = this.selfMutationEnabled;
for (const stageNode of flatten(structure.graph.sortedChildren())) {
if (!isGraph(stageNode)) {
throw new Error(`Top-level children must be graphs, got '${stageNode}'`);
throw new ValidationError(`Top-level children must be graphs, got '${stageNode}'`, this);
}

// Group our ordered tranches into blocks of 50.
Expand Down Expand Up @@ -610,7 +610,7 @@ export class CodePipeline extends PipelineBase {
case 'group':
case 'stack-group':
case undefined:
throw new Error(`actionFromNode: did not expect to get group nodes: ${node.data?.type}`);
throw new ValidationError(`actionFromNode: did not expect to get group nodes: ${node.data?.type}`, this);

case 'self-update':
return this.selfMutateAction();
Expand All @@ -630,7 +630,7 @@ export class CodePipeline extends PipelineBase {
return this.actionFromStep(node, node.data.step);

default:
throw new Error(`CodePipeline does not support graph nodes of type '${node.data?.type}'. You are probably using a feature this CDK Pipelines implementation does not support.`);
throw new ValidationError(`CodePipeline does not support graph nodes of type '${node.data?.type}'. You are probably using a feature this CDK Pipelines implementation does not support.`, this);
}
}

Expand Down Expand Up @@ -679,7 +679,7 @@ export class CodePipeline extends PipelineBase {
};
}

throw new Error(`Deployment step '${step}' is not supported for CodePipeline-backed pipelines`);
throw new ValidationError(`Deployment step '${step}' is not supported for CodePipeline-backed pipelines`, this);
}

private createChangeSetAction(stack: StackDeployment): ICodePipelineActionFactory {
Expand Down Expand Up @@ -824,7 +824,7 @@ export class CodePipeline extends PipelineBase {

const assetType = assets[0].assetType;
if (assets.some(a => a.assetType !== assetType)) {
throw new Error('All assets in a single publishing step must be of the same type');
throw new ValidationError('All assets in a single publishing step must be of the same type', this);
}

const role = this.obtainAssetCodeBuildRole(assets[0].assetType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { CodePipelineActionFactoryResult, ICodePipelineActionFactory, ProduceAct
import { IStage } from '../../../aws-codepipeline';
import * as cpa from '../../../aws-codepipeline-actions';
import * as sns from '../../../aws-sns';
import { Stage } from '../../../core';
import { Stage, ValidationError } from '../../../core';
import { Step } from '../blueprint';
import { ApplicationSecurityCheck } from '../private/application-security-check';

Expand Down Expand Up @@ -76,7 +76,7 @@ export class ConfirmPermissionsBroadening extends Step implements ICodePipelineA
const existing = Node.of(pipeline).tryFindChild(id);
if (existing) {
if (!(existing instanceof ApplicationSecurityCheck)) {
throw new Error(`Expected '${Node.of(existing).path}' to be 'ApplicationSecurityCheck' but was '${existing}'`);
throw new ValidationError(`Expected '${Node.of(existing).path}' to be 'ApplicationSecurityCheck' but was '${existing}'`, pipeline);
}
return existing;
}
Expand Down
Loading

0 comments on commit 14b1098

Please sign in to comment.