Skip to content

Migrate container client to moby/moby modules#5420

Merged
rdimitrov merged 1 commit into
mainfrom
rdimitrov/migrate-docker-docker-to-moby
Jun 3, 2026
Merged

Migrate container client to moby/moby modules#5420
rdimitrov merged 1 commit into
mainfrom
rdimitrov/migrate-docker-docker-to-moby

Conversation

@rdimitrov
Copy link
Copy Markdown
Member

Summary

Migrates pkg/container/docker and pkg/container/images off the deprecated github.com/docker/docker module onto the maintained split modules github.com/moby/moby/client and github.com/moby/moby/api (both already required by ToolHive).

github.com/docker/docker is deprecated as of Docker v29 (Nov 2025) and frozen — it gets no further releases, including fixes for the daemon-side advisories CVE-2026-33997 (GO-2026-4883) and CVE-2026-34040 (GO-2026-4887) that we currently suppress in govulncheck (#4521). Rather than carry a suppression for a dead module indefinitely, this removes the dependency from the production build. The moby README migration map:

github.com/docker/docker/client       → github.com/moby/moby/client
github.com/docker/docker/api/types/X   → github.com/moby/moby/api/types/X
github.com/docker/docker/pkg/stdcopy   → github.com/moby/moby/api/pkg/stdcopy
github.com/docker/go-connections/nat   → github.com/moby/moby/api/types/network

After this change the production build no longer imports github.com/docker/docker (go mod why → "main module does not need package"); it remains only as an indirect test dependency via testcontainers-go.

What changed

The new moby/moby/client is a redesigned API, not a drop-in rename, so call sites were adapted while preserving behavior:

  • Per-operation Options/Result structs — e.g. ContainerList(ctx, ContainerListOptions{…}) → res.Items, ContainerCreate(ctx, ContainerCreateOptions{Config, HostConfig, NetworkingConfig, Name}), Ping(ctx, PingOptions{}). dockerAPI interface and the fakeDockerAPI test double updated to match.
  • Filtersfilters.NewArgs()/Arg() → the chainable client.Filters map.
  • Portsnat.Port (string) → network.Port (struct): nat.NewPort("tcp", n)network.ParsePort(n), .Int()/.Proto().Num()/.Proto().
  • Host addressesPortBinding.HostIP, IPAMConfig.Gateway, EndpointSettings.IPAddress, HostConfig.DNS moved from string/[]string to netip.Addr/[]netip.Addr; a small parseHostIP helper maps empty↔zero to preserve "all interfaces" semantics, and reads are IsValid()-guarded so the zero value still renders as "".
  • Logs/AttachContainerLogsResult is an io.ReadCloser and ContainerAttachResult embeds HijackedResponse, so existing stream handling is unchanged (only the stdcopy import path moved).
  • SDK factoryclient.NewClientWithOpts(...)mobyclient.New(...); dropped WithAPIVersionNegotiation() (now the default).
  • registry.go — since *mobyclient.Client directly satisfies go-containerregistry's daemon.Client, the previously separate daemon client is dropped and a single client serves both ImageBuild and daemon.Image/daemon.Write.

~670 lines, mostly mechanical option/result adaptation + test fixtures.

Test plan

  • go build ./...
  • go vet ./...
  • go test ./pkg/container/... — all pass
  • golangci-lint run ./pkg/container/... — 0 issues
  • gofmt clean; GOOS=windows go build ./pkg/container/docker/sdk/ compiles
  • No github.com/docker/docker / go-connections/nat source imports remain in pkg/
  • Needs review + manual Docker integration testing of the full container lifecycle (deploy/list/stop/remove, network isolation, ingress/egress squid, DNS). Unit tests pass against the fake, but the live-daemon paths (port bindings, host IPs, attach/logs streaming) were not exercised against a real daemon in this change.

Reviewer notes (behavior-equivalence)

  • HostConfig.DNS now returns an error on an unparseable address (was silent passthrough). In practice additionalDNS only ever comes from an inspected container IP, so this path shouldn't trigger.
  • Please sanity-check parseHostIP, getDockerBridgeGatewayIP, the DNS-IP extraction in createDnsContainer, and the network.Port rework in setupExposedPorts/setupPortBindings/comparePortConfig.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the size/L Large PR: 600-999 lines changed label Jun 3, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

❌ Patch coverage is 66.32653% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.85%. Comparing base (8fce30e) to head (233f4a9).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/container/docker/client.go 65.47% 27 Missing and 2 partials ⚠️
pkg/container/images/registry.go 33.33% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5420      +/-   ##
==========================================
- Coverage   68.85%   68.85%   -0.01%     
==========================================
  Files         634      634              
  Lines       64422    64433      +11     
==========================================
+ Hits        44358    44363       +5     
- Misses      16783    16788       +5     
- Partials     3281     3282       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rdimitrov rdimitrov force-pushed the rdimitrov/migrate-docker-docker-to-moby branch from 190c8a9 to 9b2461b Compare June 3, 2026 09:03
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jun 3, 2026
github.com/docker/docker is deprecated as of Docker v29 (November
2025) and is frozen: it receives no further releases, including fixes
for the daemon-side advisories CVE-2026-33997 and CVE-2026-34040 that
govulncheck currently suppresses for this project. Upstream now ships
the maintained client and API types as the split modules
github.com/moby/moby/client and github.com/moby/moby/api, both of
which ToolHive already requires.

Migrate pkg/container/docker and pkg/container/images off
github.com/docker/docker/{client,api/types,pkg/stdcopy} and
github.com/docker/go-connections/nat onto github.com/moby/moby/client
and github.com/moby/moby/api. The new client exposes a redesigned
surface, so call sites are adapted to the per-operation
Options/Result structs, the client.Filters type, the network.Port
struct (replacing the nat.Port string), and netip.Addr host
addresses, while preserving existing behavior. After this change the
production build no longer imports github.com/docker/docker; it
remains only as an indirect test dependency via testcontainers-go.

Drop the now-dormant govulncheck ignore entries for GO-2026-4883 and
GO-2026-4887. These daemon-side Moby advisories are no longer in
govulncheck's called set once docker/docker is off the production
build, so the suppressions are no longer needed. Also drop the stale
GO-2026-4514 entry: buger/jsonparser is no longer in the module
graph, so that ignore is dead.

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rdimitrov rdimitrov force-pushed the rdimitrov/migrate-docker-docker-to-moby branch from 9b2461b to 233f4a9 Compare June 3, 2026 09:21
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jun 3, 2026
@rdimitrov rdimitrov merged commit 535e808 into main Jun 3, 2026
93 of 100 checks passed
@rdimitrov rdimitrov deleted the rdimitrov/migrate-docker-docker-to-moby branch June 3, 2026 10:28
rdimitrov added a commit to stacklok/toolhive-registry-server that referenced this pull request Jun 3, 2026
## Summary

Removes **all** govulncheck and grype vuln suppressions from the repo.
After this PR there are no active ignore entries in
`.github/workflows/security-scan.yml` (`IGNORED_VULNS=""`) or
`.grype.yaml` (`ignore: []`).

### Dropped (stale): `GO-2026-4771` / `GO-2026-4772` (pgx pgproto3
decode)
These were added when no fix for `github.com/jackc/pgx/v5` existed. The
fix shipped in **`pgx/v5 v5.9.0`**, and this project is already on
**`v5.9.2`** — so the advisories no longer apply, the version isn't in
the affected range, and govulncheck doesn't report them. Removing them
does not change which vulnerabilities are reported (they were never in
the found/called set).

### Dropped (intentionally surfaced): `GO-2026-4883` / `GO-2026-4887`
(docker daemon)
Also removes the `GHSA-x744-4wpc-v9h2` / `CVE-2026-34040` block from
`.grype.yaml`.

These are Docker daemon advisories (plugin privilege off-by-one and
AuthZ bypass). This client-only consumer reaches
`github.com/docker/docker` solely through toolhive's transitive import —
the daemon-side vulnerable paths are unreachable here.
`github.com/docker/docker` is frozen at `v28.5.2` with no fixed release,
so there is **nothing to bump to**; the real remediation is the upstream
`docker/docker` → `github.com/moby/moby/{client,api}` migration
(stacklok/toolhive#5420).

Rather than keep these masked, we remove the suppressions now so the
advisories surface. Once the moby-migrated toolhive is bumped here,
`docker/docker` leaves the dependency graph and both scanners go green
again — at which point no follow-up cleanup is needed because the ignore
entries are already gone.

## ⚠️ Expected CI state

Both `Security Scan / Go Vulnerability Check` and `Security Scan / Grype
Repository Scan` are **required status checks** and will be **red** on
this PR until the toolhive/moby bump lands:

- **Grype** fails on `GHSA-x744-4wpc-v9h2` (`docker/docker@v28.5.2`,
High, fix `29.3.1`).
- **govulncheck** fails on `GO-2026-4883` / `GO-2026-4887` (docker),
plus transiently on `GO-2026-5037/5038/5039` (stdlib, until CI's
`stable` toolchain reaches `go1.26.4`).

Merging therefore requires an admin bypass, and downstream PRs cut from
`main` will inherit the same red required checks until the bump merges.

## Notes
- No code changes — security-scan workflow + grype config only.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rdimitrov added a commit to stacklok/toolhive-registry-server that referenced this pull request Jun 3, 2026
## Summary

Bumps `github.com/stacklok/toolhive` **v0.28.3 → v0.29.0**, which
removes `github.com/docker/docker` from this project's build.

v0.29.0 migrates toolhive's container runtime off the **deprecated,
frozen** `docker/docker` module onto the maintained
`github.com/moby/moby/{client,api}` modules (stacklok/toolhive#5420).
With that, nothing in registry-server's import graph pulls
`docker/docker` anymore, so it drops out of the production build and
`go.mod` entirely (it remains only as an indirect `go.sum` checksum via
the module graph).

This **completes** the remediation that #805 set up: #805 removed the
`GO-2026-4883` / `GO-2026-4887` (`CVE-2026-33997` / `CVE-2026-34040`)
docker daemon-advisory suppressions ahead of this bump (with a note that
the scans were expected to fail until it landed). This is that bump.

## Verification (local)

- `go build ./...` ✓, `go vet ./...` ✓ — no breaking changes from the
minor bump
- `docker/docker` in production build: **0 packages**; `go mod why` →
"main module does not need package"; gone from `go.mod`
- **Grype** (`--only-fixed --fail-on high`, matching CI): no
`docker/docker` / high+fixed findings → exit 0 — the Grype Repository
Scan will pass
- govulncheck: no `docker/docker` findings (the advisories are off the
build)
- Refreshed the now-satisfied "until the bump lands" notes in
`security-scan.yml` and `.grype.yaml`

## Note on the `Go Vulnerability Check` job

It will likely **stay red on the pre-existing stdlib batch**
`GO-2026-5037/5038/5039` (`crypto/x509`, `mime`, `net/textproto`; fixed
in **Go 1.26.4**), reached via this project's own code. That's unrelated
to this bump — it's red on `main` for the same reason, pending CI's Go
toolchain reaching 1.26.4 (the Actions `go-versions` manifest still tops
out at 1.26.3). This PR only removes the **docker** advisories from the
reported set.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants