Allow to read env from dotenv file

[crazy-max]

Fixes #282

Signed-off-by: CrazyMax [email protected]

[tonistiigi]

@thaJeztah I know there were some weirdnesses in compose env file. Is this the correct way to read that file? Or at least the way we want to support it.

[crazy-max]

.env file processing has been intriduced in docker-compose since 1.7.0 (2016-04-13) and looks for .env file in the directory where it runs and reads any environment variables defined inside through the python-dotenv lib.

In this PR godotenv module has the same behavior as the python lib. The only difference with compose is if the --project-directory option is used in compose, it will resolve the .env file from that path (since 1.23.0).

[thaJeztah]

Sorry for the delay.

Hmm, yes, I have thoughts on .env. I do agree we should try to make bake be compatible with docker-compose build, to allow existing projects to be built, but we should probably have a close look "how" to achieve this.

The .env file in general is a bit problematic (IMO);

1. the .env file is not specific to docker-compose

   A project could happen to have a .env file (for the project itself) that is not intended to be used by Compose. In these situations, users will have to use workarounds to make Compose ignore the file (Feature Request: option to disable reading from .env  compose#6741)

2. the format is under-defined (and validation was too relaxed)

   For example, compose at some point accepted environment-variables with white-space in their name, and lines having export, mostly because validation was lacking, and when validation was added (reject environment variable that contains white spaces compose#6403), this broke existing users (see .env with export Lines No Longer Works compose#6511). While supporting (or "ignoring") export lines could be "ok", it may also set the expectation that other shell-like constructs are possible, but they are not ( docker-compose .env file support dynamic values #3849 compose#7598)

3. the .env file is not associated with a specific docker-compose file

   Not sure what the current state is for compose, but (see docker-compose doesn't load .env file when is run from outside of compose-file location compose#7928 (comment)) docker compose traverses parent directories both for the compose file, and for the .env file, which will result in confusing behavior.

4. Naming

   This is more of an implementation detail in Compose; naming is hard. There's some ambiguity between .env and env_file (equivalent to docker run --env-file). Compose has a top-level --env-file flag that allows specifying an alternative .env file. Often, users confuse the Compose --env-file flag with the docker run --env-file, which has a very different meaning.

[thaJeztah]

So, back to making buildx bake compatible with Docker Compose projects.

• Perhaps we should NOT automatically load .env, but require a path to be specified for a file to load env-vars from. Doing so prevents the ambiguity about "where" to load the .env from (same dir as compose-file?, project-dir? any "parent" dir that contains a .env ?). I think this makes things more flexible (e.g. users can use an .env file with any build-definition file, not just when using docker-compose.yml), and we don't paint ourself in a corner.
• Make sure to document the file-format and behavior; mostly thinking of:
  • wether or not we ignore export FOO=bar lines (do we print a warning? do we silently ignore? do we error out?)
  • wether or not the file support substitution (FOO=${BAR:-hello}), and if so, which variants thereof (e.g. "required" variables)
  • wether or not the file supports multi-line environment variables
  • if we do expand variables, what's the default/behavior if the env-var is not set? (FOO=$NOSUCHVAR); empty string? FOO "not set"?
  • does the format have the same semantics as --env-file? (FOO means "take value from $FOO if it exists, or don't set FOO otherwise, and FOO= means "set $FOO to an empty string)

[tonistiigi]

@crazy-max What do you think about adding a flag instead (and maybe a warning if file exists but flag not used).

@chris-crone Any thoughts about the issues about the format issues of the file @thaJeztah listed? Also, I guess compose itself is not interested in fixing the ambiguity issues?

[crazy-max]

@thaJeztah

Compose has a top-level --env-file flag that allows specifying an alternative .env file.

Didn't know about that flag, that could be confusing as you say.

Perhaps we should NOT automatically load .env, but require a path to be specified for a file to load env-vars from.

I'm ok with a new flag to load env vars to avoid any confusion or misleading about "default" paths. I think we should discuss about that and find a common implementation to use with compose-cli (docker-archive/compose-cli#1152)?

Make sure to document the file-format and behavior; mostly thinking of

Agree about documentation. If we use godotenv lib we can use a similar usage doc as this one to start with.

[tonistiigi]

I think we should also start to move over to https://github.com/compose-spec/compose-go/ . Had a quick look and seems it has some env file support builtin in parser. Not sure if we still want changes for the default .env behavior because of the issues above. I guess https://github.com/docker/cli/tree/master/cli/compose is now deprecated although don't see it mentioned. @ndeloof

[ndeloof]

cli/compose is not deprecated but is only relevant in the context of stack deploy command and should not be used in another context. compose-go has diverged in the meantime to match compose-spec needs, but should be preferred as a "compose library".

[crazy-max]

I think we can close this PR since #541 and use compose-go in a follow-up.