-
Notifications
You must be signed in to change notification settings - Fork 2k
Use DOCKER_AUTH_CONFIG
env as credential store
#6008
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
Conversation
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6008 +/- ##
==========================================
+ Coverage 55.03% 55.56% +0.52%
==========================================
Files 361 356 -5
Lines 30153 29999 -154
==========================================
+ Hits 16596 16670 +74
+ Misses 12599 12366 -233
- Partials 958 963 +5 🚀 New features to boost your workflow:
|
866c70a
to
eb43744
Compare
e7c6f4d
to
b3aa07f
Compare
de90f77
to
b7770d7
Compare
d801989
to
0417f99
Compare
5e95a08
to
63c990c
Compare
|
4978971
to
2d0b124
Compare
2d0b124
to
5f2a40a
Compare
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 noticed I had some pending comments
5f2a40a
to
bb1bf8c
Compare
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.
LGTM
@vvoland PTAL |
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.
Overall looks good, but left some comments
This patch enables the CLI to natively pick up the `DOCKER_AUTH_CONFIG` environment variable and use it as a credential store. The `DOCKER_AUTH_CONFIG` value should be a JSON object and must store the credentials in a base64 encoded string under the `auth` key. Specifying additional fields will cause the parser to fail. For example: `printf "username:pat" | openssl base64 -A` `export DOCKER_AUTH_CONFIG='{ "auths": { "https://index.docker.io/v1/": { "auth": "aGk6KTpkY2tyX3BhdF9oZWxsbw==" } } }'` Credentials stored in `DOCKER_AUTH_CONFIG` would take precedence over any credential stored in the file store (`~/.docker/config.json`) or native store (credential helper). Destructive actions, such as deleting a credential would result in a noop if found in the environment credential. Credentials found in the file or native store would get removed. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
bb1bf8c
to
9b83d5b
Compare
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.
LGTM, thanks!
This patch enables the CLI to natively pick up the
DOCKER_AUTH_CONFIG
environment variable and use it as a credential store.
The
DOCKER_AUTH_CONFIG
value should be a JSON object and must storethe credentials in a base64 encoded string under the
auth
key.Credentials stored in
DOCKER_AUTH_CONFIG
would take precedence over anycredential stored in the file store (
~/.docker/config.json
) or native store(credential helper).
Destructive actions, such as deleting a credential would result in a noop if
found in the environment credential. Credentials found in the file or
native store would get removed.
- What I did
- How I did it
- How to verify it
printf "username:pat" | openssl base64 -A
Setup the
DOCKER_AUTH_CONFIG
environment variable- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)