Closed optiz0r closed 5 years ago
I've had another go at this and can't seem to re-create the issue I was seeing now, which is confusing. While the docker docs suggest it might provide a full URL including scheme and path, 19.03.2 at least does not seem to do this and only provides the hostname:port which works.
Re-created under nomad. When running docker pull host:port/image:tag
, the credential helper is called with just the host:port
component. When nomad tries to download an image, the URL appears to be in the form https://host:port/image
(although no tag).
I was able to reproduce this when using the credential helper with nomad as well. Similarly, it works fine on docker pull
, but fails on nomad. As @optiz0r points out there is a discrepancy in how each passes the registry urls.
Thanks @optiz0r and @sarkis for bringing this up and confirming it. I have not been able to reproduce this with our nomad clusters yet, but will discuss with @dbellinghoven tomorrow and review #25.
Thanks all. For ref I'm running a slightly older version of nomad, 0.8.x, there could be a change in behaviour in 0.9 or master which I haven't checked. Although I imagine nomad is just asking docker daemon to pull an image, so I doubt it has any direct control over the URL being used. Perhaps a difference between the docker API and docker CLI?
Here is the versions I reproduced on:
Client: Docker Engine - Community
Version: 19.03.4
API version: 1.40
Go version: go1.12.10
Git commit: 9013bf583a
Built: Fri Oct 18 15:54:09 2019
OS/Arch: linux/amd64
Experimental: false
Server: Docker Engine - Community
Engine:
Version: 19.03.4
API version: 1.40 (minimum version 1.12)
Go version: go1.12.10
Git commit: 9013bf583a
Built: Fri Oct 18 15:52:40 2019
OS/Arch: linux/amd64
Experimental: false
containerd:
Version: 1.2.10
GitCommit: b34a5c8af56e510852c35414db4c1f4fa6172339
runc:
Version: 1.0.0-rc8+dev
GitCommit: 3e425f80a8c931f88e6d94a8c831b9d5aa481657
docker-init:
Version: 0.18.0
GitCommit: fec3683
Nomad v0.10.0+ent (a17224f9d5d57148e2fdf2a163a9a354aef7ee9d)
@optiz0r I think you are onto something, digging into nomad to see what docker client is being used and found this in master:
{"path":"github.com/fsouza/go-dockerclient","checksumSHA1":"fvbo1J+cFWDrPDs0Yudwv+POIb4=","revision":"01c3e9bd8551d675a60382c0303ef51f5849ea99","revisionTime":"2018-11-29T02:57:25Z"},
So, almost a year old go-dockerclient
even on master... I still think it makes sense to just strip out what we are after on the docker helper here. It will make it so it's compatible with older versions of nomad if and when this gets fixed there 🤔
I confirmed at least for my reproduced use case, fixed by #25. Mind confirming if you are set @optiz0r as of v0.2.18?
LGTM, thanks very much :)
Overview of the Issue
The README suggests only the host/port will be used to lookup the secret path for a registry, however in https://github.com/morningconsult/docker-credential-vault-login/pull/20/files#diff-65a64ca27fd0731d6dc44b45e687d76aR101, the full URL as given by the docker daemon to the helper is used to lookup the path to the secret, including the image name/tag. The documented way makes sense, as in general you have one set of credentials for the registry as a whole. The way it's implemented is unsable without filling the configuration file with every possible image name/tag.
Reproduction Steps
Expected
Actual
With the following in the dcvl log file:
Proposed fix
getPath
(https://github.com/morningconsult/docker-credential-vault-login/pull/20/files#diff-3baf47c64847a8fb8aaa8cc2e088513bR54) should pre-process the URL and strip out the image path/name/tag before looking up the secret.While the parameter is called the serverURL, it seems to be missing the scheme, I'm not sure why or if the scheme would ever be present. It might be prudent to also strip out the scheme if it might ever be present in future?