-
Notifications
You must be signed in to change notification settings - Fork 381
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
Add DockerCompatAuthFilePath to allow login/logout to interoperate #2173
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.
Code LGTM
The plumbing into podman needs very careful consideration. What (I think) needs to work "out of the box" on Mac is a podman login
... docker-compose ...
without having to specify any new flags on the CLI.
At a first glance, that seems impossible - in Docker mode we must reject And either way it’s not necessary for containers/podman#18617, that one starts with an explicit |
8dddf38
to
b8f8916
Compare
... so that the Set/Remove functions are together. Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
For now, this is not really helpful, but we will conditionalize the returned values. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... using a new types.SystemContext.DockerCompatAuthFilePath. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
b8f8916
to
7b94d26
Compare
Manual tests show that
And there are the unit tests in this PR. So I think this is ready for review/merging. |
> go get github.com/containers/image/v5@main > go mod tidy && go mod vendor Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to include containers/image#2173 and containers/common#1731 . Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to include containers/image#2173 and containers/common#1731 . Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to include containers/image#2173 and containers/common#1731 . Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to include containers/image#2173, containers/common#1731 and containers/buildah#5143 . Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Allow the existing
pkg/docker/config
credential API to be used with Docker-compatibleconfig.json
files, using a newtypes.SystemContext.DockerCompatAuthFilePath
option. This should help with containers/podman#18617 ; I plan to expose this option inc/common/pkg/auth
, and users would say something likepodman login --docker-file=~/.docker/config.json docker.io
(end-user design doc TBD).It must be an option primarily because we want to refuse to write namespaced credentials when writing to
.docker/config.json
.Also add interoperability testing between c/image and
docker/cli
, to the extent cheaply possible (e.g.docker/cli
, understandably, is not structured to trigger the “write credentials” code oflogin
without trying to actually login — so the tests use a somewhat lower-level API and need to duplicate some of the logic.I was considering introducing this option with a new functional-option API instead of continuing with problematic
SystemContext
, butpkg/docker/config
depends onsysregistriesv2
for the credential helpers list, so that would be a rather invasive change.WIP, filed for early review on the API. Cc: @vrothberg