docker / compose

Define and run multi-container applications with Docker
https://docs.docker.com/compose/
Apache License 2.0
34.11k stars 5.25k forks source link

Support remote DOCKER_HOST by copying configs and secrets files instead of bind mounting them #11867

Open andoks opened 5 months ago

andoks commented 5 months ago

Description

Per title, currently docker compose uses bind mounts to put configs and secrets in containers, but this does not work when using either docker context or DOCKER_HOST variable to remotely deploy to a different system.

This would help some of the use cases described in these issues:

It builds upon the config content and environment support added in https://github.com/compose-spec/compose-spec/issues/346

andoks commented 5 months ago

A suggestion for how to fix this, see the following attached patch that should apply to main branch 0001-Enable-config.file-s-on-remote-docker-hosts.patch.gz (had to gzip the file, as it would not upload as a plain .patch file)

I can provide the change as a pull-request if necessary.

andoks commented 5 months ago

Two alternatives if this change in existing behaviour is undesireable, could be to make the config.driver = "copy" change to this new behaviour, or support a new config.copy property for copying the file into the container/service.

@ndeloof hope it is ok I am tagging you here, as I saw you were involved in the https://github.com/compose-spec/compose-spec/issues/346 issue

ndeloof commented 5 months ago

I fully agree with this direction. Regarding change in behavior, I don't feel like this is an issue as long as this is documented in release note. A trivial workaround for those who require dynamic sync with container would be to declare an additional bind mount or to rely on a dedicated watch trigger to sync changes

ndeloof commented 5 months ago

see the following attached patch

Please open a pull-request

ndeloof commented 5 months ago

related: https://github.com/docker/compose/issues/9648#issuecomment-1557068598

andoks commented 5 months ago

see the following attached patch

Please open a pull-request

@ndeloof: done -> https://github.com/docker/compose/pull/11871

LawrenceWarren commented 5 months ago

This is a good feature that lends itself to a GitOps approach to remote Docker Compose deployments. However I think this should be a toggle, especially with secrets.

For example, in my workflow I have various docker-compose.ymls each in different directories, with each directory corresponding to a remote server. As a very basic example:

infrastructure-as-code
├── dev
│   └── instances
│       ├── dev-server-1
│       │   └── docker-compose.yml
│       └── dev-server-2
│           └── docker-compose.yml
└── prod
    └── instances
        ├── prod-server-1
        │   └── docker-compose.yml
        └── prod-server-2
            └── docker-compose.yml

This is all Git tracked as a single source of truth to track server state and changes to deployments over time. By being able to use configs from my localhost, we can also track Docker config changes in Git. However, it doesn't make sense to track secrets in the Git repository, as they are meant to be secret - it would be more ideal for my workflow to bind/copy secrets from DOCKER_HOST.

I think an attribute such as from_docker_host would be useful, so you can explicitly choose where the config/secret files will be found when using Docker contexts, and it is flexible around different workflows people may use:

configs:
  my_config:
    file: ./service.conf
    from_docker_host: false # Will search my localhost for service.conf

secrets:
  my_secret:
    file: /etc/my-docker-secerts/private.pem
    from_docker_host: true # Will search DOCKER_HOST for /etc/my-docker-secerts/private.pem
andoks commented 5 months ago

@LawrenceWarren: I can see the usecase for that, but IMO that is an entire new feature, with associated tasks. This issue and the connected MR is from my perspective more of a bugfix to make the existing file stanza work at all when interacting with a remote host. However, is the second example of yours (the my_secret one) possible to do with the current bind mount implementation, and this MR would break that use case?

LawrenceWarren commented 5 months ago

@andoks - You're probably right, my idea/request may be outside the scope of this specific issue. I think in general secrets & configs are not quite perfect when it comes to interactions with Docker contexts. Before creating my own issue to discuss this, I searched GitHub for a similar issue and found this.

To your question, yes, my example with secrets does work, and I do currently use it to securely write secrets into containers. In my opinion the current behaviour (reading the file path as a path on DOCKER_HOST, and mounting that file) has the correct effect when it comes to secrets, and this merge request would break this workflow.

ndeloof commented 5 months ago

@LawrenceWarren that's an interesting use-case that demonstrate we can't adopt this feature without considerations for the impacts on existing installations. Bind mounts indeed always target a path on remote Docker Host

LawrenceWarren commented 5 months ago

@ndeloof Switching to copying does not necessarily break the workflow mentioned above, so long as you're still copying from DOCKER_HOST.

Of course, the point of this issue is to use copying so that we can source data from localhost, but that is a breaking change if we make it the default behaviour. I am unsure how many people are using my workflow, but we must assume that there are people out there using these features together in their current format, particularly with secrets, since (in my opinion) reading from DOCKER_HOST is probably the desired effect.

I think having a key to specify where the file is found is a simple solution (and the default should be from_docker_host: true so that existing config does not suddenly have different behaviour), but this expands the scope of the issue and pull request significantly.

ndeloof commented 5 months ago

so long as you're still copying from DOCKER_HOST.

Which is a blocker here: Compose being a client-side command, it can't access files on DOCKER_HOST, can only ask daemon to set a bind mount.

andoks commented 5 months ago

@ndeloof: what do you think about introducing a new key (e.g "copy_file", "local_file" or something else that makes sense) that can do it the new way? Or perhaps by setting file: <some-path> in combination with driver: "copy_to_remote" maybe? I am mostly open to anything as long as it enables use of local (configuration) files while deploying remotely. I assume this may also affect the compose spec, which may require a somewhat holistic view

andoks commented 5 months ago

@LawrenceWarren: is there any particular reason for why you use the secrets stanza instead of bind-mounting your secret explicitly? I would think it could be reasonable to go through with this change if bind-mounting explicitly works, as that seems more in line with the use case you specified above?

Maybe an alternative could be to introduce a new config.mount and secrets.mount that retains the existing logic, although that might be excessive as bind mounting already exists after all.

( Although Hyrums law might mean it is worth conserving the existing behavior regardless)

ndeloof commented 5 months ago

secrets abstraction should be preferred over bind mount as this clarify role of mounts in container. Also will allow to benefit native support for secrets if/once docker engine introduce support for those (even when not running in swarm mode)

LawrenceWarren commented 5 months ago

@andoks I use the secrets stanza for our secrets because... that's what it is for, I suppose. You're right, they could be explicitly mounted as a volume, but this also seems bad as again it would be a breaking change. We can't assume there are no users out there where this wouldn't break their workflow.

andoks commented 5 months ago

secrets abstraction should be preferred over bind mount as this clarify role of mounts in container. Also will allow to benefit native support for secrets if/once docker engine introduce support for those (even when not running in swarm mode)

I agree that using the secrets feature to define secrets makes sense, but the secrets.file (and configs.file) behavior should be well defined should it not? Either it always refers to a local file that resides on the filesystem together with the compose file (the changed behavior), or it is defined to exist on the file system where the deployment is being run (the current implementation). And I guess that should perhaps be included in the spec somehow?

If it is the former, then applying the change, possibly with a new keyword (some suggestions above) for retaining the current functionality could be made, but break existing users until they update to the new method.

If it is the latter, then the current implementation should be retained, and if desirable by upstream, I can change my PR to work with a different configs/secrets property (new configs. property, setting the driver or something else) if it is acceptable to support both ways.

Also, I take it from your comment that docker in swarm mode has some engine-level primitive for secrets that the current compose implementation tries to mimic (currently by bind mounts)?

I guess this might warrant some more input from other docker maintainers?

I will follow the issue, and if it is decided on, I can try to change my implementation accordingly :slightly_smiling_face:

From https://docs.docker.com/compose/use-secrets/ I read it as the files used are local to the docker-compose.yml filesystem since the files are specified relative, but it does not call out anything explicitly (neither does https://docs.docker.com/compose/compose-file/09-secrets/ AFAICS)

ndeloof commented 5 months ago

By allowing paths to be relative, Compose always made the assumption docker daemon is local. This obviously isn't always the case, still this is obvious in the compose model in many places, including here when it comes to accessing secrets/configs files

andoks commented 5 months ago

@ndeloof: do you know what is needed to arrive at a decision on this to move forward?

LawrenceWarren commented 5 months ago

Hi, I hope everyone had a good weekend, just coming in to bump the topic a bit.

I found this issue last week while trying to see if there was already an issue open about writing configs from localhost when DOCKER_HOST is set. While I have thrown a spanner in the works by mentioning my secrets use case, I would still like to see configs from localhost as a viable possibility.

If there were a way to create configs from localhost, while retaining the current behaviour of secrets (and therefore kicking the issue of "native support for secrets" that @ndeloof mentioned down the road) I would be grateful. Docker contexts is a brilliant tool that is 90% of the way there, it would be good to see these changes made.

andoks commented 5 months ago

Hi, I hope everyone had a good weekend...

Thanks, same to you :-)

..., just coming in to bump the topic a bit.

I found this issue last week while trying to see if there was already an issue open about writing configs from localhost when DOCKER_HOST is set. While I have thrown a spanner in the works by mentioning my secrets use case, I would still like to see configs from localhost as a viable possibility.

I realize this might be a figure of speech with regards to the spanner, but I do not think that is the case at all. As with any widely used open source project like this, you are probably just one out of potentially 10's/100's/1000's that use the secrets.file support with the expectation of docker binding the file from a potential remote host, which is very useful to know. At least this way, even if the behavior is changed, the project knows that it will be important to at least document this potentially breaking users, and potentially supply descriptions of workarounds ahead of time for anyone that are affected by this change.

If there were a way to create configs from localhost, while retaining the current behaviour of secrets (and therefore kicking the issue of "native support for secrets" that @ndeloof mentioned down the road) I would be grateful. Docker contexts is a brilliant tool that is 90% of the way there, it would be good to see these changes made.

My 2 cents regarding this: I do not think it is a good idea to make the behavior of secrets and configs differ for consistency, but also with an expectation that there might be users that will be broken by the change to configs anyway, using it to bind configs on remote hosts today.

andoks commented 5 months ago

@ndeloof: sorry to bother you again if you are busy or otherwise preoccupied, but do you know what is needed to move forward with this?

ndeloof commented 5 months ago

time ;)

schaubl commented 4 months ago

Hello @andoks @ndeloof @LawrenceWarren and others,

I'm the author of the PR #11984 which I just realized is a duplicate of #11871 (don't know why I missed it).

I've read carefully all the comments in this issue and below are my 2 cents:

I think the initial implementation of (file-based) configs&secrets in compose was already a kind of a breaking change compared to the implementation in swarm/stack (and the doc). File-based configs&secrets are also discrepant with inlined/env-based configs&secrets.

... There's no technical reason docker engine can't offer secrets/configs natively, just this is guarded by Swarm mode, but AFAIK some discussion happened to get them also available in standalone mode. I can't tell if/when this would take place, but preferably the logic here should consider this may happen ... Source: https://github.com/docker/compose/pull/11931#issuecomment-2219642915

I agree that it would totally make sense to be able to use docker configs&secrets when swarm mode is not enabled. I think one of the reasons they are not available without swarm mode is because they are made to be distributed across swarm nodes. It probably also needs some swarm keys/certificates to decrypt secrets. Considering it could be possible in the future to use docker's native configs&secrets, the source file should be read by the docker compose client because this is how it is implemented in docker swarm/stack and it's not possible AFAIK to ask the docker daemon to create a config/secret from the filesystem where the docker daemon reside.

@LawrenceWarren: is there any particular reason for why you use the secrets stanza instead of bind-mounting your secret explicitly? I would think it could be reasonable to go through with this change if bind-mounting explicitly works, as that seems more in line with the use case you specified above? ... Source: https://github.com/docker/compose/issues/11867#issuecomment-2147927511

Using the current implementation of docker compose secrets is effectively the same as declaring a bind mount. It might give the impression that it adds some security/secrecy by using file-based secrets compared to a standard bind mount but in fact it doesn't.

The proposed change is a breaking change that "fix" the initial breaking change.

My personal opinion is that this change could be done with a proper mention in the release notes explaining how to use bind mounts to migrate in case file-based configs/secrets were used "remotely" (as mentioned here).

LawrenceWarren commented 4 months ago

OK, I'm inclined to agree with you @schaubl because I think being able to copy files from your localhost, even when dockerhost is a different server, is the correct behaviour, and you are right that the same behaviour can be achieved using bind mounts.

However I would be cautious; just because this breaking change is technically a fix (as you put it), it could still break production workflows - in fact, it would break mine. Also, as mentioned above, the secrets stanza is a nice abstraction even if it fundamentally is just the same as a bind mount, so using a bind mount for secrets is definitely a step back.

LaXiS96 commented 2 months ago

I also am in dire need of this feature, as my use case is a Windows development environment and multiple remote Linux Engines.

I feel the client-server architecture of Docker implies this kind of behavior, but it looks like many features expect the daemon to be running locally, which is unintuitive.

domsew commented 4 weeks ago

I would also like to support @schaubl opinion. Important feature of the compose is that we can create reproducible workspaces, that anyone can use without special host machine configurations. Ability to use workspace config / secret files is the last missing block which would make compose agnostic regarding the docker context. For now I have to switch somehow from relative to absolute path depending on context used (and push required files with a different path). Of course such versatilityt can be achieved now with secrets.environment, but it is not super convinient. It also ensures consistency with different compose attributes - enviroment variables, dockerfiles, build contexts - all of these are read locally from current workspace, not remotely. Configs and Secrets should work the same way.

I believe current behaviour should be changed. Current production workflows will be able to be simplified (no need to manually copy files), or straightforward migration to volumes exists.

andoks commented 3 weeks ago

For anyone interested in testing my fix, feel free to pull down the changes in #12251, building is quite straightforward. I'm already using it locally for my own convenience with regards to using file configs and secrets =)