Skip to content
This repository has been archived by the owner on Apr 16, 2022. It is now read-only.

Add support for recursive resolving #177

Merged
merged 4 commits into from Jul 18, 2018
Merged

Conversation

martysweet
Copy link
Owner

Add support for recursive resolving, which allows for Fn::If parameters to be resolved for whole blocks which depend on a Condition, see #106

@martysweet martysweet added in-progress Work is being done WIP and removed in-progress Work is being done labels Jun 30, 2018
@martysweet
Copy link
Owner Author

@RazzM13 Does this look sane?

@RazzM13
Copy link
Contributor

RazzM13 commented Jun 30, 2018

Hi @martysweet, sorry I've been rather busy lately and I haven't really had the time to take a proper look into this but based on a quick glance, I believe that your solution tackles the problem from a different angle than what I had anticipated. I was under the impression that if Fn::If resolves with AWS::NoValue then the property that contains the intrinsic should be removed, as stated here. Then again, I haven't really had a proper look into this so please ignore if it doesn't make sense.

@martysweet
Copy link
Owner Author

Yep so there are two issues here:

  1. The successful branch of the IF statement was not being resolved, so its possible to put whole nested chunks of resources in here with more intrinsics, these were being thrown back out on a successful IF but caused issue cause nothing was done to resolve them

  2. When a NoValue was set by the user, the return value of the IF statement was undefined, this was passed all the way back and being checked by one of the type checkers, instead of being removed and thus throwing an error. Prehaps removing the item if it's undefined would be better, not sure the best place to put that though.

@martysweet martysweet added this to the v1.7.4 milestone Jul 5, 2018
@martysweet martysweet removed the WIP label Jul 18, 2018
@martysweet martysweet merged commit 42b75e1 into master Jul 18, 2018
@martysweet martysweet deleted the fix-condition-issue-106 branch July 18, 2018 05:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants