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

core(cli): hotswap to be more tolerant of the Fn::ImportValue usage #21320

Closed
2 tasks
huyphan opened this issue Jul 25, 2022 · 2 comments · Fixed by #27292
Closed
2 tasks

core(cli): hotswap to be more tolerant of the Fn::ImportValue usage #21320

huyphan opened this issue Jul 25, 2022 · 2 comments · Fixed by #27292
Labels
@aws-cdk/core Related to core CDK functionality effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1

Comments

@huyphan
Copy link
Contributor

huyphan commented Jul 25, 2022 •
edited
Loading

Describe the feature

The CDK CLI will refuse to hotswap a resource if its definition contains the intrinsic function Fn::Import.

Could not perform a hotswap deployment, because the CloudFormation template could not be resolved: CloudFormation function Fn::ImportValue is not supported

This is because the hotswap feature needs the template to be fully resolved in order to correctly determine the changes; and Fn::ImportValue is not supported by CDK's resolver function.

The hotswap feature should be more tolerant of the Fn::ImportValue usage. It should accept the case where Fn::ImportValue is use and the import name has not changed. This is the case where we can safely assume that the value will not change (thanks to CloudFormation's protection around exported/imported names). To be more specific on the expected behavior:

  • If the imported value needs to be used in the request of the hotswap operation, CDK should try to fetch the value
  • If the imported value does not need to be in the request, CDK can just ignore it.

Use Case

Fn::ImportValue is commonly used by CDK applications. Some users may be using it without knowing it. But as soon as their template uses this intrinsic function, hotswap will stop working.

Proposed Solution

No response

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.33.0 (build 859272d)

Environment details (OS name and version, etc.)

Darwin Kernel Version 21.6.0
@huyphan huyphan added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jul 25, 2022
@github-actions github-actions bot added the @aws-cdk/core Related to core CDK functionality label Jul 25, 2022
@rix0rrr rix0rrr added effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 25, 2022
@rix0rrr rix0rrr removed their assignment Jul 25, 2022
@mrgrain mrgrain self-assigned this Aug 11, 2022
@mrgrain mrgrain removed their assignment Aug 23, 2022
@tomwwright tomwwright mentioned this issue Sep 26, 2023
Merged
3 tasks
@tomwwright
Copy link
Contributor

I'm working on an implementation of this here: #27292

@mergify mergify bot closed this as completed in #27292 Oct 19, 2023
mergify bot pushed a commit that referenced this issue Oct 19, 2023
@tomwwright
feat(cli): support Fn::ImportValue intrinsic function for hotswap d…
a54ea0f 
…eployments (#27292)

## Purpose 🎯 

Extend the `EvaluateCloudFormationTemplate` class to support the `Fn::ImportValue` intrinsic function. This allows for more diverse templates to be evaluated for the purposes of determining eligibility for `--hotswap` deployments

Closes #21320

## Approach 🧠 

Implement `LazyLookupExport` in similar fashion to `LazyListStackResources` to cache required CloudFormation API calls _(preference was to implement using a generator function instead so style is not entirely consistent, is this an issue?)_

Add some basic unit tests for `EvaluateCloudFormationTemplate.evaluateCfnExpression()` is they were absent, then add some tests for `Fn::ImportValue`

## Todo 📝 

- [x] Update doco where appropriate
- [x] Add to hotswap deployment tests
- [x] Look for appropriate integration tests to update

----

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

@bellomal bellomal mentioned this issue Feb 22, 2024
Open
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/core Related to core CDK functionality effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(cli): support Fn::ImportValue intrinsic function for hotswap deployments tomwwright/aws-cdk
4 participants
@huyphan @mrgrain @rix0rrr @tomwwright