-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Update config/secret handling: Copy files into containers instead of bind mounting #12448
base: main
Are you sure you want to change the base?
Conversation
Question, will this allow file sources to leverage |
…it into the container Signed-off-by: schaubl <schaubl@users.noreply.github.com>
c34fb16
to
d9e531d
Compare
This PR doesn't address comments on #11867 |
@polarathene yes indeed, this is the main reason for this approach to be experimented |
@polarathene @ndeloof Yes,
|
…and secrets Signed-off-by: schaubl <schaubl@users.noreply.github.com>
f9611db
to
b530e6c
Compare
Signed-off-by: schaubl <schaubl@users.noreply.github.com>
b8884c2
to
66579df
Compare
…iner creation Signed-off-by: schaubl <schaubl@users.noreply.github.com>
@ndeloof I think if the goal of a user is to mount files from the (remote) docker host, he should use bind mounts. @andoks @LaXiS96 @domsew @jgraichen @terev @polarathene @0xbase12 I'm mentioning you here because you showed interest into this and you might want to test this branch and then maybe provide feedback. |
Signed-off-by: schaubl <schaubl@users.noreply.github.com>
926b0df
to
0762b94
Compare
values = append(values, v) | ||
} | ||
return values, nil | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious - why not keeping the convention of value, error
instead of value or error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
It's not value or error
but just error
if any, nil
otherwise.
This function now only do checks, it doesn't return any value (its name&definition has been changed accordingly).
See point "9. Return nil for Success" of Best Practices For Error Handling in Go.
Currently, Docker Compose copies a config or secret into the container if the source is an environment variable or inlined content. However, if the source is a file, the file is bind mounted into the container. This approach fails when Docker Compose is invoked on a different filesystem than the Docker daemon host filesystem (e.g., when called from within a container or remotely using the -H option).
This PR changes the behavior for file-based config/secret sources to match the handling of environment variables and inlined content. Specifically, it reads the file from the filesystem where Docker Compose is executed (like Docker Stack) and copies it into the container.
This update enhances the functionality of Docker Compose when run "remotely," providing the ability to push configuration files, a feature previously available only with Docker Stack/Swarm.
What I did:
Notes:
Differences it that this PR checks the validity of the config/secret parameters in
getCreateConfigs
before the container is created where the other one does it after.Also this PR doesn't support directories which correspond to the doc and the behaviour of Docker Configs&Secrets used by Swarm/Stack.