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

CDK Hotswap: Firelens Log Router Method Incompatible With Hotswap #25483

Closed
bw-abrowne opened this issue May 8, 2023 · 2 comments · Fixed by #26382
Closed

CDK Hotswap: Firelens Log Router Method Incompatible With Hotswap #25483

bw-abrowne opened this issue May 8, 2023 · 2 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

@bw-abrowne
Copy link

Describe the bug

When CDK hotswapping a task definition that uses the function obtainDefaultFluentBitECRImage found in firelens-log-router.ts, the use of an image URI which is grabbed from an SSM repository leads to failed deployments. Upon hotswapping, the API call made to ECS simply uses the SSM parameter name (/aws/service/aws-for-fluent-bit:latest) instead of the value of the SSM parameter, as happens in CDK. This makes the function incompatible with CDK hotswap.

Expected Behavior

CDK Hotswap uses a resolvable image URI when making ECS API calls

Current Behavior

CDK Hotswap fails with the error:

Task is stopping
InternalError: failed to create container model: failed to normalize image reference "/aws/service/aws-for-fluent-bit/2.23.4". Launch a new task to retry.

Reproduction Steps

Using a log router which gets a firelens container from obtainDefaultFluentBitECRImage leads to this issue. After deploying a stack, attempt to run a hotswap deployment.

Possible Solution

CDK Hotswap should either resolve ssm parameters in these scenarios such that the image URI is resolved in time for the image field in a task definition to be usable by ECS, or obtainDefaultFluentBitECRImage should change its implementation to be compatible with hotswap. Otherwise, CDK hotswap perhaps could continue to use correct fields from the original/previous deploy when the fields have no diff.

Additional Information/Context

No response

CDK CLI Version

2.78

Framework Version

No response

Node.js Version

16

OS

Mac OSX

Language

Typescript

Language Version

No response

Other information

No response
@bw-abrowne bw-abrowne added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 8, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ssm Related to AWS Systems Manager label May 8, 2023
@pahud
Copy link
Contributor

pahud commented May 8, 2023

Yes, unfortunately hotswap does not resolve the SSM parameter at this moment.

related to #25387

@pahud pahud added hotswap p1 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. and removed needs-triage This issue or PR still needs to be triaged. bug This issue is a bug. labels May 8, 2023
@tmokmss tmokmss mentioned this issue Jul 16, 2023
Merged
@mergify mergify bot closed this as completed in #26382 Jul 18, 2023
mergify bot pushed a commit that referenced this issue Jul 18, 2023
@tmokmss
fix(cli): hotswap doesn't update SSM parameter environment variables …
32654f5 
…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
@tmokmss @bmoffatt
fix(cli): hotswap doesn't update SSM parameter environment variables …
3880087 
…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
Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(cli): hotswap doesn't update SSM parameter environment variables properly tmokmss/aws-cdk
2 participants
@pahud @bw-abrowne