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

Add quotes around AWS variables #1008

Merged
merged 1 commit into from
Apr 12, 2022
Merged

Add quotes around AWS variables #1008

merged 1 commit into from
Apr 12, 2022

Conversation

ctgardner
Copy link
Contributor

All references to AWS variables (eg, AWS::Region) have been enclosed
within quotations.

Omitting the quotes can cause some YAML parsers to fail.

All references to AWS variables (eg, AWS::Region) have been enclosed
within quotations.

Omitting the quotes can cause some YAML parsers to fail.
@ctgardner
Copy link
Contributor Author

When upgrading from v5.7.0 to v5.8.0, one of our tools was failing with a YAML parsing error. We discovered this was caused by the addition of this line to the template:

LocalSecretsBucketRegion: !If [ CreateSecretsBucket, !Ref AWS::Region, !Ref SecretsBucketRegion ],

Specifically, the failure was caused by the second argument. Enclosing AWS::Region within quotes fixes this issue. I've proactively corrected all occurrences of this within the template.

I'm not an expert, but without the quotes this might technically be invalid YAML, which would explain the parsing error.

Copy link
Contributor

@moskyb moskyb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, the joys of YAML parsing 🙃

this LGTM, it shouldn't break anything. i'll merge it in and release it as part of the next patch release, which will be pretty soon. thanks so much for your contribution!

out of curiosity, what was the tool that was failing to parse the yaml? there's quite a bit of variability between yaml parsers unfortunately, but i'd be keen to do some more testing on my end to dig into the problem a bit more.

@ctgardner
Copy link
Contributor Author

@moskyb moskyb merged commit c2c8d0a into buildkite:master Apr 12, 2022
@ctgardner ctgardner deleted the ctgardner/quote-aws-vars branch April 12, 2022 22:55
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.

2 participants