Skip to content

Commit

Permalink
feat(ssm): throw ValidationError instead of untyped errors (#33067)
Browse files Browse the repository at this point in the history 
### Issue 

`aws-ssm` + friends for #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 basically 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
@kaizencc
kaizencc authored Jan 22, 2025
1 parent 9b4fd6b commit 6677b33
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 26 deletions.
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/.eslintrc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ const enableNoThrowDefaultErrorIn = [
'aws-s3',
'aws-sns',
'aws-sqs',
'aws-ssm',
'aws-ssmcontacts',
'aws-ssmincidents',
'aws-ssmquicksetup',
];
baseConfig.overrides.push({
files: enableNoThrowDefaultErrorIn.map(m => `./${m}/lib/**`),
Expand Down
44 changes: 22 additions & 22 deletions packages/aws-cdk-lib/aws-ssm/lib/parameter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ContextProvider, Fn, IResource, Resource, Stack, Token,
Tokenization,
} from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* An SSM Parameter reference.
Expand Down Expand Up @@ -479,7 +480,7 @@ export class StringParameter extends ParameterBase implements IStringParameter {
*/
public static fromStringParameterArn(scope: Construct, id: string, stringParameterArn: string): IStringParameter {
if (Token.isUnresolved(stringParameterArn)) {
throw new Error('stringParameterArn cannot be an unresolved token');
throw new ValidationError('stringParameterArn cannot be an unresolved token', scope);
}

// has to be the same region
Expand All @@ -488,13 +489,13 @@ export class StringParameter extends ParameterBase implements IStringParameter {
const arnParts = stringParameterArn.split(':');
const stackRegion = Stack.of(scope).region;
if (arnParts.length !== 6) {
throw new Error('unexpected StringParameterArn format');
throw new ValidationError('unexpected StringParameterArn format', scope);
} else if (Token.isUnresolved(stackRegion)) {
// Region is unknown during synthesis, emit a warning for visibility
Annotations.of(scope).addWarningV2('aws-cdk-lib/aws-ssm:crossAccountReferenceSameRegion', 'Cross-account references will only work within the same region');
} else if (!Token.isUnresolved(stackRegion) && arnParts[3] !== stackRegion) {
// If the region is known, it must match the region specified in the ARN string
throw new Error('stringParameterArn must be in the same region as the stack');
throw new ValidationError('stringParameterArn must be in the same region as the stack', scope);
}

const parameterType = ParameterValueType.STRING;
Expand All @@ -516,10 +517,10 @@ export class StringParameter extends ParameterBase implements IStringParameter {
*/
public static fromStringParameterAttributes(scope: Construct, id: string, attrs: StringParameterAttributes): IStringParameter {
if (!attrs.parameterName) {
throw new Error('parameterName cannot be an empty string');
throw new ValidationError('parameterName cannot be an empty string', scope);
}
if (attrs.type && ![ParameterType.STRING, ParameterType.AWS_EC2_IMAGE_ID].includes(attrs.type)) {
throw new Error(`fromStringParameterAttributes does not support ${attrs.type}. Please use ParameterType.STRING or ParameterType.AWS_EC2_IMAGE_ID`);
throw new ValidationError(`fromStringParameterAttributes does not support ${attrs.type}. Please use ParameterType.STRING or ParameterType.AWS_EC2_IMAGE_ID`, scope);
}

const type = attrs.type ?? attrs.valueType ?? ParameterValueType.STRING;
Expand Down Expand Up @@ -627,8 +628,8 @@ export class StringParameter extends ParameterBase implements IStringParameter {
*/
public static valueForTypedStringParameter(scope: Construct, parameterName: string, type = ParameterType.STRING, version?: number): string {
if (type === ParameterType.STRING_LIST) {
throw new Error('valueForTypedStringParameter does not support STRING_LIST, '
+'use valueForTypedListParameter instead');
throw new ValidationError('valueForTypedStringParameter does not support STRING_LIST, '
+'use valueForTypedListParameter instead', scope);
}
const stack = Stack.of(scope);
const id = makeIdentityForImportedValue(parameterName);
Expand Down Expand Up @@ -666,17 +667,17 @@ export class StringParameter extends ParameterBase implements IStringParameter {
});

if (props.allowedPattern) {
_assertValidValue(props.stringValue, props.allowedPattern);
_assertValidValue(this, props.stringValue, props.allowedPattern);
}

validateParameterName(this.physicalName);
validateParameterName(this, this.physicalName);

if (props.description && props.description?.length > 1024) {
throw new Error('Description cannot be longer than 1024 characters.');
throw new ValidationError('Description cannot be longer than 1024 characters.', this);
}

if (props.type && props.type === ParameterType.AWS_EC2_IMAGE_ID) {
throw new Error('The type must either be ParameterType.STRING or ParameterType.STRING_LIST. Did you mean to set dataType: ParameterDataType.AWS_EC2_IMAGE instead?');
throw new ValidationError('The type must either be ParameterType.STRING or ParameterType.STRING_LIST. Did you mean to set dataType: ParameterDataType.AWS_EC2_IMAGE instead?', this);
}

const resource = new ssm.CfnParameter(this, 'Resource', {
Expand Down Expand Up @@ -705,7 +706,6 @@ export class StringParameter extends ParameterBase implements IStringParameter {
* @resource AWS::SSM::Parameter
*/
export class StringListParameter extends ParameterBase implements IStringListParameter {

/**
* Imports an external parameter of type string list.
* Returns a token and should not be parsed.
Expand All @@ -726,7 +726,7 @@ export class StringListParameter extends ParameterBase implements IStringListPar
*/
public static fromListParameterAttributes(scope: Construct, id: string, attrs: ListParameterAttributes): IStringListParameter {
if (!attrs.parameterName) {
throw new Error('parameterName cannot be an empty string');
throw new ValidationError('parameterName cannot be an empty string', scope);
}

const type = attrs.elementType ?? ParameterValueType.STRING;
Expand Down Expand Up @@ -774,17 +774,17 @@ export class StringListParameter extends ParameterBase implements IStringListPar
});

if (props.stringListValue.find(str => !Token.isUnresolved(str) && str.indexOf(',') !== -1)) {
throw new Error('Values of a StringList SSM Parameter cannot contain the \',\' character. Use a string parameter instead.');
throw new ValidationError('Values of a StringList SSM Parameter cannot contain the \',\' character. Use a string parameter instead.', this);
}

if (props.allowedPattern && !Token.isUnresolved(props.stringListValue)) {
props.stringListValue.forEach(str => _assertValidValue(str, props.allowedPattern!));
props.stringListValue.forEach(str => _assertValidValue(this, str, props.allowedPattern!));
}

validateParameterName(this.physicalName);
validateParameterName(this, this.physicalName);

if (props.description && props.description?.length > 1024) {
throw new Error('Description cannot be longer than 1024 characters.');
throw new ValidationError('Description cannot be longer than 1024 characters.', this);
}

const resource = new ssm.CfnParameter(this, 'Resource', {
Expand Down Expand Up @@ -815,26 +815,26 @@ export class StringListParameter extends ParameterBase implements IStringListPar
* @throws if the ``value`` does not conform to the ``allowedPattern`` and neither is an unresolved token (per
* ``cdk.unresolved``).
*/
function _assertValidValue(value: string, allowedPattern: string): void {
function _assertValidValue(scope: Construct, value: string, allowedPattern: string): void {
if (Token.isUnresolved(value) || Token.isUnresolved(allowedPattern)) {
// Unable to perform validations against unresolved tokens
return;
}
if (!new RegExp(allowedPattern).test(value)) {
throw new Error(`The supplied value (${value}) does not match the specified allowedPattern (${allowedPattern})`);
throw new ValidationError(`The supplied value (${value}) does not match the specified allowedPattern (${allowedPattern})`, scope);
}
}

function makeIdentityForImportedValue(parameterName: string) {
return `SsmParameterValue:${parameterName}:C96584B6-F00A-464E-AD19-53AFF4B05118`;
}

function validateParameterName(parameterName: string) {
function validateParameterName(scope: Construct, parameterName: string) {
if (Token.isUnresolved(parameterName)) { return; }
if (parameterName.length > 2048) {
throw new Error('name cannot be longer than 2048 characters.');
throw new ValidationError('name cannot be longer than 2048 characters.', scope);
}
if (!parameterName.match(/^[\/\w.-]+$/)) {
throw new Error(`name must only contain letters, numbers, and the following 4 symbols .-_/; got ${parameterName}`);
throw new ValidationError(`name must only contain letters, numbers, and the following 4 symbols .-_/; got ${parameterName}`, scope);
}
}
9 changes: 5 additions & 4 deletions packages/aws-cdk-lib/aws-ssm/lib/util.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { IConstruct } from 'constructs';
import { ArnFormat, Stack, Token } from '../../core';
import { ValidationError } from '../../core/lib/errors';

export const AUTOGEN_MARKER = '$$autogen$$';

Expand All @@ -19,7 +20,7 @@ export function arnForParameterName(scope: IConstruct, parameterName: string, op
const nameToValidate = physicalName || parameterName;

if (!Token.isUnresolved(nameToValidate) && nameToValidate.includes('/') && !nameToValidate.startsWith('/')) {
throw new Error(`Parameter names must be fully qualified (if they include "/" they must also begin with a "/"): ${nameToValidate}`);
throw new ValidationError(`Parameter names must be fully qualified (if they include "/" they must also begin with a "/"): ${nameToValidate}`, scope);
}

if (isSimpleName()) {
Expand Down Expand Up @@ -48,7 +49,7 @@ export function arnForParameterName(scope: IConstruct, parameterName: string, op
if (!concreteName || Token.isUnresolved(concreteName)) {

if (options.simpleName === undefined) {
throw new Error('Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "simpleName" explicitly');
throw new ValidationError('Unable to determine ARN separator for SSM parameter since the parameter name is an unresolved token. Use "fromAttributes" and specify "simpleName" explicitly', scope);
}

return options.simpleName;
Expand All @@ -60,10 +61,10 @@ export function arnForParameterName(scope: IConstruct, parameterName: string, op
if (options.simpleName !== undefined && options.simpleName !== result) {

if (concreteName === AUTOGEN_MARKER) {
throw new Error('If "parameterName" is not explicitly defined, "simpleName" must be "true" or undefined since auto-generated parameter names always have simple names');
throw new ValidationError('If "parameterName" is not explicitly defined, "simpleName" must be "true" or undefined since auto-generated parameter names always have simple names', scope);
}

throw new Error(`Parameter name "${concreteName}" is ${result ? 'a simple name' : 'not a simple name'}, but "simpleName" was explicitly set to ${options.simpleName}. Either omit it or set it to ${result}`);
throw new ValidationError(`Parameter name "${concreteName}" is ${result ? 'a simple name' : 'not a simple name'}, but "simpleName" was explicitly set to ${options.simpleName}. Either omit it or set it to ${result}`, scope);
}

return result;
Expand Down

0 comments on commit 6677b33

Please sign in to comment.