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

Jump to bottom

(aws-ssm): aws-ssm.ParameterType.STRING_LIST value is incorrect #12477

Closed
akuma12 opened this issue Jan 12, 2021 · 2 comments · Fixed by #21880
Closed

(aws-ssm): aws-ssm.ParameterType.STRING_LIST value is incorrect #12477

akuma12 opened this issue Jan 12, 2021 · 2 comments · Fixed by #21880
Assignees
@corymhall
Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@akuma12
Copy link

akuma12 commented Jan 12, 2021

In Python, when trying to get the value of a StringList type SSM Parameter at deploy-time using ssm.StringParameter.value_for_typed_string_parameter(), the enum value for aws-ssm.ParameterType.STRING_LIST returns 'StringList'. This generates a CFN Parameter like:

SsmParameterValueMyParameterC96584B6F00A464EAD1953AFF4B05118Parameter:
    Type: AWS::SSM::Parameter::Value<StringList>
    Default: /my/string/list/parameter

However, AWS::SSM::Parameter::Value<StringList> is not a valid CloudFormation parameter type. It should be AWS::SSM::Parameter::Value<List<String>> or AWS::SSM::Parameter::Value<CommaDelimitedList>

Reproduction Steps

Reference a StringList type SSM parameter like:

my_string_list_parameter = aws_ssm.StringParameter.value_for_typed_string_parameter(
            self,
            parameter_name="/my/string/list/parameter",
            type=aws_ssm.ParameterType.STRING_LIST
        )

What did you expect to happen?

A correct SSM List type parameter is added to the CDK output file, like:

SsmParameterValueMyParameterC96584B6F00A464EAD1953AFF4B05118Parameter:
    Type: AWS::SSM::Parameter::Value<List<String>>
    Default: /my/string/list/parameter

What actually happened?

An invalid SSM parameter type is generated:

SsmParameterValueMyParameterC96584B6F00A464EAD1953AFF4B05118Parameter:
    Type: AWS::SSM::Parameter::Value<StringList>
    Default: /my/string/list/parameter

When I try to deploy this project, I get the error Template format error: Unrecognized parameter type: StringList

Environment

  • CDK CLI Version: 1.83.0
  • Framework Version: 1.83.0
  • Node.js Version: v12.18.3
  • OS: MacOS 10.15.7
  • Language (Version): Python (3.8.5)

This is 🐛 Bug Report
@akuma12 akuma12 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 12, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ssm Related to AWS Systems Manager label Jan 12, 2021
@MrArnoldPalmer MrArnoldPalmer added effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Feb 22, 2021
@MrArnoldPalmer MrArnoldPalmer removed their assignment Jun 21, 2021
@OrEisenberg
Copy link

Any updates on this?

@corymhall corymhall self-assigned this Aug 15, 2022
@corymhall corymhall mentioned this issue Sep 1, 2022
Merged
4 tasks
@mergify mergify bot closed this as completed in #21880 Sep 15, 2022
mergify bot pushed a commit that referenced this issue Sep 15, 2022
@corymhall
feat(ssm): reference existing SSM list parameters (#21880)
8f7ee2b 
This PR fixes some issues with how SSM parameter types are implemented.
Currently this module models a single type of parameter (`ParameterType`
enum) and that type is used to represent _both_ [CloudFormation SSM
Parameter types](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html#aws-ssm-parameter-types)
For example,

```ts
new cdk.CfnParameter(this, 'Param', {
  type: 'AWS::SSM::Parameter::Value<String>', // type
});
```

_and_ the [AWS::SSM::Parameter.type](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ssm-parameter.html#cfn-ssm-parameter-type)
For example,

```ts
new ssm.CfnParameter(this, 'Param', {
  type: 'String',
});
```

This overloading caused the issue in the referenced issue as well as
making it more confusing for the user. For example, You can specify a type when
creating a `StringParameter`, but you shouldn't need to since the only
valid values are `String | StringList` and these are modeled as two
separate classes `StringParameter & StringListParameter`.

To address this, the PR introduces a new enum `ParameterValueType` to
model the CloudFormation SSM Parameter Types. This enum is only used in
the `valueForXXX` and `fromXXX` methods since those return a CFN
parameter.

- Deprecated `ssm.StringParameter.valueForTypedStringParameter` since it
  uses the old overloaded `ParameterType`.
  - Introduce a new `ssm.StringParameter.valueForTypedStringParameterV2`
    that uses the new `ParameterValueType` enum
- Add `ssm.StringListParameter.valueForTypedListParameter`
- Add `ssm.StringListParameter.fromListParameterAttributes`
- Deprecated `StringParameterProps.type` since the value should only be
  `String`.

fix #12477, #14364


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

homakk pushed a commit to homakk/aws-cdk that referenced this issue Dec 1, 2022
@corymhall @madeline-k
feat(ssm): reference existing SSM list parameters (aws#21880)
9979d90 
This PR fixes some issues with how SSM parameter types are implemented.
Currently this module models a single type of parameter (`ParameterType`
enum) and that type is used to represent _both_ [CloudFormation SSM
Parameter types](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html#aws-ssm-parameter-types)
For example,

```ts
new cdk.CfnParameter(this, 'Param', {
  type: 'AWS::SSM::Parameter::Value<String>', // type
});
```

_and_ the [AWS::SSM::Parameter.type](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ssm-parameter.html#cfn-ssm-parameter-type)
For example,

```ts
new ssm.CfnParameter(this, 'Param', {
  type: 'String',
});
```

This overloading caused the issue in the referenced issue as well as
making it more confusing for the user. For example, You can specify a type when
creating a `StringParameter`, but you shouldn't need to since the only
valid values are `String | StringList` and these are modeled as two
separate classes `StringParameter & StringListParameter`.

To address this, the PR introduces a new enum `ParameterValueType` to
model the CloudFormation SSM Parameter Types. This enum is only used in
the `valueForXXX` and `fromXXX` methods since those return a CFN
parameter.

- Deprecated `ssm.StringParameter.valueForTypedStringParameter` since it
  uses the old overloaded `ParameterType`.
  - Introduce a new `ssm.StringParameter.valueForTypedStringParameterV2`
    that uses the new `ParameterValueType` enum
- Add `ssm.StringListParameter.valueForTypedListParameter`
- Add `ssm.StringListParameter.fromListParameterAttributes`
- Deprecated `StringParameterProps.type` since the value should only be
  `String`.

fix aws#12477, aws#14364


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@corymhall corymhall

Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(ssm): reference existing SSM list parameters
4 participants
@akuma12 @MrArnoldPalmer @OrEisenberg @corymhall