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

fix(all): Fn::ImportValue fails during deserialization if it contains nested intrinsic function #581

Merged
merged 12 commits into from
Feb 27, 2024

Conversation

colifran
Copy link
Contributor

@colifran colifran commented Feb 27, 2024

This PR fixes a bug that causes failures if a CloudFormation template contains Fn::ImportValue with a nested intrinsic function as a value.

At a high-level, this bug was a result of the IntrinsicFunction enum representing ImportValue as a string. As a result, nested Fn::ImportValue with a nested intrinsic function will fail to synthesize with the following error message:

Migrate failed for `<stack name>`: stack generation failed due to error '<Resource Path>.Fn::ImportValue: invalid type: map, expected a string at ...'

stack generation failed due to error '<Resource Path>.Fn::ImportValue: invalid type: map, expected a string at ...'

The fix for this bug is to update the IntrinsicFunction enum to represent ImportValue as a ResourceType.

Fixes #29014

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Signed-off-by: Francis <colifran@amazon.com>
@colifran colifran changed the title fix: Fn::ImportValue fails during deserialization if it contains nested intrinsic functions fix: Fn::ImportValue fails during deserialization if it contains nested intrinsic function Feb 27, 2024
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
@colifran colifran marked this pull request as ready for review February 27, 2024 20:09
Signed-off-by: Francis <colifran@amazon.com>
Copy link

codecov bot commented Feb 27, 2024

Codecov Report

Merging #581 (3d6f74f) into main (65fe906) will increase coverage by 0.2%.
The diff coverage is 95.5%.

Additional details and impacted files
Components Coverage Δ
Parser 76.2% <ø> (ø)
Intermediate Representation 79.4% <66.7%> (+0.6%) ⬆️
Synthesizers 87.3% <100.0%> (+0.1%) ⬆️
Other 34.3% <ø> (ø)
@@           Coverage Diff           @@
##            main    #581     +/-   ##
=======================================
+ Coverage   80.8%   81.0%   +0.2%     
=======================================
  Files         27      27             
  Lines       5075    5092     +17     
  Branches    5075    5092     +17     
=======================================
+ Hits        4101    4123     +22     
+ Misses       773     767      -6     
- Partials     201     202      +1     
Files Coverage Δ
src/parser/intrinsics/mod.rs 68.1% <ø> (ø)
src/synthesizer/csharp/mod.rs 85.1% <100.0%> (+0.1%) ⬆️
src/synthesizer/golang/mod.rs 85.0% <100.0%> (+0.1%) ⬆️
src/synthesizer/java/mod.rs 87.8% <100.0%> (+0.1%) ⬆️
src/synthesizer/python/mod.rs 89.1% <100.0%> (+0.1%) ⬆️
src/synthesizer/typescript/mod.rs 89.6% <100.0%> (+<0.1%) ⬆️
src/ir/resources/mod.rs 78.3% <66.7%> (+1.1%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65fe906...3d6f74f. Read the comment docs.

@colifran colifran changed the title fix: Fn::ImportValue fails during deserialization if it contains nested intrinsic function fix(all): Fn::ImportValue fails during deserialization if it contains nested intrinsic function Feb 27, 2024
@cdklabs-automation cdklabs-automation added this pull request to the merge queue Feb 27, 2024
Merged via the queue into main with commit 77ebf1b Feb 27, 2024
7 checks passed
@cdklabs-automation cdklabs-automation deleted the colifran/nested-intrinsics branch February 27, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(cli): cdk migrate fails to process Fn::Sub within Fn::ImportValue
3 participants