Add env file quotes handling #3660
Conversation
Signed-off-by: Phong Tran <tran.pho@northeastern.edu>
Codecov Report
@@ Coverage Diff @@
## master #3660 +/- ##
=======================================
Coverage 59.02% 59.02%
=======================================
Files 289 289
Lines 24626 24630 +4
=======================================
+ Hits 14535 14539 +4
Misses 9218 9218
Partials 873 873 |
opts/file.go
Outdated
| value := data[1] | ||
| value = strings.TrimLeft(value, quotes) | ||
| value = strings.TrimRight(value, quotes) |
There was a problem hiding this comment.
We should probably make sure that we only trim the "outer" quotes, and only if "left" and "right" have the same quote; I was writing along some ideas for that, and push it as a PR against your branch photra#2
Overall, we should look if there's potential side-effects / changes in behavior because of this though 😅 - perhaps there's other expectations (such as, when using single quotes, don't expand other env-vars inside the value).
I know the original design for the env-file was explicitly to only split on =, and nothing else.
There was a problem hiding this comment.
Given that parseKeyValueFile() is also used in other parts of the code, we may need to check if we want the behavior to change for those areas as well
There was a problem hiding this comment.
That's a good point! I checked parseKeyValueFile() and after evaluating readKVStrings() and ParseEnvFile(), syntactically there shouldn't be side effects given their signatures.
Additionally, I read our documentation on --env-file, and it looks like we do not perform any operations on the variable or its value. Where we do this is within the Dockerfile when using shell form:
"When using the exec form and executing a shell directly, as in the case for the shell form, it is the shell that is doing the environment variable expansion, not docker."
I think we should be OK on env-file behavior, but we should look closer for Dockerfile image building. I'll update here if I find anything. Let me know if you found something!
There was a problem hiding this comment.
👋 Hi guys
Any update on this? If not, I can look into a solution
This makes sure that only the outer-most quotes are trimmed, and that quotes are only trimmed if equal (start/end). Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…gestions use trimQuotes() for trimming quotes
|
This is sitting here for 2 years now? |
- What I did
Closes #3630
- How I did it
Added quotes trimming for env files
- How to verify it
Run the updated unit test TestParseEnvFileGoodFile
- Description for the changelog
Added --env-file quotes handling
- A picture of a cute animal (not mandatory but encouraged)