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

[ssm] BucketName: "dummy-value-for-" results in unpredictable behavior #9138

Closed
OlegBoulanov opened this issue Jul 17, 2020 · 5 comments
Closed
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

@OlegBoulanov
Copy link

OlegBoulanov commented Jul 17, 2020

SystemsManager parameter lookup returns "dummy-value-for-XXXX" at first run.
But if used as a part of BucketName, this throws an exception on invalid bucket name

This results in unpredictable behavior

  • synth/deploy may work for several code changes in a row,
  • and fail miserably after cdk context --clear

Reproduction Steps

var bucketName = StringParameter.ValueFromLookup(stack, "/ssm/param/bucketName");
var bucketProps = new BucketProps { BucketName = bucketName, }

Error Log

Unhandled exception. Amazon.JSII.Runtime.JsiiException: Invalid S3 bucket name (value: logs.dummy-value-for-/ssm/param/bucketName)
Bucket name must be at least 3 and no more than 63 characters
Bucket name must only contain lowercase characters and the symbols, period (.) and dash (-) (offset: 21)

Environment

  • cdk --version 1.51.0 (build 8c2d53c)

  • Node.js Version: v12.7.0
  • OS: Windows
  • Language (Version): C#

Other

Can you please check for dummy and not throw from resource constructors?
You do check for tokens already, don't you?


This is 🐛 Bug Report

@OlegBoulanov OlegBoulanov added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 17, 2020
@OlegBoulanov OlegBoulanov changed the title [module] BucketName: "dummy-value-for-" results in unpredictable behavior Jul 17, 2020
@SomayaB SomayaB added @aws-cdk/aws-ssm Related to AWS Systems Manager and removed @aws-cdk/aws-ssm Related to AWS Systems Manager labels Jul 17, 2020
@SomayaB SomayaB changed the title BucketName: "dummy-value-for-" results in unpredictable behavior [ssm] BucketName: "dummy-value-for-" results in unpredictable behavior Jul 17, 2020
@github-actions github-actions bot added the @aws-cdk/aws-ssm Related to AWS Systems Manager label Jul 17, 2020
@MrArnoldPalmer MrArnoldPalmer added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 27, 2020
@MrArnoldPalmer
Copy link
Contributor

related to #8699

@MrArnoldPalmer MrArnoldPalmer added the effort/medium Medium work item – several days of effort label Aug 17, 2020
@MrArnoldPalmer MrArnoldPalmer removed their assignment Jun 21, 2021
@mspolitaev
Copy link

Hi team! Still planned to be fixed or waiting for better times?)

@gel1123
Copy link

gel1123 commented Nov 30, 2021

Almost the same problem occurs when specifying a custom domain in API Gateway.

@jsamarziya
Copy link

A potential workaround might be to use StringParameter.valueForStringParameter() instead of StringParameter.valueFromLookup(). That method returns a token instead of a string, and I found that it worked for me in the case of

s3.Bucket.fromBucketName(stack, "MyBucket", ssm.StringParameter.valueForStringParameter(stack, "myBucketParamName"))

mergify bot pushed a commit that referenced this issue Aug 9, 2022
…mats (#21520)

`StringParameter.valueFromLookup` can be used to retrieve a string
value, but that value could be in any format and there are times where
it is provided as input to properties that require a specific format
(i.e. arn format). Because of the way that lookups are resolved, it is
possible for the initial value to be the dummy value
`dummy-value-for-${parameterName}` which might cause synth errors.

Since there is no way for the CDK to know _how_ you will use the
returned value, we can't really add logic to specifically handle edge
cases. For example, we could have `valueFromLookup` always return a
token, but then it would no longer be able to be used in cases where a
`string` is required. See #8699 (comment)
for a good analysis.

This PR adds documentation instructing users how to handle these use
cases.

closes #8699, #9138


----

### All Submissions:

* [ ] 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

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] 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.

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
…mats (aws#21520)

`StringParameter.valueFromLookup` can be used to retrieve a string
value, but that value could be in any format and there are times where
it is provided as input to properties that require a specific format
(i.e. arn format). Because of the way that lookups are resolved, it is
possible for the initial value to be the dummy value
`dummy-value-for-${parameterName}` which might cause synth errors.

Since there is no way for the CDK to know _how_ you will use the
returned value, we can't really add logic to specifically handle edge
cases. For example, we could have `valueFromLookup` always return a
token, but then it would no longer be able to be used in cases where a
`string` is required. See aws#8699 (comment)
for a good analysis.

This PR adds documentation instructing users how to handle these use
cases.

closes aws#8699, aws#9138


----

### All Submissions:

* [ ] 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

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] 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/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
Development

No branches or pull requests

7 participants