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

Hotswap doesn't update SSM parameter environment variables properly #25387

Closed
fongchinghinunsw opened this issue May 2, 2023 · 3 comments · Fixed by #26382
Closed

Hotswap doesn't update SSM parameter environment variables properly #25387

fongchinghinunsw opened this issue May 2, 2023 · 3 comments · Fixed by #26382
Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. hotswap p1

Comments

@fongchinghinunsw
Copy link

fongchinghinunsw commented May 2, 2023

Describe the bug

My lambda function is like the following.
new NodejsFunction(this, "some-id", { environment: { VAR1: StringParameter.valueForStringParameter(this, "/au/parameter/cool-parameter") } })

If I deploy the function before and apply hotswap, the deployment works fine. However, if I add a new environment variable to the function and use hotswap to deploy it. The value of the variable VAR1 is resolved to the path (i.e. /au/parameter/cool-parameter) instead of the true value in the parameter store.

Expected Behavior

CDK should be able to resolve the right SSM parameter value.

Current Behavior

The value of the variable VAR1 is resolved to the path (i.e. /au/parameter/cool-parameter) instead of the true value in the parameter store.

Reproduction Steps

new NodejsFunction(this, "some-id", { environment: { VAR1: StringParameter.valueForStringParameter(this, "/au/parameter/cool-parameter") } })

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.69.0

Framework Version

No response

Node.js Version

16

OS

macOS Ventura 13.2.1

Language

Typescript

Language Version

No response

Other information

No response

@fongchinghinunsw fongchinghinunsw added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 2, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ssm Related to AWS Systems Manager label May 2, 2023
@pahud pahud self-assigned this May 2, 2023
@pahud
Copy link
Contributor

pahud commented May 2, 2023

According to this, I believe hotswap has not supported SSM parameter environment variables yet. I am making this p2 feature request and please help us prioritize by upvotes.

@pahud pahud added p2 feature-request A feature should be added or improved. effort/medium Medium work item – several days of effort hotswap labels May 2, 2023
@pahud pahud removed their assignment May 2, 2023
@pahud pahud removed the needs-triage This issue or PR still needs to be triaged. label May 2, 2023
@peterwoodworth
Copy link
Contributor

@pahud In this use case, they aren't hotswapping an SSM parameter, but are instead using a dynamic references to specify an environment variable on a lambda function. When we're hotswapping, we're not using CloudFormation deploy, we're using the Lambda API

const updateFunctionCodeResponse = await lambda.updateFunctionConfiguration(updateRequest).promise();

I'm not sure there's a good solution here that works both for CloudFormation deployments and for hotswapping

@pahud pahud added p1 and removed bug This issue is a bug. p2 labels May 8, 2023
@mergify mergify bot closed this as completed in #26382 Jul 18, 2023
mergify bot pushed a commit that referenced this issue Jul 18, 2023
…properly (#26382)

Closes #25387
Closes #25483  

The actual value for a CfnParameter backed by a SSM parameter is returned via `ResolvedValue` (and only for SSM parameters, [doc](https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_Parameter.html#API_Parameter_Contents)). We use the field from DescirbeStacks API response to populate `stackParams`, instead of `ParameterValue`, which is just a parameter's name. 

As far as I checked it is not a breaking change. The `parameter` field is not a public API, and internally the values of SSM parameters are only used for hotswap.

----

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

bmoffatt pushed a commit to bmoffatt/aws-cdk that referenced this issue Jul 29, 2023
…properly (aws#26382)

Closes aws#25387
Closes aws#25483  

The actual value for a CfnParameter backed by a SSM parameter is returned via `ResolvedValue` (and only for SSM parameters, [doc](https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_Parameter.html#API_Parameter_Contents)). We use the field from DescirbeStacks API response to populate `stackParams`, instead of `ParameterValue`, which is just a parameter's name. 

As far as I checked it is not a breaking change. The `parameter` field is not a public API, and internally the values of SSM parameters are only used for hotswap.

----

*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 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. hotswap p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants