Skip to content

SaveImageCmd API doesn't support X-Registry-Auth header#175

Merged
marcuslinke merged 3 commits intodocker-java:masterfrom
xfournet:pr1
Mar 29, 2015
Merged

SaveImageCmd API doesn't support X-Registry-Auth header#175
marcuslinke merged 3 commits intodocker-java:masterfrom
xfournet:pr1

Conversation

@xfournet
Copy link
Copy Markdown
Contributor

@xfournet xfournet commented Mar 19, 2015

As for #165 a NPE will be throw if the auth config is not complete.
However according the API documentation the X-Registry-Auth header is not part of the save image API, so I removed it

@marcuslinke
Copy link
Copy Markdown
Contributor

marcuslinke commented Mar 21, 2015

@xfournet Although it is not documented in the API i guess it would make sense to allow OPTIONAL authentication when working against a private repository. So I'm not sure if this an error in the API documentation...

@xfournet
Copy link
Copy Markdown
Contributor Author

xfournet commented Mar 28, 2015

@marcuslinke
The save image API doesn't access to the repository:

$ docker images -a
REPOSITORY          TAG                 IMAGE ID            CREATED             VIRTUAL SIZE
$ docker save -o image.tar progrium/busybox
FATA[0000] Error: could not find image: no such id: progrium/busybox
$ docker pull progrium/busybox
Pulling repository progrium/busybox
8cee90767cfe: Download complete
511136ea3c5a: Download complete
46e263e5de56: Download complete
cf677a5f718c: Download complete
6adb1b0566e7: Download complete
fdb314abe596: Download complete
cfed5ebfc893: Download complete
653376e72714: Download complete
46bed15d45c7: Download complete
Status: Downloaded newer image for progrium/busybox:latest
$ docker images
REPOSITORY          TAG                 IMAGE ID            CREATED             VIRTUAL SIZE
progrium/busybox    latest              8cee90767cfe        9 weeks ago         4.789 MB
$ docker save -o image.tar progrium/busybox
$ ls image.tar
-rw-rw-r-- 1 me me 4.9M Mar 28 22:33 image.tar

so it doesn't make sense to have authentication here. The offical API documentation seems good ;) My guess is that the code of the SaveImageCmd has been done through copy/paste of PushImageCmd : in 8af3d6f the SaveImageCmdExec and SaveImageCmdImplTest loggers reference PushImageCmdImplTest and PushImageCmdExec... oups copy/paste fail detected
Then, no evaluation of the need of the authentication has been done...

@marcuslinke
Copy link
Copy Markdown
Contributor

marcuslinke commented Mar 29, 2015

@xfournet Thanks for detailed info! Will be merged soon.

marcuslinke added a commit that referenced this pull request Mar 29, 2015
@marcuslinke
Merge pull request #175 from xfournet/pr1
63ea2fa 
SaveImageCmd API doesn't support X-Registry-Auth header
@marcuslinke marcuslinke merged commit 63ea2fa into docker-java:master Mar 29, 2015
@xfournet xfournet deleted the pr1 branch May 13, 2015 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xfournet @marcuslinke