Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ssm): StringParameter accepts ParameterType.AWS_EC2_IMAGE_ID as type #16884

Merged
merged 8 commits into from
Oct 13, 2021
Merged

fix(ssm): StringParameter accepts ParameterType.AWS_EC2_IMAGE_ID as type #16884

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9da68c2
add dataType and expose it in StringParameter
kaizencc Oct 8, 2021
d4cdc6b
change error message
kaizencc Oct 8, 2021
d013fb9
Merge branch 'master' into bug/ssm
kaizencc Oct 11, 2021
c9b9e2f
change type of datatype to ParameterDataType
kaizencc Oct 13, 2021
a110a9c
Merge branch 'master' into bug/ssm
kaizencc Oct 13, 2021
f497030
change none to undefined
kaizencc Oct 13, 2021
95b7469
Merge branch 'bug/ssm' of https://github.com/kaizen3031593/aws-cdk in…
kaizencc Oct 13, 2021
dc768b3
Merge branch 'master' into bug/ssm
mergify[bot] Oct 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-ssm/lib/parameter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,13 @@ export interface StringParameterProps extends ParameterOptions {
* @default ParameterType.STRING
*/
readonly type?: ParameterType;

/**
* The data type of the parameter, such as `text` or `aws:ec2:image`.
*
* @default - 'text'
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
*/
readonly dataType?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about creating a ParameterDataType enum for this property? Just to avoid people having to remember/hard-coded 'aws:ec2:image' in their code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum it is. My original worry was that the docs do not specify that the dataType is exactly text | aws:ec2:image. Theoretically it could accept other data types as well...

But my thoughts are that we are likely covering 99.9% of the use case for dataType with this enum, so I think it is fine.

}

/**
Expand Down Expand Up @@ -436,12 +443,17 @@ export class StringParameter extends ParameterBase implements IStringParameter {
throw new Error('Description cannot be longer than 1024 characters.');
}

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: "aws:ec2:image" instead?');
}

const resource = new ssm.CfnParameter(this, 'Resource', {
allowedPattern: props.allowedPattern,
description: props.description,
name: this.physicalName,
tier: props.tier,
type: props.type || ParameterType.STRING,
dataType: props.dataType,
value: props.stringValue,
});

Expand Down
28 changes: 28 additions & 0 deletions packages/@aws-cdk/aws-ssm/test/parameter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,34 @@ test('creating a String SSM Parameter', () => {
});
});

test('type cannot be specified as AWS_EC2_IMAGE_ID', () => {
// GIVEN
const stack = new cdk.Stack();

// THEN
expect(() => new ssm.StringParameter(stack, 'myParam', {
stringValue: 'myValue',
type: ssm.ParameterType.AWS_EC2_IMAGE_ID,
})).toThrow('The type must either be ParameterType.STRING or ParameterType.STRING_LIST. Did you mean to set dataType: "aws:ec2:image" instead?');
});

test('dataType can be specified', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new ssm.StringParameter(stack, 'myParam', {
stringValue: 'myValue',
dataType: 'aws:ec2:image',
});

// THEN
expect(stack).toHaveResource('AWS::SSM::Parameter', {
Value: 'myValue',
DataType: 'aws:ec2:image',
});
});

test('expect String SSM Parameter to have tier properly set', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down