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

[v2] Faulty creation of env file #1720

Closed
andrei-dascalu opened this issue May 10, 2021 · 11 comments
Closed

[v2] Faulty creation of env file #1720

andrei-dascalu opened this issue May 10, 2021 · 11 comments
Labels
feature New feature or enhancement request

Comments

@andrei-dascalu
Copy link

I am intercepting a service as such

telepresence intercept <my intercept> --service <my service> -n <namespace> --port 8080:80 --env-file ./.env 

which also dumps the env of the pods serviced by that service.
However, in my case I have a value as follows

RABBITMQ_PASS=9HKCNJY8$cGSanrbJLD7X!ky

In the pod, the value is set as follows

RABBITMQ_PASS="9HKCNJY8$cGSanrbJLD7X!ky"

since the $ is a special shell character. However Telepresence dumps it as-is, without quotes and without escaping it which results in incorrect value passed on to the local service.

I know it may not seem like a big deal, but I've spent a fair amount of time debugging why my services were failing locally.

@thallgren
Copy link
Member

This is the intended and documented behavior. We're using the same format as Docker Compose: https://www.getambassador.io/docs/telepresence/latest/reference/environment/

Also see https://docs.docker.com/compose/env-file/ where you'll find the following text: "There is no special handling of quotation marks. This means that they are part of the VAL."

How are you using the env file?

@andrei-dascalu
Copy link
Author

Hi,

Quotation marks have nothing to do with that, they don't appear in the value as it reaches the pod, I just had to use them when defining the secrets it gets the value from.

In env files, to be able to read the $ correctly, it just needs to be escaped.

The var file is sourced into the environment where my local application runs.

@thallgren
Copy link
Member

The Docker Compose format knows nothing about escaping so If you need a file that you can source into your environment, then you will need to create a proper shell script that contains the variables. I.e. you need to perform the escaping so that the intended shell reads the values correctly. Here's an example in python that does that. It assumes --env-json <file> output.

import json
import shlex

with open("env.json", "r") as jsonEnv:
    envs = json.load(jsonEnv)
    for k, v in envs.items():
        print('{}={}'.format(k, shlex.quote(v)))

@andrei-dascalu
Copy link
Author

andrei-dascalu commented May 11, 2021

Hello,

I'm not sure to what extend Docker Compose "knows nothing about escaping". Docker Compose doesn't use any particular formatting of .env files that's different than regular .env.

A .env file is just a set of key/value pairs that can be sourced. But Docker Compose clearly doesn't just source a .env file plainly because any $ in a .env value will get correctly loaded in a container environment as opposed to simply sourcing the same .env file - so Docker Compose does escape the values in practice.

I'm not sure why your snippet uses json though? The files that telepresence creates are not json files and it's not a matter of quoting, it's a matter of escaping

eg:

MY_PASS=12345$678

should become

MY_PASS=12345\$678

in order for it to be sourced correctly into an environment.
Docker Compose can load an env file that has the former k/v and it produces the correct value.

Any plain shell which does a source .env to load values (the default shell behaviour) needs the latter k/v to work correctly, the former would result in looking for a variable $678.

@thallgren
Copy link
Member

thallgren commented May 11, 2021

Docker treats what's right of the equal sign verbatim, e.g.

MY_PASS="123$678"

is to Docker what

MY_PASS="\"123\$678\""

is to a shell.

The reason I used json in the example is that a) json has a canonical way of representing the strings (no discussion around Docker or shell syntax), and b) it is extremely easy to parse into a dictionary. And yes, telepresence will create a json file if you use --env-json instead of --env-file.

@thallgren
Copy link
Member

I found this blog entry which might bring some more clarity: https://dev.to/tvanantwerp/don-t-quote-environment-variables-in-docker-268h

@andrei-dascalu
Copy link
Author

Ah, I see the point now - thanks for your patience.

My 2 cents are that if I'm expected to use the dumped .env file with a locally running service, then it would be most useful to fully support the use case (as a service running on localhost will read/source by default the file as in a shell).

I respect that t2 may not want to presume how those values will be loaded but if the advertised use case is to be able to run your code locally then this would be a touch of usability that could be optional.

@thallgren
Copy link
Member

I agree. Perhaps we should add a --env-shell in addition to --env-file and --env-json?

I'm actually tempted to also rename --env-file to --env-docker to avoid confusion (and keep --env-file as a deprecated option for backward compatibility).

@andrei-dascalu
Copy link
Author

--env-shell makes sense, as well as the rename definitely clears up the use cases!

@cindymullins-dw cindymullins-dw added the feature New feature or enhancement request label Oct 30, 2023
@alnr
Copy link

alnr commented Aug 1, 2024

Docker treats what's right of the equal sign verbatim, e.g.

MY_PASS="123$678"

is to Docker what

MY_PASS="\"123\$678\""

is to a shell.

The reason I used json in the example is that a) json has a canonical way of representing the strings (no discussion around Docker or shell syntax), and b) it is extremely easy to parse into a dictionary. And yes, telepresence will create a json file if you use --env-json instead of --env-file.

Maybe something has changed in the meantime, but the Docker docs clearly say that the env file does escaping and so on. Am I missing something?

@thallgren thallgren added stale Issue is stale and will be closed and removed stale Issue is stale and will be closed labels Aug 16, 2024
@thallgren
Copy link
Member

@alnr something has indeed changed for docker compose where they have implemented interpolation and escaping. Thanks for pointing this out. Unfortunately, it has not changed for docker run, which can be verified by doing this:

docker run -e 'MY_PASS="123$678"' busybox env | grep MY_PASS

You can note that the output here is echoed verbatim, quotes, dollar sign and all. Putting the declaration in a file and using --env-file will give you the exact same behavior.

A better solution to the problem than what I proposed before (with --env-shell) is probably to add an --env-syntax flag as a complement to the --env-file flag. The syntax flag could then allow for different shells, exported or not exported variables, docker run, or docker compose etc. (I do suspect that "docker compose" uses standard shell escapes).

thallgren added a commit that referenced this issue Aug 25, 2024
A new `--env-syntax <syntax>` was introduced to allow control over the
syntax of the file created when using the flag `--env-file <file>`.
Valid syntaxes are "docker", "compose", "sh", "csh", "cmd", and "ps";
where "sh", "csh", and "ps" can be suffixed with ":export".

Closes #1720

Signed-off-by: Thomas Hallgren <thomas@tada.se>
thallgren added a commit that referenced this issue Aug 25, 2024
A new `--env-syntax <syntax>` was introduced to allow control over the
syntax of the file created when using the flag `--env-file <file>`.
Valid syntaxes are "docker", "compose", "sh", "csh", "cmd", and "ps";
where "sh", "csh", and "ps" can be suffixed with ":export".

Closes #1720

Signed-off-by: Thomas Hallgren <thomas@tada.se>
Signed-off-by: Thomas Hallgren <thomas@datawire.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or enhancement request
Projects
None yet
Development

No branches or pull requests

4 participants