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.

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

(core): SecretValue.ssmSecure not working without version #18729

Closed
dbartholomae opened this issue Jan 28, 2022 · 4 comments · Fixed by #18730
Closed

(core): SecretValue.ssmSecure not working without version #18729

dbartholomae opened this issue Jan 28, 2022 · 4 comments · Fixed by #18730
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@dbartholomae
Copy link
Contributor

What is the problem?

When I try to use SecretValue.ssmSecure('/name'), without specifying a version, I get the following error message:

Incorrect format is used in the following SSM reference: [{{resolve:ssm-secure:/name:}}]

Reproduction Steps

Create an SSM SecureString parameter called /name. Init an app with TypeScript CDK, install "@aws-cdk/aws-amplify-alpha", change the Stack to the following and then cdk deploy:

import { Construct } from "constructs";
import { SecretValue, Stack, StackProps } from "aws-cdk-lib";
import { App, GitHubSourceCodeProvider } from "@aws-cdk/aws-amplify-alpha";

export class TestStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const app = new App(this, "AmplifyApp", {
      appName: "test-app",
      sourceCodeProvider: new GitHubSourceCodeProvider({
        owner: "owner",
        repository: "repo",
        oauthToken: SecretValue.ssmSecure("/name"),
      }),
    });
  }
}

What did you expect to happen?

Deploy should not have trouble receiving the value from Parameter Store.

What actually happened?

Incorrect format is used in the following SSM reference: [{{resolve:ssm-secure:/name:}}]

CDK CLI Version

2.9.0 (build a69725c)

Framework Version

2.9.0

Node.js Version

v16.13.1

OS

Windows 10

Language

Typescript

Language Version

4.5.3

Other information

When I manually changed the following lines

const parts = [parameterName, version ?? ''];
return this.cfnDynamicReference(new CfnDynamicReference(CfnDynamicReferenceService.SSM_SECURE, parts.join(':')));

to

return this.cfnDynamicReference(new CfnDynamicReference(CfnDynamicReferenceService.SSM_SECURE, version ? `${parameterName}:${version}` : parameterName));

it worked, so it seems that the expected form is {{resolve:ssm-secure:/name}} without the colon at the end.

@dbartholomae dbartholomae added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 28, 2022
@github-actions github-actions bot added the @aws-cdk/core Related to core CDK functionality label Jan 28, 2022
@dbartholomae
Copy link
Contributor Author

The original issue was #17091, the PR #18187.

@dbartholomae
Copy link
Contributor Author

What confuses me: Based on CloudFormation documentation, the format with the : at the end should be right. But it is the other one that works for me. So eyes from someone more experienced with this might be helpful.

@dbartholomae
Copy link
Contributor Author

Regradless, I've created #18730, that fixes the issue as described above. This does not fix Parameter.fromSecureStringParameterAttributes. As that method was also changed in the original PR that introduced this feature, I assume it needs to change the same.
I'm happy to look into that and adjust my PR for that as well if required.

@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jan 31, 2022
@rix0rrr rix0rrr removed their assignment Jan 31, 2022
@mergify mergify bot closed this as completed in #18730 Jan 31, 2022
mergify bot pushed a commit that referenced this issue Jan 31, 2022
When no version is specified, the `:` at the end is not allowed.

Closes #18729.


----

*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.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
When no version is specified, the `:` at the end is not allowed.

Closes aws#18729.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
dglsparsons added a commit to dglsparsons/aws-cdk that referenced this issue Oct 1, 2022
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:}}]
```
mergify bot pushed a commit that referenced this issue Oct 25, 2022
…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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants