-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Continuation of the docker secret
storage feature
#6697
Conversation
|
||
This is an example of using the secrets database in building an image that will | ||
have buildtime access to a sensitive file, that will not be committed in the | ||
final image. |
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.
This example does not show how one would use the secrets file, nor how to clean up afterwards - for example - are you expecting users to softlink the secret file to its expected location?
How do you then manage the hidden dependency on a file that is then gone? will a subsequent docker run
give the user a sensible error message (before the container starts?)
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.
it seems useful to me to add a local secret file mapped into my /etc/apt/sources.d
so that I get my local debian repo - but doing so will either leave a dangling sofltlink, or some kind of confusion.
I'm starting to think that secret
is a wrong name for this.
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.
Dangling symlinks, that is what we are expecting. Which matches expectations. If the secret is no longer present, then the dangling link will fail.
@SvenDowideit I am not opposed to splitting the global secrets out to a separate PR, but there is a definite use-case for global secrets, rather than having to explicitily grant secrets for every As for the name "secrets", perhaps there could be other names, though that is the essence of the conveyance. Something like token, keys, and certificates. |
Hm, just some terms that come in mind, for inspiration;
|
thing is - the functionality can inject pretty much anything - you're talking secrets, I'm thinking I can inject my data just as easily. |
@thaJeztah agreed. Something like 'vault' would be closer, to what is happening. @SvenDowideit would that semantic be more inline with the functionality? |
it seems less shifty :) - and if I can tell a container run that I want to put vault item X at location Y in the container, then we're being blatantly obvious about it. ie - when I run a debian container, i might have a vault item or similarly at build time. and in writing that rather obvious documentation - can you please remind me why you're not using |
@SvenDowideit the whole point of naming this "secrets" was to have "build time volumes with a name that doesn't suggest that they should be used for anything but redhat's product key hackery". It's not supposed to be named something friendly. Its specifically intended to be named something that doesn't sound general purpose-y. |
@timthelion if it's only to be used for "redhat's product key hackery", and not to be used for other purposes, should it be implemented at all? should it be documented? Playing devil's advocate here :) |
@thaJeztah There is a non-redhat @shykes approved use for this feature; signing of builds with a private GPG key. You don't want the private key to stay in the docker image once you're done ;) |
please refer to my example of a non-secret use that this feature would be useful for. However - the image portability risks do need to be addressed. One possible resolution is that a secret should replace an existing default file - so that the image works (to whatever degree is reasonable) with the local secret - my apt sources file is a good eg, but also shows that there are flaws (what if I want to add a new file to the with my support hat on - good luck in keeping this feature even a semi-secret - why not design it properly instead? |
@SvenDowideit @timthelion Basically that's what I was pointing at; If it's for internal use only, don't document it and don't make it accessible via the API. However, since it's being developed for external use (red hat, signing builds) - although for very special cases, it must be properly documented, maybe just to point out what it is meant to be used for and what not. Maybe even to warn people that it is an unsupported 'feature'. Hiding things doesn't really work in open-source projects, does it? :) Signing off now, because this is probably heading in the wrong direction, distracting from the main purpose of this issue :) |
rebased |
@vbatts any discussion still going on about the naming of this? Still in favour of |
@thaJeztah there hasn't been much conversation on the naming. I think the bigger hang-up is on the global secrets, rather than just explicit grants. |
@vbatts thx. Just wanted to give the naming a bit of attention before it was merged and I'm too late to mention it 😸 |
meeting notes https://botbot.me/freenode/docker-dev/msg/22757310/ |
Would the local secrets file be something that lives in the repo alongside the Dockerfile? I think this would be useful and allow for reproducible builds by having the secrets file be under source control. The secrets file could be encrypted when on disk (like ansible vault. Then for |
@defunctzombie not. That is both the benefit and drawback of the concept of secrets. That they are managed independently and subject to change. |
The secrets store keeps track of a set of named secrets, such as API keys or ssh keys that you can later access from containers. There are two types of secrets, the host based ones, which are stored in /etc/docker/secrets, and are automatically applied to all containers, and user secrets which you can add via "docker secret add <name> <file>" and which can be granted permission to by name. You can also list secrets with "docker secret list" and remove them with "docker secret rm". Add --grant-secret support to docker build This is useful if you want to use secrets during docker build which will not be recorded in the final image. API docs: add the new secret based requests docs: Add cli docs for the secret store cli: Allow multi-level command like docker foo list, and docker foo add If you implent CmdFoo with an extra boolean first arg, and CmdFooList and CmdFooAdd as usual the code will make this just work. vbatts: * adding and clarifying docs * fix the change in error handling * rebasing * removed global/host secrets Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson) Signed-off-by: Vincent Batts <vbatts@redhat.com>
36ca0b0
to
e9b3bb9
Compare
alrighty. also I squashed the commits down because it was such an overhaul to rebase and the sub command feature has since been implemented another way. There are still a few issues with the implementation that I'll like to address, but notably this PR now does not include the global/host secrets, only added secrets that have to be explicitly granted. |
@@ -775,6 +800,10 @@ func (container *Container) jsonPath() (string, error) { | |||
return container.getRootResourcePath("config.json") | |||
} | |||
|
|||
func (container *Container) secretsPath() (string, error) { | |||
return container.getRootResourcePath("secrets") |
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.
I sugest you place the secrets in container.getRootResourcePath("secrets/secret-files")
this way it will be easy to add a container.getRootResourcePath("secrets/secret-properties.json")
file in the future.
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.
Good thought. That would make it easier for a next step of handling ACLs of
who added the secret and what is the permissions on it.
On Oct 9, 2014 4:06 AM, "Timothy Hobbs" notifications@github.com wrote:
In daemon/container.go:
@@ -775,6 +800,10 @@ func (container *Container) jsonPath() (string, error) {
return container.getRootResourcePath("config.json")
}+func (container *Container) secretsPath() (string, error) {
- return container.getRootResourcePath("secrets")
I sugest you place the secrets in
container.getRootResourcePath("secrets/secret-files") this way it will be
easy to add a
container.getRootResourcePath("secrets/secret-properties.json") file in
the future.—
Reply to this email directly or view it on GitHub
https://github.com/docker/docker/pull/6697/files#r18629350.
After design discussion:
I will welcome specific docs-first proposals for solving each of these use cases, separately. Sorry that this is taking so long. |
shameless plug Since this issue is now closed, I figured I would share my stop-gap solution docket which I currently use to build images that require sensitive information. Docket is built on top of The project is still relatively new and experimental, but I welcome contributions and feedback from anyone that wishes to try it! |
@shykes Before I go and create a proposal for the "dev ssh key for builds" use case, I just want to make sure there aren't any currently open. This one is closed and #10310 doesn't address an individual use case like you asked for on Nov 4th. I couldn't find any other related proposals but just wanted to make sure. Can you confirm? |
Damn we're in 2017 and after reading tons of PR and articles the only thing I learned is that Docker has this problem for a long time even tough a lot of PR has been proposed. Weirdo. |
The trouble that happened was that during one of the regular IRC
meetings a design was proposed by three people and agreed upon by
@shykes (I don't even recal the exact design) a seccond design was
proposed by like two people and implemented by Alex Larson. And the two
designs were very different and @shykes explicitly said during the
meeting that he didn't like the one that Alex proposed. And so it lead
to this weird situation of having an implemented PR that was against the
wishes of the BDFL and no PR that was according to his wishes. Now I
think that shykes is quite busy right now, and so maybe his oppinion
matters less and this PR can go through. Or someone can go look up those
old IRC conversations and try to implement the "correct" design.
|
@NitroBAY @timthelion have a look at #27794 (which will be in docker 1.13), and #28079 (which probably will be continued on for docker 1.14) |
Okay thanks, I may be suprised because I'm used to such huge projects, it's maybe normal that implement a feature is that complex in that kind of projects. |
Closes #6075
Starting on a new PR to accommodate alex being on holiday