Skip to content
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

docker credential helpers don't work with containerd image store #4653

Closed
StealthyCoder opened this issue Nov 10, 2023 · 9 comments · Fixed by #4655
Closed

docker credential helpers don't work with containerd image store #4653

StealthyCoder opened this issue Nov 10, 2023 · 9 comments · Fixed by #4655
Labels

Comments

@StealthyCoder
Copy link
Contributor

Description

I was experimenting with the WASM runtime setup documented here. I enabled the containerd image store feature and I got my simple docker image working I pushed to Docker Hub. I have to say, that felt really cool and awesome.

Then all of a sudden I could not get any of my images I had for the product I work for. After digging quite a bit, I found the issue. A one liner in the NativeStore Get method for the docker credential helpers does not set the ServerAddress property on the AuthConfig object. Which gets propagated all the way down to a line of code in dockerd of the moby/moby project.

Reproduce

  1. Enable containerd image store as documented here.
  2. Restart docker daemon
  3. Setup credential helper as documented here
  4. docker pull image that will use that credential helper

Expected behavior

The authorization should just keep on working like normal and as is the case with credential stores.

docker version

Client: Docker Engine - Community
 Version:           24.0.7
 API version:       1.43
 Go version:        go1.20.10
 Git commit:        afdd53b
 Built:             Thu Oct 26 09:07:41 2023
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          24.0.7
  API version:      1.43 (minimum version 1.12)
  Go version:       go1.20.10
  Git commit:       311b9ff
  Built:            Thu Oct 26 09:07:41 2023
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.24
  GitCommit:        61f9fd88f79f081d64d6fa3bb1a0dc71ec870523
 runc:
  Version:          1.1.9
  GitCommit:        v1.1.9-0-gccaecfc
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

docker info

Client: Docker Engine - Community
 Version:    24.0.7
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.11.2
    Path:     /usr/libexec/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.21.0
    Path:     /usr/libexec/docker/cli-plugins/docker-compose

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 8
 Server Version: 24.0.7
 Storage Driver: overlayfs
  driver-type: io.containerd.snapshotter.v1
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 61f9fd88f79f081d64d6fa3bb1a0dc71ec870523
 runc version: v1.1.9-0-gccaecfc
 init version: de40ad0
 Security Options:
  apparmor
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 6.5.6-76060506-generic
 Operating System: Pop!_OS 22.04 LTS
 OSType: linux
 Architecture: x86_64
 CPUs: 16
 Total Memory: 31.07GiB
 Name: pop-os
 ID: XZSV:MMHH:Z4R5:AILG:YYNZ:7YBN:YITO:UCQH:KFKN:EPAA:VWX2:F4HF
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Username: greagnath
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

Additional Info

In the logs of dockerd you will find the following line:

WARN[2023-11-10T16:29:20.476763317+01:00] Host doesn't match                            cfgHost=registry-1.docker.io host=hub.foundries.io

That is what narrowed it down for me. I already have a PR planned for this issue. I will link them together.

@StealthyCoder
Copy link
Contributor Author

Just created #4655 that should solve this issue.

@thaJeztah
Copy link
Member

Looks related to;

@thaJeztah thaJeztah added containerd-integration Issues and PRs related to containerd integration area/authentication labels Nov 10, 2023
@StealthyCoder
Copy link
Contributor Author

I forgot to check on the moby/moby side of things. Thanks for finding those. This is not to spam or self promote, but I wrote a little post on the debugging adventure. To give more context and hopefully a better overview for anyone who wishes to gain more information.

@thaJeztah
Copy link
Member

thaJeztah commented Nov 11, 2023

This is not to spam or self promote, but I wrote a little post on the debugging adventure. To give more context and hopefully a better overview for anyone who wishes to gain more information.

Wow, that is ... awesome, that's a great read! If you are on the Docker Community Slack, also always feel free to give a "ping" in the #moby-project channel; most of the maintainers should be in that channel as well if you have questions (but admitted; they may not always be paying close attention, so never hesitate to @-mention someone if you don't get a reply 🤗).

Also full disclosure; the whole authentication flow is currently far too complicated. Many parts of this originated from the very early beginnings of Docker, at which time code was often poorly documented, if at all ; the code-base was a lot smaller, there was only "one" registry (docker hub), and intent would be clear from the context/code itself. But this code went through many iterations after that (Authentication on Docker Hub changed, other registries started to appear, code moved to different repositories, and things like "credential helpers" were added).

With the work on the containerd image store integration, we discovered many areas where context was lost, intended behavior/purpose not documented or ambiguous, and a decent amount of "archeology" was needed to try to track back these things. My team also has an internal backlog for things we discovered in the process, and which may need follow-up work. The intent is for most of those to be opened as ticket in upstream (public) issue trackers (but some of them need some cleaning up, as they may be just a quick "blurb" / "reminder"). Also see my comment on moby/moby#46779 (comment) (which is yet to be split into some more actionable items).

In either case; if you found things on your Journey, such as Go structs or Fields that are not documented; contributions are always welcome and appreciated. Also don't hesitate to open pull requests for cases where you're not sure if your change is "correct" (or your change is in an "early state"); it's OK to iterate on changes; that's what code-reviews and conversations on GitHub are for (worst case; a pull request is closed without merging, and no harm done).

@StealthyCoder
Copy link
Contributor Author

@thaJeztah I am not on the Docker Community Slack. I would be interested in helping out with the work of what comes out of that internal backlog. If you are able and provide an invite, I would like to join the Slack community.

@thaJeztah
Copy link
Member

I think this link should allow you to join (but I know the invite-URLs expire after some days, so if that link doesn't work, let me know then I can ask them to re-generate); https://dockr.ly/slack

@StealthyCoder
Copy link
Contributor Author

It does not match one of the domains listed. I do not have a @docker.com address. At least that is the error I get.

@thaJeztah
Copy link
Member

@StealthyCoder can you try this URL? This one seems to be still active; https://dockercommunity.slack.com/join/shared_invite/zt-26vrhfx2r-GeFdhfxh1xlmW4yCFxou8w#/shared-invite/email

@thaJeztah
Copy link
Member

closing this one ashttps://github.com//pull/4655 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants