fix(ssm): `StringParameter.fromSecureStringParameterAttributes` not working without version #22311

dglsparsons · 2022-09-30T16:11:27Z

It is possible to omit the version of an SSM SecureString parameter.

When omitted, the reference generated by CDK results in a ValidationError when applying the changes.

e.g.

Error [ValidationError]: Incorrect format is used in the following SSM reference: [{{resolve:ssm-secure:/some/parameter:}}]

Related to #18729

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- 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

gitpod-io · 2022-09-30T16:11:36Z

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.

PRs must pass status checks before we can provide a meaningful review.

mrgrain · 2022-09-30T16:15:21Z

Nice catch! Would you mind applying the same fix here: https://github.com/aws/aws-cdk/pull/22311/files#diff-c6cc0ad12503f28d8ab4c545f7efa3d406d90dfe0d34b67c2b29ad115e7a889bR671

Pull request has been modified.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.
❌ The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.

PRs must pass status checks before we can provide a meaningful review.

Pull Request updated. Dissmissing previous PRLinter Review.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.
❌ The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.

PRs must pass status checks before we can provide a meaningful review.

Pull request has been modified.

Pull Request updated. Dissmissing previous PRLinter Review.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.
❌ The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.

PRs must pass status checks before we can provide a meaningful review.

Pull Request updated. Dissmissing previous PRLinter Review.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.
❌ The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.

PRs must pass status checks before we can provide a meaningful review.

Pull request has been modified.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.
❌ The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.

PRs must pass status checks before we can provide a meaningful review.

Pull Request updated. Dissmissing previous PRLinter Review.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.
❌ The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.

PRs must pass status checks before we can provide a meaningful review.

Pull Request updated. Dissmissing previous PRLinter Review.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.
❌ The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.

PRs must pass status checks before we can provide a meaningful review.

dglsparsons · 2022-10-01T10:52:33Z

Hey @mrgrain - I'm not sure where you intended to link with that comment, but it just takes me to the changes I've already made.

I can't see anywhere else that doesn't have conditional logic on the version being provided though.

I've fixed the unit test, but can't seem to make the PR title happy for conventional commits, and it's unhappy with me for not updating any snapshots - although they all seem happy...

Would appreciate any pointers here, if you have any! :)

→ yarn integ-runner --directory packages/@aws-cdk/aws-ssm
yarn run v1.22.18
$ ./aws-cdk/node_modules/.bin/integ-runner --directory packages/@aws-cdk/aws-ssm

Verifying integration test snapshots...

  UNCHANGED  integ.parameter-store-string.lit 0.46s
  UNCHANGED  integ.list-parameter 0.559s
  UNCHANGED  integ.parameter.lit 1.152s
  UNCHANGED  integ.parameter-arns 1.161s

Snapshot Results:

Tests:    4 passed, 4 total
✨  Done in 1.66s.

Pull Request updated. Dissmissing previous PRLinter Review.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

mrgrain · 2022-10-01T11:41:01Z

@dglsparsons Apologies, I thought the List one needed a fix as well, but I see now how that's not the case. 🙇🏻

Regarding integration tests, it appears there is no existing integration test that covered omitting version (which makes sense, otherwise it would have failed). So you'll need to add or update one of the integration tests to include the scenario you are fixing.

You were missing a : in your PR title. I fixed that for you and update the title to our preferred phrasing.

Pull Request updated. Dissmissing previous PRLinter Review.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

Pull Request updated. Dissmissing previous PRLinter Review.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

…orking without version It is possible to omit the `version` of an SSM SecureString parameter. When omitted, the reference generated by CDK results in a ValidationError when applying the changes. e.g. ``` Error [ValidationError]: Incorrect format is used in the following SSM reference: [{{resolve:ssm-secure:/some/parameter:}}] ```

Pull request has been modified.

aws-cdk-automation

The Pull Request Linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

dglsparsons · 2022-10-02T10:37:06Z

@mrgrain - it looks like the integration tests don't really exist for SecureString parameters as there is no way to provision them (no construct for SecureString, only String). So I'm a bit unsure on how to proceed.

aws-cdk-automation · 2022-10-02T11:12:43Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: a96f622
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mrgrain · 2022-10-02T16:21:02Z

I see. Could we maybe use an AwsCustomResource to provision them?
https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.custom_resources.AwsCustomResource.html#oncreate

Something like (untested code):

new cr.AwsCustomResource(this, 'GetParameter', {
  onUpdate: { // will also be called for a CREATE event
    service: 'SSM',
    action: 'putParameter',
    parameters: {
      Name: '/My/Secure/Parameter',
      Type: 'SecureString',
      Value: 'abcdef'
    },
    physicalResourceId: cr.PhysicalResourceId.of('/My/Secure/Parameter'),
  },
  onDelete: {
    service: 'SSM',
    action: 'deleteParameter',
    parameters: {
      Name:  new cr.PhysicalResourceIdReference(),
    },
  },
  policy: cr.AwsCustomResourcePolicy.fromSdkCalls({
    resources: cr.AwsCustomResourcePolicy.ANY_RESOURCE,
  }),
});

See also:

aws-cdk-automation · 2022-10-24T00:15:30Z

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

mrgrain · 2022-10-24T14:03:11Z

@dglsparsons Thanks for your original contribution. FYI I've prepared the new PR #22618 that includes your change, as well as a refacor of the SSM Parameter Store integ tests.

dglsparsons · 2022-10-24T14:11:07Z

@dglsparsons Thanks for your original contribution. FYI I've prepared the new PR #22618 that includes your change, as well as a refacor of the SSM Parameter Store integ tests.

That's amazing! Thank you! I've been meaning to carve out some time to address those tests for ages but haven't been able to get around to it :).

mrgrain · 2022-10-24T14:12:45Z

That's amazing! Thank you! I've been meaning to carve out some time to address those tests for ages but haven't been able to get around to it :).

Yeah no worries! It turned out to be a bit more complicated anyway. But I'm glad we've now got proper tests for this. 🎉

mrgrain · 2022-10-24T16:33:57Z

Closing in favour of #22618

…orking without version (#22618) It is possible to omit the `version` of an SSM SecureString parameter. When omitted, the reference generated by CDK results in a ValidationError when applying the changes. e.g. ``` Error [ValidationError]: Incorrect format is used in the following SSM reference: [{{resolve:ssm-secure:/some/parameter:}}] ``` Related to #18729 Replaces #22311 ---- ### 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*

aws-cdk-automation requested a review from a team September 30, 2022 16:11

github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Sep 30, 2022

aws-cdk-automation previously requested changes Sep 30, 2022

View reviewed changes

dglsparsons force-pushed the main branch from add3a81 to ccce54c Compare October 1, 2022 10:20

dglsparsons changed the title ~~Don't specify SSM secure string parameter version when it is not supp…~~ fix(ssm) don't specify secure string parameter version when not supplied Oct 1, 2022

aws-cdk-automation previously requested changes Oct 1, 2022

View reviewed changes

dglsparsons force-pushed the main branch from ccce54c to f03a7bf Compare October 1, 2022 10:22

aws-cdk-automation previously requested changes Oct 1, 2022

View reviewed changes

dglsparsons force-pushed the main branch from f03a7bf to da38cee Compare October 1, 2022 10:38

dglsparsons force-pushed the main branch from da38cee to 6615173 Compare October 1, 2022 10:39

aws-cdk-automation previously requested changes Oct 1, 2022

View reviewed changes

dglsparsons changed the title ~~fix(ssm) don't specify secure string parameter version when not supplied~~ fix(ssm) parameter version incorrectly specied when omitted Oct 1, 2022

aws-cdk-automation previously requested changes Oct 1, 2022

View reviewed changes

mrgrain changed the title ~~fix(ssm) parameter version incorrectly specied when omitted~~ fix(ssm): parameter version incorrectly specified when omitted Oct 1, 2022

aws-cdk-automation previously requested changes Oct 1, 2022

View reviewed changes

mrgrain changed the title ~~fix(ssm): parameter version incorrectly specified when omitted~~ fix(ssm): StringParamter.fromSecureStringParameterAttributes not working without version Oct 1, 2022

aws-cdk-automation previously requested changes Oct 1, 2022

View reviewed changes

dglsparsons changed the title ~~fix(ssm): StringParamter.fromSecureStringParameterAttributes not working without version~~ fix(ssm): StringParameter.fromSecureStringParameterAttributes not working without version Oct 1, 2022

aws-cdk-automation previously requested changes Oct 1, 2022

View reviewed changes

dglsparsons force-pushed the main branch from 6615173 to a96f622 Compare October 2, 2022 10:33

aws-cdk-automation requested changes Oct 2, 2022

View reviewed changes

mrgrain mentioned this pull request Oct 24, 2022

fix(ssm): StringParameter.fromSecureStringParameterAttributes not working without version #22618

Merged

4 tasks

mrgrain closed this Oct 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ssm): `StringParameter.fromSecureStringParameterAttributes` not working without version #22311

fix(ssm): `StringParameter.fromSecureStringParameterAttributes` not working without version #22311

dglsparsons commented Sep 30, 2022 •

edited

Loading

gitpod-io bot commented Sep 30, 2022

aws-cdk-automation left a comment

mrgrain commented Sep 30, 2022

aws-cdk-automation left a comment

aws-cdk-automation left a comment

aws-cdk-automation left a comment

aws-cdk-automation left a comment

aws-cdk-automation left a comment

aws-cdk-automation left a comment

aws-cdk-automation left a comment

dglsparsons commented Oct 1, 2022

aws-cdk-automation left a comment

mrgrain commented Oct 1, 2022 •

edited

Loading

aws-cdk-automation left a comment

aws-cdk-automation left a comment

aws-cdk-automation left a comment

dglsparsons commented Oct 2, 2022

aws-cdk-automation commented Oct 2, 2022

mrgrain commented Oct 2, 2022 •

edited

Loading

aws-cdk-automation commented Oct 24, 2022

mrgrain commented Oct 24, 2022 •

edited

Loading

dglsparsons commented Oct 24, 2022

mrgrain commented Oct 24, 2022

mrgrain commented Oct 24, 2022

fix(ssm): StringParameter.fromSecureStringParameterAttributes not working without version #22311

fix(ssm): StringParameter.fromSecureStringParameterAttributes not working without version #22311

Conversation

dglsparsons commented Sep 30, 2022 • edited Loading

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Sep 30, 2022

aws-cdk-automation left a comment

Choose a reason for hiding this comment

mrgrain commented Sep 30, 2022

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

dglsparsons commented Oct 1, 2022

aws-cdk-automation left a comment

Choose a reason for hiding this comment

mrgrain commented Oct 1, 2022 • edited Loading

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

dglsparsons commented Oct 2, 2022

aws-cdk-automation commented Oct 2, 2022

AWS CodeBuild CI Report

mrgrain commented Oct 2, 2022 • edited Loading

aws-cdk-automation commented Oct 24, 2022

mrgrain commented Oct 24, 2022 • edited Loading

dglsparsons commented Oct 24, 2022

mrgrain commented Oct 24, 2022

mrgrain commented Oct 24, 2022

fix(ssm): `StringParameter.fromSecureStringParameterAttributes` not working without version #22311

fix(ssm): `StringParameter.fromSecureStringParameterAttributes` not working without version #22311

dglsparsons commented Sep 30, 2022 •

edited

Loading

mrgrain commented Oct 1, 2022 •

edited

Loading

mrgrain commented Oct 2, 2022 •

edited

Loading

mrgrain commented Oct 24, 2022 •

edited

Loading