Add support for Docker credential helpers#460
Conversation
This adds the ability to retrieve registry credentials from a credential helper specified in `credHelpers` or `credsStore` inside ~/.docker/config.json.
|
@microsoft-github-policy-service agree company="Netflix" |
samruddhikhandale
left a comment
There was a problem hiding this comment.
Nice, thanks for putting up this PR! Left some minor comments
|
Thanks for the review! I made the suggested changes. |
chrmarti
left a comment
There was a problem hiding this comment.
Thanks, left a few comments.
|
Thanks! I've made these changes. |
chrmarti
left a comment
There was a problem hiding this comment.
Great, thanks! Left another question and ran the linter.
|
Thanks, addressed this latest round of comments. |
There was a problem hiding this comment.
This is awesome, thanks for taking the time (triggered the existing unit tests to run on this PR).
Do you have any suggestions on how to craft unit tests for this? Most of our unit tests run in linux, and we do have a subset of them running in windows. Theoretically we could run against macOS too (if we wanted to...)
Currently familiarizing myself with how this works
Great question. The credential helper "protocol" is very simple and it would be easy to mock a credential helper. But to ensure the returned creds are actually used, we'd need an authenticated registry to connect to, and I'm not sure what kind of test infra exists for that. |
There was a problem hiding this comment.
+1 to some of the earlier feedback (which you've addressed). I don't want to block on the tests - I can make it an action item for myself if you don't have the time. There's one minor linting error in CI.
Validated this works well on MacOS with AzureCR creds (this branch is ./devcontainer.js and the current release is on my PATH)
chrmarti
left a comment
There was a problem hiding this comment.
Thanks for the update. Left another comment on the use of a sync call. Otherwise 👍
chrmarti
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
|
Thanks so much for the help geting this through! It's been a great experience. |
This adds the ability to retrieve registry credentials from a credential helper specified in
credHelpersorcredsStoreinside ~/.docker/config.json.