Use credentials from authfile for skopeo commands #453

umohnani8 · 2017-11-16T21:31:58Z

skopeo copy, delete, and inspect can now use credentials stored in the auth file
by the kpod login command
e.g kpod login docker.io -> skopeo copy dir:mydir docker://username/image

Signed-off-by: Urvashi Mohnani umohnani@redhat.com

umohnani8 · 2017-11-16T21:32:14Z

@rhatdan @TomSweeneyRedHat @mtrmac PTAL

umohnani8 · 2017-11-16T21:33:09Z

@mtrmac would you prefer the authfile flag be flagPrefix+"authfile" as well?

rhatdan · 2017-11-17T13:28:08Z

docs/skopeo.1.md


  **docker://**_docker-reference_
-  An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in `$HOME/.docker/config.json`, which is set e.g. using `(docker login)`.
+  An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in `$XDG_RUNTIME_DIR/containers/auth.json`, which is set e.g. using `(kpod login)`.


I think we should leave in that this works with either kpod login or docker login. Since skopeo is used by the larger docker community.

rhatdan · 2017-11-17T13:28:25Z

docs/skopeo.1.md

 ```

+# SEE ALSO
+kpod-login(1)


Also add docker-login(1)

Should we not this in the Buildah/kpod commands too? I'm torn, we do use docker-login for those too, just don't want people to think we didn't think about that if we don't document it. Then again I'd rather not add a docker dependency to them. i.e. if docker changes auth at some point, then we'd have to adjust those for that rather than just dropping it.

fixed.
@rhatdan should this be mentioned in buildah and kpod as well?

We can mention in buildah and kpod that docker login works, but don't need to specify it directly. Since those tools are not as tied into the Docker infrastructure as skopeo is.

TomSweeneyRedHat · 2017-11-17T15:11:15Z

docs/skopeo.1.md

+
+  Path of the authentication file. Default is ${XDG_RUNTIME_DIR}/containers/auth.json
+  which is set using `kpod login`
+


Just double checking as I've not tried. If you use the $HOME/.docker/config.json file as the auth file, is docker happy afterwards? I.e. if you do 'kpod login --auth ~/.docker/config.json', can you then run a docker command that requires auth without it spitting up? Really is not related to this review though.

I actually haven't tried that yet. It might not work because there is a slight difference in the way kpod and docker store the registry name in auth.json and config.json.

mtrmac

ACK overall, just one nit.

mtrmac · 2017-11-21T00:24:00Z

docs/skopeo.1.md

+  **--authfile** _path_
+
+  Path of the authentication file. Default is ${XDG_RUNTIME_DIR}/containers/auth.json
+  which is set using `kpod login` or `docker login`


This seems to say docker login writes to auth.json, which is not true. (Also in two other instances.)

If we have this in kpod and/or Buildah, can we touch it up there too please?

fixed.
@TomSweeneyRedHat there is no mention of docker login in buildah or kpod. Is it something we want to add?

mtrmac · 2017-11-21T00:25:00Z

@mtrmac would you prefer the authfile flag be flagPrefix+"authfile" as well?

No, a single flag as you have implemented is better. The authFile can already carry configurations for multiple registries, so it doesn’t seem useful to support having a different one for sources and destinations.

umohnani8 · 2017-11-21T14:53:19Z

@mtrmac @runcom PTAL

mtrmac · 2017-11-21T14:54:41Z

LGTM

runcom · 2017-11-21T14:56:10Z

LGTM

rhatdan · 2017-11-21T14:56:51Z

@umohnani8 Lets add something to kpod and buildah that mentions docker login.

TomSweeneyRedHat · 2017-11-21T15:00:54Z

docs/skopeo.1.md


  **docker://**_docker-reference_
-  An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in `$HOME/.docker/config.json`, which is set e.g. using `(docker login)`.
+  An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in either `$XDG_RUNTIME_DIR/containers/auth.json`, which is set using `(kpod login)` or in `$HOME/.docker/config.json`, which is set using `(docker login)`


👍
@umohnani8 I do think this should probably be added to kpod/buildah somewhere. We're currently searching thorough containers/auth.json and then .docker/config.json in both of those now right? If not, then this should be added when we are.

I think we probably ought to note precedence here too. Perhaps: "By default, uses the authorization state in $XDG_RUNTIME_DIR/containers/auth.json, which is set using (kpod login). If the authorization state is not found there, then $HOME/.docker/config.jsonis checked which is set using(docker login)`."

FWIW, missing a period at the end of your current line here.

TomSweeneyRedHat · 2017-11-21T15:08:21Z

docs/skopeo.1.md


+  **--authfile** _path_
+
+  Path of the authentication file. Default is ${XDG_RUNTIME\_DIR}/containers/auth.json, which is set using `kpod login`


Missing period after kpod login. A mention of precedence would probably be good here too. Perhaps: "Default is ${XDG_RUNTIME_DIR}/containers/auth.json, which is set using kpod login. If the authorization state is not found there, then $HOME/.docker/config.json is checked, which is set using docker login."

skopeo copy, delete, and inspect can now use credentials stored in the auth file by the kpod login command e.g kpod login docker.io -> skopeo copy dir:mydir docker://username/image Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>

umohnani8 · 2017-11-21T16:21:06Z

Tests passed

rhatdan reviewed Nov 17, 2017

View reviewed changes

TomSweeneyRedHat reviewed Nov 17, 2017

View reviewed changes

umohnani8 force-pushed the creds branch from 8511858 to ed477bd Compare November 17, 2017 15:43

mtrmac reviewed Nov 21, 2017

View reviewed changes

umohnani8 force-pushed the creds branch from ed477bd to 870fbb2 Compare November 21, 2017 14:52

TomSweeneyRedHat reviewed Nov 21, 2017

View reviewed changes

umohnani8 force-pushed the creds branch from 870fbb2 to 6da32b6 Compare November 21, 2017 15:29

Use credentials from authfile for skopeo commands

cbedcd9

skopeo copy, delete, and inspect can now use credentials stored in the auth file by the kpod login command e.g kpod login docker.io -> skopeo copy dir:mydir docker://username/image Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>

umohnani8 force-pushed the creds branch from 6da32b6 to cbedcd9 Compare November 21, 2017 15:34

mtrmac merged commit 04e04ed into containers:master Nov 21, 2017

mtrmac mentioned this pull request Jul 12, 2018

RFC: Reliable CLI parsing #523

Merged

github-actions bot added the locked - please file new issue/PR label Oct 5, 2023

github-actions bot locked as resolved and limited conversation to collaborators Oct 5, 2023


		Path of the authentication file. Default is ${XDG_RUNTIME_DIR}/containers/auth.json
		which is set using `kpod login`


		--authfile _path_

		Path of the authentication file. Default is ${XDG_RUNTIME\_DIR}/containers/auth.json, which is set using `kpod login`

Use credentials from authfile for skopeo commands #453

Use credentials from authfile for skopeo commands #453

Uh oh!

Conversation

umohnani8 commented Nov 16, 2017

Uh oh!

umohnani8 commented Nov 16, 2017

Uh oh!

umohnani8 commented Nov 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac commented Nov 21, 2017

Uh oh!

umohnani8 commented Nov 21, 2017

Uh oh!

mtrmac commented Nov 21, 2017

Uh oh!

runcom commented Nov 21, 2017

Uh oh!

rhatdan commented Nov 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomSweeneyRedHat Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

umohnani8 commented Nov 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

TomSweeneyRedHat Nov 21, 2017 •

edited

Loading