Fix condition in lambda deploy template #326
Merged
+2
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, the EnableKeyGroup condition in exodus-lambda-deploy.yaml checked that a joined list of keyids was an empty string. However, the default value of keyids is the string "None", so the joined keyids string will never be "". The failure of this condition meant that an attempt was made to create a KeyGroup resource even when no valid IDs existed. This commit corrects the condition to look for the string "None".
rohanpm
approved these changes
Mar 10, 2022
| - !Join ["", !Ref keyids] | ||
| - "" | ||
| EnableKeyGroup: | ||
| !Not [!Equals [!Join ["", !Ref keyids], None]] |
There was a problem hiding this comment.
This probably works but maybe the default value should be an empty list? Not sure how that looks for CommaDelimitedList, perhaps "". For a list of key IDs, "empty list" seems a better default value than either a string "None" or an actual None/null.
There was a problem hiding this comment.
Yeah, any string will work but None is already used in the template for default values so I stuck with that.
It doesn't really make sense as a default for any of the other parameters either. Maybe we should follow up with a PR to update them all.
|
I was actually preferring the following format. But I don't object to this fix if it works. |
dichn
approved these changes
Mar 14, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the EnableKeyGroup condition in exodus-lambda-deploy.yaml
checked that a joined list of keyids was an empty string. However, the
default value of keyids is the string "None", so the joined keyids
string will never be "".
The failure of this condition meant that an attempt was made to create
a KeyGroup resource even when no valid IDs existed.
This commit corrects the condition to look for the string "None".