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(manifest): escape env var values while rendering CFN template #1322

Merged
merged 4 commits into from
Aug 25, 2020

Conversation

efekarakus
Copy link
Contributor

Use the %q verb (a double-quoted string safely escaped with Go syntax)
when writing env var values. This ensures that values like "{}" or
on are written as strings instead of a mapping or boolean object.

Fixes #1120, fixes #1292

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

Use the %q verb (a double-quoted string safely escaped with Go syntax)
when writing env var values. This ensures that values like `"{}"` or
`on` are written as strings instead of a mapping or boolean object.

Fixes aws#1120, fixes aws#1292
@efekarakus efekarakus requested a review from a team as a code owner August 25, 2020 00:31
@efekarakus efekarakus requested a review from kohidave August 25, 2020 00:31
@efekarakus efekarakus changed the title fix(manifest): escape environment variable values fix(manifest): escape environment variable values while rendering CFN template Aug 25, 2020
@efekarakus efekarakus changed the title fix(manifest): escape environment variable values while rendering CFN template fix(manifest): escape env var values while rendering CFN template Aug 25, 2020
@efekarakus efekarakus added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Aug 25, 2020
@efekarakus
Copy link
Contributor Author

Added the "do-not-merge" label while running e2e tests.

@kohidave
Copy link
Contributor

I kind of miss our golden file integ tests. They'd compare the packaged cf template to the expected cf template. That'd help with this class of error.

@efekarakus
Copy link
Contributor Author

I kind of miss our golden file integ tests. They'd compare the packaged cf template to the expected cf template. That'd help with this class of error.

I'll use testdata/ while fixing the same bug for pipelines and add integ tests for both

@efekarakus efekarakus added do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. and removed do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. labels Aug 25, 2020
@efekarakus efekarakus removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Aug 25, 2020
Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

LGTM!

@mergify mergify bot merged commit e966463 into aws:mainline Aug 25, 2020
@efekarakus efekarakus deleted the fix-env-var-escape branch January 6, 2021 17:53
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
…s#1322)

Use the %q verb (a double-quoted string safely escaped with Go syntax)
when writing env var values. This ensures that values like `"{}"` or
`on` are written as strings instead of a mapping or boolean object.

Fixes aws#1120, fixes aws#1292

_By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants