moby: Don't send all credstore entries to daemon on docker build

In Docker For Mac/Win in 17.03.0-ce we make default the usage of docker-credential-helper.

A user reported that we scan each and every entry in the all user keychains, as per https://github.com/docker/for-mac/issues/1371.

This behaviour is observable, only on docker build and not docker pull.

This leads us to believe that https://github.com/docker/docker/blob/master/cli/command/image/build.go#L280 is probably partly faulty. Along with https://github.com/docker/docker-credential-helpers/blob/master/osxkeychain/osxkeychain_darwin.c#L118 which seems to open all keychains. All entries in the keychains are sent to the daemon. Which in turn triggered the behaviour described by the user.

They should probably be filtered by the one used by docker login.

Output of docker version:

> docker version
Client:
 Version:      17.03.0-ce
 API version:  1.26
 Go version:   go1.7.5
 Git commit:   60ccb22
 Built:        Thu Feb 23 10:40:59 2017
 OS/Arch:      darwin/amd64

Server:
 Version:      17.03.0-ce
 API version:  1.26 (minimum version 1.12)
 Go version:   go1.7.5
 Git commit:   3a232c8
 Built:        Tue Feb 28 07:52:04 2017
 OS/Arch:      linux/amd64
 Experimental: true

Output of docker info:

> docker info
Containers: 0
 Running: 0
 Paused: 0
 Stopped: 0
Images: 1
Server Version: 17.03.0-ce
Storage Driver: overlay2
 Backing Filesystem: extfs
 Supports d_type: true
 Native Overlay Diff: true
Logging Driver: json-file
Cgroup Driver: cgroupfs
Plugins: 
 Volume: local
 Network: bridge host ipvlan macvlan null overlay
Swarm: inactive
Runtimes: runc
Default Runtime: runc
Init Binary: docker-init
containerd version: 977c511eda0925a723debdc94d09459af49d082a
runc version: a01dafd48bc1c7cc12bdb01206f9fea7dd6feb70
init version: 949e6fa
Security Options:
 seccomp
  Profile: default
Kernel Version: 4.9.12-moby
Operating System: Alpine Linux v3.5
OSType: linux
Architecture: x86_64
CPUs: 2
Total Memory: 5.572 GiB
Name: moby
ID: T4H4:FHZA:GOQW:7GND:BENW:HYGI:JHRC:H3XI:OQQP:YW22:6YNH:O2DX
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): true
 File Descriptors: 16
 Goroutines: 26
 System Time: 2017-03-03T13:24:29.210944844Z
 EventsListeners: 1
No Proxy: *.local, 169.254/16
Registry: https://index.docker.io/v1/
Experimental: true
Insecure Registries:
 127.0.0.0/8
Live Restore Enabled: false

About this issue

  • Original URL
  • State: open
  • Created 7 years ago
  • Comments: 19 (7 by maintainers)

Most upvoted comments

We’re hitting this problem, too, using the ECR creds helper. I’ve done a bunch of digging around, and this is not a bug in that, except insofar as the cred helper protocol is insufficient for the task.

What I think docker build should do is to use the list of registries in credHelpers to get the list of credentials to send, rather than asking each cred helper what they think is a list of valid registries. But, given the never-ending PRs that may, one day, result in us not having to pre-populate every registry ever, I doubt such a PR would be accepted (the more intelligent one, to parse the Dockerfile to find registries to send creds for, got closed in favour of another PR to do cred negotiation – which also got closed), so I’m not going to take the time to put a PR together unless there’s a clear indication from Docker core that it had a hope of being merged.

We’re going to start doing an explicit pull before every build, which neatly avoids this problem, and has the added benefit of making sure we’re always building against the latest version of a tag, rather than a potentially out-of-date version of the same tag (non-immutable tags ftl).

I can write the patch by the way.

Removed the P0 as this is fixed for desktop version — the docker-credential-x binaries now use labels to just get/ask for the docker credentials and not all of them 👼

After discussions, we found we couldn’t fix this issue elegantly / easily without introducing changes in the communication between credential helpers and docker client.

One interesting way to solve that permanently (and much more elegantly), is to leverage @tonistiigi work on streamed context (https://github.com/docker/docker/issues/31829) once it is done, to expose grpc methods allowing to request credentials on demand from the builder to the client in addition to the filesystem context.