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 plaintext credentials as multi-call binary #5536

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

laurazard
Copy link
Member

@laurazard laurazard commented Oct 15, 2024

- What I did

The problem

The Docker CLI supports storing/managing credentials without a credential-helper, in which case credentials are fetched from/saved to the CLI config file (~/.docker/config.json). This is all managed entirely by the CLI itself, without resort to a separate binary.

There are a few issues with this approach – for one, saving the credentials together with all the configurations make it impossible to share one without the other, so one can't for example bind mount the config file into a container without also including all configured credentials.

Another issue is that this has made it so that any other clients accessing registry credentials (such as
https://github.com/google/go-containerregistry) all have to both:

  • read/parse the CLI config.json, to check for credentials there, which also means they're dependent on this type and might break if the type changes/we need to be careful not to break other codebases parsing this file, and can't change the location where plaintext credentials are stored.
  • support the credential helper protocol, so that they can access credentials when users do have configured credential helpers.

This means that if we want to do something like support oauth credentials by having credential-helpers refresh oauth tokens before returning them, we have to both implement that in each credential-helper and in the CLI itself, and any client directly reading config.json will also need to implement this logic.

The proposed solution

This commit turns the Docker CLI binary into a multicall binary, acting as a standalone credentials helper when invoked as docker-credential-file, while still storing/fetching credentials from the configuration file (~/.docker/config.json), and without any further changes.

This represents a first step into aligning the "no credhelper"/plaintext flow with the "credhelper" flow, meaning that instead of this being an exception where credentials must be read directly from the config file, credentials can now be accessed in the exact same way as with other credential helpers – by invoking docker-credential-[credhelper name], such as docker-credential-pass, docker-credential-osxkeychain or docker-credential-wincred.

This would also make it possible for any other clients accessing credentials to untangle themselves from things like the location of the credentials, parsing credentials from config.json, etc. and instead simply support the credential-helper protocol, and call the docker-credential-file binary as they do others.

Asides from addressing the aforementioned issues, the multi-call binary solution allows us to to do this while making minimal changes to packaging/delivery pipelines, as the CLI binary already has all the packaging setup required and we don't need to package or ship a separate binary. The only change required to use this is for existing packages to create a new symlink during installation, and this flow can be used. This also reduces overhead with managing a new repo, dependencies, etc.

- How I did it

Make the CLI binary multi-call, and act as a file credentials-helper when invoked as such.

- How to verify it

  • ln -s [docker CLI binary] [somewhere in your path/docker-credential-file]
  • docker-credential-file list

- Description for the changelog

todo

- A picture of a cute animal (not mandatory but encouraged)

Screenshot 2024-10-15 at 12 12 18

@codecov-commenter
Copy link

codecov-commenter commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 6.45161% with 29 lines in your changes missing coverage. Please review.

Project coverage is 60.85%. Comparing base (31eeed7) to head (d6ce046).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5536      +/-   ##
==========================================
+ Coverage   60.53%   60.85%   +0.32%     
==========================================
  Files         342      346       +4     
  Lines       23483    23522      +39     
==========================================
+ Hits        14216    14315      +99     
+ Misses       8292     8234      -58     
+ Partials      975      973       -2     

The Docker CLI supports storing/managing credentials without a
credential-helper, in which case credentials are fetched from/saved to
the CLI config file (`~/.docker/config.json`). This is all managed
entirely by the CLI itself, without resort to a separate binary.

There are a few issues with this approach – for one, saving the
credentials together with all the configurations make it impossible to
share one without the other, so one can't for example bind mount the
config file into a container without also including all configured
credentials.

Another issue is that this has made it so that any other clients
accessing registry credentials (such as
https://github.com/google/go-containerregistry) all have to both:
- read/parse the CLI `config.json`, to check for credentials there,
  which also means they're dependent on this type and might break if the
  type changes/we need to be careful not to break other codebases parsing
  this file, and can't change the location where plaintext credentials
  are stored.
- support the credential helper protocol, so that they can access
  credentials when users do have configured credential helpers.

This means that if we want to do something like support oauth
credentials by having credential-helpers refresh oauth tokens before
returning them, we have to both implement that in each credential-helper
and in the CLI itself, and any client directly reading `config.json`
will also need to implement this logic.

This commit turns the Docker CLI binary into a multicall binary, acting
as a standalone credentials helper when invoked as
`docker-credential-file`, while still storing/fetching credentials from
the configuration file (`~/.docker/config.json`), and without any
further changes.

This represents a first step into aligning the "no credhelper"/plaintext
flow with the "credhelper" flow, meaning that instead of this being an
exception where credentials must be read directly from the config file,
credentials can now be accessed in the exact same way as with other
credential helpers – by invoking `docker-credential-[credhelper name]`,
such as `docker-credential-pass`, `docker-credential-osxkeychain` or
`docker-credential-wincred`.

This would also make it possible for any other clients accessing
credentials to untangle themselves from things like the location of the
credentials, parsing credentials from `config.json`, etc. and instead
simply support the credential-helper protocol, and call the
`docker-credential-file` binary as they do others.

Signed-off-by: Laura Brehm <laurabrehm@hey.com>
@laurazard laurazard force-pushed the multicall-file-helper branch from b1d9e8a to d6ce046 Compare October 15, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants