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

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

Open
andoks opened this issue May 30, 2024 · 23 comments

Comments

@andoks
Copy link

andoks commented May 30, 2024

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 compose-spec/compose-spec#346

@andoks
Copy link
Author

andoks commented May 30, 2024

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
Copy link
Author

andoks commented May 30, 2024

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 compose-spec/compose-spec#346 issue

@ndeloof
Copy link
Contributor

ndeloof commented May 31, 2024

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
Copy link
Contributor

ndeloof commented May 31, 2024

see the following attached patch

Please open a pull-request

@ndeloof
Copy link
Contributor

ndeloof commented May 31, 2024

related: #9648 (comment)

andoks added a commit to andoks/compose that referenced this issue May 31, 2024
Copy configs.file's instead of bind-mounting them to make it possible to
use file configs when working with remote docker hosts (like setting
DOCKER_HOST to a ssh address or setting docker context)

implements: docker#11867
@andoks
Copy link
Author

andoks commented May 31, 2024

see the following attached patch

Please open a pull-request

@ndeloof: done -> #11871

andoks added a commit to andoks/compose that referenced this issue Jun 1, 2024
Copy configs.file's and secrets.file's instead of bind-mounting them to
make it possible to use file configs when working with remote docker
hosts (like setting DOCKER_HOST to a ssh address or setting docker
context)

Includes support for config.files and secrets.files as directories.

Note that file.Content as source of secrets is denied elsewhere with the
error "validating docker-compose.yml: secrets.content_secret Additional
property content is not allowed"

implements: docker#11867
@LawrenceWarren
Copy link

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
Copy link
Author

andoks commented Jun 4, 2024

@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
Copy link

@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
Copy link
Contributor

ndeloof commented Jun 4, 2024

@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
Copy link

@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
Copy link
Contributor

ndeloof commented Jun 4, 2024

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
Copy link
Author

andoks commented Jun 4, 2024

@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
Copy link
Author

andoks commented Jun 4, 2024

@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
Copy link
Contributor

ndeloof commented Jun 4, 2024

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
Copy link

LawrenceWarren commented Jun 4, 2024

@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
Copy link
Author

andoks commented Jun 4, 2024

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 🙂

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
Copy link
Contributor

ndeloof commented Jun 4, 2024

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
Copy link
Author

andoks commented Jun 6, 2024

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

@LawrenceWarren
Copy link

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
Copy link
Author

andoks commented Jun 10, 2024

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
Copy link
Author

andoks commented Jun 13, 2024

@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
Copy link
Contributor

ndeloof commented Jun 13, 2024

time ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants