diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index c57580bfa1..fda149077c 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -56,23 +56,8 @@ jobs: - name: Check for vulnerabilities (with exclusions) run: | - # Ignored vulnerabilities with justification: - # GO-2026-4514: buger/jsonparser Delete function DoS via malformed JSON (CVE-2025-54410) - # Indirect dependency via mcp-go, invopop/jsonschema, wk8/go-ordered-map. - # The vulnerability is in the Delete function which is not called by ToolHive - # or any of its dependencies. No fixed version exists yet (all versions affected). - # GO-2026-4883: Off-by-one error in Moby plugin privilege validation (CVE-2026-33997) - # Affects the Docker daemon's plugin privilege handling code. ToolHive only uses - # the Docker client SDK to manage containers, not the daemon plugin subsystem. - # No fixed version exists for github.com/docker/docker; fix is only in - # github.com/moby/moby/v2 v2.0.0-beta.8+ which is not yet available as a - # docker/docker release. - # GO-2026-4887: AuthZ plugin bypass with oversized request bodies (CVE-2026-34040) - # Affects the Docker daemon's AuthZ plugin mechanism. ToolHive only uses the - # Docker client SDK and does not run or configure AuthZ plugins. No fixed version - # exists for github.com/docker/docker; fix is only in github.com/moby/moby/v2 - # v2.0.0-beta.8+ which is not yet available as a docker/docker release. - IGNORED_VULNS="GO-2026-4514 GO-2026-4883 GO-2026-4887" + # Ignored vulnerabilities with justification: none currently. + IGNORED_VULNS="" # Show the raw output for debugging echo "::group::govulncheck raw output" diff --git a/go.mod b/go.mod index 22b39e5ef6..e13f74a7a5 100644 --- a/go.mod +++ b/go.mod @@ -18,8 +18,8 @@ require ( github.com/charmbracelet/x/ansi v0.11.7 github.com/containerd/errdefs v1.0.0 github.com/coreos/go-oidc/v3 v3.18.0 - github.com/docker/docker v28.5.2+incompatible - github.com/docker/go-connections v0.7.0 + github.com/docker/docker v28.5.2+incompatible // indirect + github.com/docker/go-connections v0.7.0 // indirect github.com/evanphx/json-patch/v5 v5.9.11 github.com/go-chi/chi/v5 v5.2.5 github.com/go-git/go-billy/v5 v5.9.0 @@ -208,7 +208,7 @@ require ( github.com/mfridman/interpolate v0.0.2 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/moby/go-archive v0.1.0 // indirect - github.com/moby/moby/api v1.54.2 // indirect + github.com/moby/moby/api v1.54.2 github.com/moby/patternmatcher v0.6.0 // indirect github.com/moby/spdystream v0.5.1 // indirect github.com/moby/sys/sequential v0.6.0 // indirect diff --git a/pkg/container/docker/client.go b/pkg/container/docker/client.go index ea5ef8fe88..e85142682e 100644 --- a/pkg/container/docker/client.go +++ b/pkg/container/docker/client.go @@ -12,6 +12,7 @@ import ( "fmt" "io" "log/slog" + "net/netip" "os" "path/filepath" rt "runtime" @@ -21,14 +22,11 @@ import ( "time" "github.com/containerd/errdefs" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/filters" - "github.com/docker/docker/api/types/mount" - "github.com/docker/docker/api/types/network" - "github.com/docker/docker/client" - "github.com/docker/docker/pkg/stdcopy" - "github.com/docker/go-connections/nat" - v1 "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/moby/moby/api/pkg/stdcopy" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/mount" + "github.com/moby/moby/api/types/network" + mobyclient "github.com/moby/moby/client" "github.com/stacklok/toolhive-core/permissions" "github.com/stacklok/toolhive/pkg/container/docker/sdk" @@ -62,20 +60,31 @@ const ( // dockerAPI defines the minimal Docker client surface we need for unit-testing // ListWorkloads/GetWorkloadInfo through an adapter without requiring a live daemon. +// The signatures mirror the methods on *mobyclient.Client so the real client can +// be assigned directly to Client.api. type dockerAPI interface { - ContainerList(ctx context.Context, options container.ListOptions) ([]container.Summary, error) - ContainerInspect(ctx context.Context, containerID string) (container.InspectResponse, error) - ContainerStop(ctx context.Context, containerID string, options container.StopOptions) error - ContainerCreate( + ContainerList(ctx context.Context, options mobyclient.ContainerListOptions) (mobyclient.ContainerListResult, error) + ContainerInspect( ctx context.Context, - config *container.Config, - hostConfig *container.HostConfig, - networkingConfig *network.NetworkingConfig, - platform *v1.Platform, - containerName string, - ) (container.CreateResponse, error) - ContainerStart(ctx context.Context, containerID string, options container.StartOptions) error - ContainerRemove(ctx context.Context, containerID string, options container.RemoveOptions) error + containerID string, + options mobyclient.ContainerInspectOptions, + ) (mobyclient.ContainerInspectResult, error) + ContainerStop( + ctx context.Context, + containerID string, + options mobyclient.ContainerStopOptions, + ) (mobyclient.ContainerStopResult, error) + ContainerCreate(ctx context.Context, options mobyclient.ContainerCreateOptions) (mobyclient.ContainerCreateResult, error) + ContainerStart( + ctx context.Context, + containerID string, + options mobyclient.ContainerStartOptions, + ) (mobyclient.ContainerStartResult, error) + ContainerRemove( + ctx context.Context, + containerID string, + options mobyclient.ContainerRemoveOptions, + ) (mobyclient.ContainerRemoveResult, error) } // deployOps defines the internal operations used by DeployWorkload. @@ -129,7 +138,7 @@ type deployOps interface { type Client struct { runtimeType runtime.Type socketPath string - client *client.Client + client *mobyclient.Client api dockerAPI imageManager images.ImageManager ops deployOps @@ -337,17 +346,17 @@ func (c *Client) DeployWorkload( // ListWorkloads lists workloads func (c *Client) ListWorkloads(ctx context.Context) ([]runtime.ContainerInfo, error) { // Create filter for toolhive containers - filterArgs := filters.NewArgs() - filterArgs.Add("label", "toolhive=true") + filterArgs := mobyclient.Filters{}.Add("label", "toolhive=true") // List containers - containers, err := c.api.ContainerList(ctx, container.ListOptions{ + listResult, err := c.api.ContainerList(ctx, mobyclient.ContainerListOptions{ All: true, Filters: filterArgs, }) if err != nil { return nil, NewContainerError(err, "", fmt.Sprintf("failed to list containers: %v", err)) } + containers := listResult.Items // Convert to our ContainerInfo format result := make([]runtime.ContainerInfo, 0, len(containers)) @@ -381,7 +390,7 @@ func (c *Client) ListWorkloads(ctx context.Context) ([]runtime.ContainerInfo, er Name: name, Image: c.Image, Status: c.Status, - State: dockerToDomainStatus(c.State), + State: dockerToDomainStatus(string(c.State)), Created: created, Labels: c.Labels, Ports: ports, @@ -411,7 +420,7 @@ func (c *Client) StopWorkload(ctx context.Context, workloadName string) error { // Use a reasonable timeout timeoutSeconds := 30 - err = c.api.ContainerStop(ctx, workloadName, container.StopOptions{Timeout: &timeoutSeconds}) + _, err = c.api.ContainerStop(ctx, workloadName, mobyclient.ContainerStopOptions{Timeout: &timeoutSeconds}) if err != nil { return NewContainerError(err, workloadName, fmt.Sprintf("failed to stop workload: %v", err)) } @@ -497,7 +506,7 @@ func (c *Client) GetWorkloadLogs(ctx context.Context, workloadName string, follo tail = fmt.Sprintf("%d", lines) } - options := container.LogsOptions{ + options := mobyclient.ContainerLogsOptions{ ShowStdout: true, ShowStderr: true, Follow: follow, @@ -577,9 +586,9 @@ func (c *Client) GetWorkloadInfo(ctx context.Context, workloadName string) (runt } ports = append(ports, runtime.PortMapping{ - ContainerPort: containerPort.Int(), + ContainerPort: int(containerPort.Num()), HostPort: hostPort, - Protocol: containerPort.Proto(), + Protocol: string(containerPort.Proto()), }) } } @@ -599,8 +608,8 @@ func (c *Client) GetWorkloadInfo(ctx context.Context, workloadName string) (runt return runtime.ContainerInfo{ Name: strings.TrimPrefix(info.Name, "/"), Image: info.Config.Image, - Status: info.State.Status, - State: dockerToDomainStatus(info.State.Status), + Status: string(info.State.Status), + State: dockerToDomainStatus(string(info.State.Status)), Created: created, StartedAt: startedAt, Labels: info.Config.Labels, @@ -620,7 +629,7 @@ func (c *Client) AttachToWorkload(ctx context.Context, workloadName string) (io. } // Attach to workload - resp, err := c.client.ContainerAttach(ctx, workloadName, container.AttachOptions{ + resp, err := c.client.ContainerAttach(ctx, workloadName, mobyclient.ContainerAttachOptions{ Stream: true, Stdin: true, Stdout: true, @@ -655,7 +664,7 @@ func (c *Client) AttachToWorkload(ctx context.Context, workloadName string) (io. // This is used to verify that the runtime is operational and can manage workloads. func (c *Client) IsRunning(ctx context.Context) error { // Try to ping the Docker server - _, err := c.client.Ping(ctx) + _, err := c.client.Ping(ctx, mobyclient.PingOptions{}) if err != nil { return fmt.Errorf("failed to ping Docker server: %w", err) } @@ -865,18 +874,16 @@ func (c *Client) getPermissionConfigFromProfile( // Uses container runtime's name filter for efficiency but then verifies exact match to prevent partial matching func (c *Client) findExistingContainer(ctx context.Context, name string) (string, error) { // Use name filter to narrow results, then verify exact match - containers, err := c.api.ContainerList(ctx, container.ListOptions{ - All: true, // Include stopped containers - Filters: filters.NewArgs( - filters.Arg("name", name), - ), + listResult, err := c.api.ContainerList(ctx, mobyclient.ContainerListOptions{ + All: true, // Include stopped containers + Filters: mobyclient.Filters{}.Add("name", name), }) if err != nil { return "", NewContainerError(err, "", fmt.Sprintf("failed to list containers: %v", err)) } // Verify exact name match from the filtered results (name filter does partial matching) - for _, cont := range containers { + for _, cont := range listResult.Items { for _, containerName := range cont.Names { // Container names in the API have a leading slash if containerName == "/"+name || containerName == name { @@ -1108,10 +1115,11 @@ func (c *Client) handleExistingContainer( desiredHostConfig *container.HostConfig, ) (bool, error) { // Get container info - info, err := c.api.ContainerInspect(ctx, containerID) + inspectResult, err := c.api.ContainerInspect(ctx, containerID, mobyclient.ContainerInspectOptions{}) if err != nil { return false, NewContainerError(err, containerID, fmt.Sprintf("failed to inspect container: %v", err)) } + info := inspectResult.Container // Compare configurations if compareContainerConfig(&info, desiredConfig, desiredHostConfig) { @@ -1120,7 +1128,7 @@ func (c *Client) handleExistingContainer( // Check if the container is running if !info.State.Running { // Container exists but is not running, start it - err = c.api.ContainerStart(ctx, containerID, container.StartOptions{}) + _, err = c.api.ContainerStart(ctx, containerID, mobyclient.ContainerStartOptions{}) if err != nil { return false, NewContainerError(err, containerID, fmt.Sprintf("failed to start existing container: %v", err)) } @@ -1149,21 +1157,21 @@ func (c *Client) createNetwork( ) error { // Check if the network already exists // Use name filter for efficiency but verify exact match to avoid partial matching - networks, err := c.client.NetworkList(ctx, network.ListOptions{ - Filters: filters.NewArgs(filters.Arg("name", name)), + networks, err := c.client.NetworkList(ctx, mobyclient.NetworkListOptions{ + Filters: mobyclient.Filters{}.Add("name", name), }) if err != nil { return fmt.Errorf("failed to list networks: %w", err) } // Verify exact name match from filtered results - for _, n := range networks { + for _, n := range networks.Items { if n.Name == name { // Network already exists return nil } } - networkCreate := network.CreateOptions{ + networkCreate := mobyclient.NetworkCreateOptions{ Driver: "bridge", Internal: internal, Labels: labels, @@ -1183,14 +1191,14 @@ func (c *Client) createNetwork( // the daemon directly is more accurate than hardcoding platform-specific IPs. // Falls back to dockerDefaultBridgeGatewayIP on any error. func (c *Client) getDockerBridgeGatewayIP(ctx context.Context) string { - nr, err := c.client.NetworkInspect(ctx, "bridge", network.InspectOptions{}) + nr, err := c.client.NetworkInspect(ctx, "bridge", mobyclient.NetworkInspectOptions{}) if err != nil { slog.Debug("failed to inspect bridge network, using default gateway IP", "error", err) return dockerDefaultBridgeGatewayIP } - for _, cfg := range nr.IPAM.Config { - if cfg.Gateway != "" { - return cfg.Gateway + for _, cfg := range nr.Network.IPAM.Config { + if cfg.Gateway.IsValid() { + return cfg.Gateway.String() } } slog.Debug("bridge network has no gateway in IPAM config, using default gateway IP") @@ -1200,8 +1208,8 @@ func (c *Client) getDockerBridgeGatewayIP(ctx context.Context) string { // DeleteNetwork deletes a network by name. func (c *Client) deleteNetwork(ctx context.Context, name string) error { // find the network by name using filter for efficiency but verify exact match - networks, err := c.client.NetworkList(ctx, network.ListOptions{ - Filters: filters.NewArgs(filters.Arg("name", name)), + networks, err := c.client.NetworkList(ctx, mobyclient.NetworkListOptions{ + Filters: mobyclient.Filters{}.Add("name", name), }) if err != nil { return err @@ -1209,7 +1217,7 @@ func (c *Client) deleteNetwork(ctx context.Context, name string) error { // Verify exact name match from filtered results var networkToRemove *network.Summary - for _, n := range networks { + for _, n := range networks.Items { if n.Name == name { networkToRemove = &n break @@ -1222,7 +1230,7 @@ func (c *Client) deleteNetwork(ctx context.Context, name string) error { return nil } - if err := c.client.NetworkRemove(ctx, networkToRemove.ID); err != nil { + if _, err := c.client.NetworkRemove(ctx, networkToRemove.ID, mobyclient.NetworkRemoveOptions{}); err != nil { return fmt.Errorf("failed to remove network %s: %w", name, err) } return nil @@ -1230,7 +1238,7 @@ func (c *Client) deleteNetwork(ctx context.Context, name string) error { // removeContainer removes a container by ID, without removing any associated networks or proxy containers. func (c *Client) removeContainer(ctx context.Context, containerID string) error { - err := c.api.ContainerRemove(ctx, containerID, container.RemoveOptions{ + _, err := c.api.ContainerRemove(ctx, containerID, mobyclient.ContainerRemoveOptions{ Force: true, }) if err != nil { @@ -1276,7 +1284,7 @@ func (c *Client) removeProxyContainers( continue } - err = c.client.ContainerRemove(ctx, containerId, container.RemoveOptions{ + _, err = c.client.ContainerRemove(ctx, containerId, mobyclient.ContainerRemoveOptions{ Force: true, }) if err != nil { @@ -1322,13 +1330,13 @@ func setupExposedPorts(config *container.Config, exposedPorts map[string]struct{ return nil } - config.ExposedPorts = nat.PortSet{} + config.ExposedPorts = network.PortSet{} for port := range exposedPorts { - natPort, err := nat.NewPort("tcp", strings.Split(port, "/")[0]) + networkPort, err := network.ParsePort(strings.Split(port, "/")[0]) if err != nil { return fmt.Errorf("failed to parse port: %w", err) } - config.ExposedPorts[natPort] = struct{}{} + config.ExposedPorts[networkPort] = struct{}{} } return nil @@ -1340,26 +1348,40 @@ func setupPortBindings(hostConfig *container.HostConfig, portBindings map[string return nil } - hostConfig.PortBindings = nat.PortMap{} + hostConfig.PortBindings = network.PortMap{} for port, bindings := range portBindings { - natPort, err := nat.NewPort("tcp", strings.Split(port, "/")[0]) + networkPort, err := network.ParsePort(strings.Split(port, "/")[0]) if err != nil { return fmt.Errorf("failed to parse port: %w", err) } - natBindings := make([]nat.PortBinding, len(bindings)) + networkBindings := make([]network.PortBinding, len(bindings)) for i, binding := range bindings { - natBindings[i] = nat.PortBinding{ - HostIP: binding.HostIP, + hostIP, err := parseHostIP(binding.HostIP) + if err != nil { + return fmt.Errorf("failed to parse host IP %q: %w", binding.HostIP, err) + } + networkBindings[i] = network.PortBinding{ + HostIP: hostIP, HostPort: binding.HostPort, } } - hostConfig.PortBindings[natPort] = natBindings + hostConfig.PortBindings[networkPort] = networkBindings } return nil } +// parseHostIP converts a host IP string into a netip.Addr. An empty string +// (meaning "all interfaces") maps to the zero netip.Addr, preserving the +// previous behavior where an empty HostIP string was passed through verbatim. +func parseHostIP(hostIP string) (netip.Addr, error) { + if hostIP == "" { + return netip.Addr{}, nil + } + return netip.ParseAddr(hostIP) +} + func (c *Client) createContainer( ctx context.Context, containerName string, @@ -1392,14 +1414,13 @@ func (c *Client) createContainer( } // Create the container - resp, err := c.api.ContainerCreate( - ctx, - config, - hostConfig, - networkConfig, - nil, - containerName, - ) + resp, err := c.api.ContainerCreate(ctx, mobyclient.ContainerCreateOptions{ + Config: config, + HostConfig: hostConfig, + NetworkingConfig: networkConfig, + Platform: nil, + Name: containerName, + }) if err != nil { return "", NewContainerError(err, "", fmt.Sprintf("failed to create container: %v", err)) } @@ -1409,7 +1430,7 @@ func (c *Client) createContainer( } // Start the container - err = c.api.ContainerStart(ctx, resp.ID, container.StartOptions{}) + _, err = c.api.ContainerStart(ctx, resp.ID, mobyclient.ContainerStartOptions{}) if err != nil { return "", NewContainerError(err, resp.ID, fmt.Sprintf("failed to start container: %v", err)) } @@ -1465,16 +1486,21 @@ func (c *Client) createDnsContainer(ctx context.Context, dnsContainerName string return "", "", fmt.Errorf("failed to create dns container: %w", err) } - dnsContainerResponse, err := c.client.ContainerInspect(ctx, dnsContainerId) + dnsInspectResult, err := c.client.ContainerInspect(ctx, dnsContainerId, mobyclient.ContainerInspectOptions{}) if err != nil { return "", "", fmt.Errorf("failed to inspect DNS container: %w", err) } - dnsNetworkSettings, ok := dnsContainerResponse.NetworkSettings.Networks[networkName] + dnsNetworkSettings, ok := dnsInspectResult.Container.NetworkSettings.Networks[networkName] if !ok { return "", "", fmt.Errorf("network %s not found in container's network settings", networkName) } - dnsContainerIP := dnsNetworkSettings.IPAddress + // EndpointSettings.IPAddress is a netip.Addr; render it back to the string + // form the callers expect, treating the zero value as an empty address. + dnsContainerIP := "" + if dnsNetworkSettings.IPAddress.IsValid() { + dnsContainerIP = dnsNetworkSettings.IPAddress.String() + } return dnsContainerId, dnsContainerIP, nil } @@ -1520,7 +1546,11 @@ func (c *Client) createMcpContainer( }, } if additionalDNS != "" { - hostConfig.DNS = []string{additionalDNS} + dnsAddr, err := netip.ParseAddr(additionalDNS) + if err != nil { + return NewContainerError(err, "", fmt.Sprintf("invalid additional DNS address %q: %v", additionalDNS, err)) + } + hostConfig.DNS = []netip.Addr{dnsAddr} } // Configure ports if options are provided @@ -1699,16 +1729,16 @@ func (c *Client) stopProxyContainer(ctx context.Context, containerName string, t if containerId == "" { return } - if err := c.api.ContainerStop(ctx, containerId, container.StopOptions{Timeout: &timeoutSeconds}); err != nil { + if _, err := c.api.ContainerStop(ctx, containerId, mobyclient.ContainerStopOptions{Timeout: &timeoutSeconds}); err != nil { slog.Debug("failed to stop internal container", "name", containerName, "error", err) } } func (c *Client) deleteNetworks(ctx context.Context, containerName string) error { // Delete networks if there are no containers using them. - toolHiveContainers, err := c.client.ContainerList(ctx, container.ListOptions{ + toolHiveContainers, err := c.client.ContainerList(ctx, mobyclient.ContainerListOptions{ All: true, - Filters: filters.NewArgs(filters.Arg("label", "toolhive=true")), + Filters: mobyclient.Filters{}.Add("label", "toolhive=true"), }) if err != nil { return fmt.Errorf("failed to list containers: %w", err) @@ -1721,7 +1751,7 @@ func (c *Client) deleteNetworks(ctx context.Context, containerName string) error slog.Warn("failed to delete network", "name", networkName, "error", err) } - if len(toolHiveContainers) == 0 { + if len(toolHiveContainers.Items) == 0 { // remove external network if err := c.deleteNetwork(ctx, "toolhive-external"); err != nil { // just log the error and continue @@ -1750,17 +1780,18 @@ func dockerToDomainStatus(status string) runtime.WorkloadStatus { // findContainerByLabel finds a container by the base name label. // Returns the container ID if found, empty string otherwise. func (c *Client) findContainerByLabel(ctx context.Context, workloadName string) (string, error) { - filterArgs := filters.NewArgs() - filterArgs.Add("label", "toolhive=true") - filterArgs.Add("label", fmt.Sprintf("toolhive-basename=%s", workloadName)) + filterArgs := mobyclient.Filters{}. + Add("label", "toolhive=true"). + Add("label", fmt.Sprintf("toolhive-basename=%s", workloadName)) - containers, err := c.api.ContainerList(ctx, container.ListOptions{ + listResult, err := c.api.ContainerList(ctx, mobyclient.ContainerListOptions{ All: true, Filters: filterArgs, }) if err != nil { return "", NewContainerError(err, "", fmt.Sprintf("failed to list containers: %v", err)) } + containers := listResult.Items if len(containers) == 0 { return "", nil @@ -1786,17 +1817,18 @@ func (c *Client) findContainerByLabel(ctx context.Context, workloadName string) // Returns the container ID if found, empty string otherwise. // Uses container runtime's name filter for efficiency but then verifies exact match to prevent partial matching func (c *Client) findContainerByExactName(ctx context.Context, workloadName string) (string, error) { - filterArgs := filters.NewArgs() - filterArgs.Add("label", "toolhive=true") - filterArgs.Add("name", workloadName) // Use name filter for efficiency + filterArgs := mobyclient.Filters{}. + Add("label", "toolhive=true"). + Add("name", workloadName) // Use name filter for efficiency - containers, err := c.api.ContainerList(ctx, container.ListOptions{ + listResult, err := c.api.ContainerList(ctx, mobyclient.ContainerListOptions{ All: true, Filters: filterArgs, }) if err != nil { return "", NewContainerError(err, "", fmt.Sprintf("failed to list containers: %v", err)) } + containers := listResult.Items if len(containers) == 0 { return "", nil @@ -1826,7 +1858,8 @@ func (c *Client) inspectContainerByName(ctx context.Context, workloadName string return empty, err } if containerID != "" { - return c.api.ContainerInspect(ctx, containerID) + result, err := c.api.ContainerInspect(ctx, containerID, mobyclient.ContainerInspectOptions{}) + return result.Container, err } // Fall back to exact name matching for backward compatibility @@ -1838,5 +1871,6 @@ func (c *Client) inspectContainerByName(ctx context.Context, workloadName string return empty, NewContainerError(runtime.ErrWorkloadNotFound, workloadName, "no containers found") } - return c.api.ContainerInspect(ctx, containerID) + result, err := c.api.ContainerInspect(ctx, containerID, mobyclient.ContainerInspectOptions{}) + return result.Container, err } diff --git a/pkg/container/docker/client_config_test.go b/pkg/container/docker/client_config_test.go index 3de902040d..629780caa8 100644 --- a/pkg/container/docker/client_config_test.go +++ b/pkg/container/docker/client_config_test.go @@ -4,11 +4,12 @@ package docker import ( + "net/netip" "testing" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/mount" - "github.com/docker/go-connections/nat" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/mount" + "github.com/moby/moby/api/types/network" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -28,9 +29,9 @@ func TestSetupExposedPorts_SetsPorts(t *testing.T) { require.NoError(t, err) require.NotNil(t, cfg.ExposedPorts) - p8080, err := nat.NewPort("tcp", "8080") + p8080, err := network.ParsePort("8080") require.NoError(t, err) - p9090, err := nat.NewPort("tcp", "9090") + p9090, err := network.ParsePort("9090") require.NoError(t, err) assert.Contains(t, cfg.ExposedPorts, p8080) @@ -63,14 +64,14 @@ func TestSetupPortBindings_SetsBindings(t *testing.T) { require.NoError(t, err) require.NotNil(t, hostCfg.PortBindings) - p8080, err := nat.NewPort("tcp", "8080") + p8080, err := network.ParsePort("8080") require.NoError(t, err) got, ok := hostCfg.PortBindings[p8080] require.True(t, ok) require.Len(t, got, 2) - assert.Equal(t, nat.PortBinding{HostIP: "127.0.0.1", HostPort: "8081"}, got[0]) - assert.Equal(t, nat.PortBinding{HostIP: "", HostPort: "8082"}, got[1]) + assert.Equal(t, network.PortBinding{HostIP: netip.MustParseAddr("127.0.0.1"), HostPort: "8081"}, got[0]) + assert.Equal(t, network.PortBinding{HostIP: netip.Addr{}, HostPort: "8082"}, got[1]) } func TestConvertMounts_BindMounts(t *testing.T) { @@ -123,15 +124,13 @@ func TestCompareHostConfig_EqualAndMismatch(t *testing.T) { t.Parallel() existing := &container.InspectResponse{ - ContainerJSONBase: &container.ContainerJSONBase{ - HostConfig: &container.HostConfig{ - NetworkMode: "bridge", - CapAdd: []string{"CAP_A"}, - CapDrop: []string{"ALL"}, - SecurityOpt: []string{"seccomp:unconfined"}, - Privileged: false, - RestartPolicy: container.RestartPolicy{Name: "unless-stopped"}, - }, + HostConfig: &container.HostConfig{ + NetworkMode: "bridge", + CapAdd: []string{"CAP_A"}, + CapDrop: []string{"ALL"}, + SecurityOpt: []string{"seccomp:unconfined"}, + Privileged: false, + RestartPolicy: container.RestartPolicy{Name: "unless-stopped"}, }, } @@ -164,18 +163,16 @@ func TestComparePortConfig_EqualAndMismatch(t *testing.T) { })) // Build existing to match desired - p8080, err := nat.NewPort("tcp", "8080") + p8080, err := network.ParsePort("8080") require.NoError(t, err) existing := &container.InspectResponse{ Config: &container.Config{ - ExposedPorts: nat.PortSet{p8080: {}}, + ExposedPorts: network.PortSet{p8080: {}}, }, - ContainerJSONBase: &container.ContainerJSONBase{ - HostConfig: &container.HostConfig{ - PortBindings: nat.PortMap{ - p8080: []nat.PortBinding{{HostIP: "0.0.0.0", HostPort: "18080"}}, - }, + HostConfig: &container.HostConfig{ + PortBindings: network.PortMap{ + p8080: []network.PortBinding{{HostIP: netip.MustParseAddr("0.0.0.0"), HostPort: "18080"}}, }, }, } @@ -183,7 +180,7 @@ func TestComparePortConfig_EqualAndMismatch(t *testing.T) { assert.True(t, comparePortConfig(existing, desiredCfg, desiredHost)) // Mismatch: different host port - existing.HostConfig.PortBindings[p8080] = []nat.PortBinding{{HostIP: "0.0.0.0", HostPort: "18081"}} + existing.HostConfig.PortBindings[p8080] = []network.PortBinding{{HostIP: netip.MustParseAddr("0.0.0.0"), HostPort: "18081"}} assert.False(t, comparePortConfig(existing, desiredCfg, desiredHost)) } @@ -222,7 +219,7 @@ func TestCompareContainerConfig_AllMatch(t *testing.T) { })) // Existing configuration (must be a superset for env vars) - p8080, err := nat.NewPort("tcp", "8080") + p8080, err := network.ParsePort("8080") require.NoError(t, err) existing := &container.InspectResponse{ @@ -236,23 +233,21 @@ func TestCompareContainerConfig_AllMatch(t *testing.T) { AttachStderr: true, OpenStdin: true, Tty: false, - ExposedPorts: nat.PortSet{p8080: {}}, + ExposedPorts: network.PortSet{p8080: {}}, }, - ContainerJSONBase: &container.ContainerJSONBase{ - HostConfig: &container.HostConfig{ - NetworkMode: "bridge", - CapAdd: []string{"NET_BIND_SERVICE"}, - CapDrop: []string{"ALL"}, - SecurityOpt: []string{"seccomp:unconfined"}, - Privileged: false, - RestartPolicy: container.RestartPolicy{Name: "unless-stopped"}, - Mounts: []mount.Mount{ - {Type: mount.TypeBind, Source: "/src1", Target: "/dst1", ReadOnly: true}, - {Type: mount.TypeBind, Source: "/src2", Target: "/dst2", ReadOnly: false}, - }, - PortBindings: nat.PortMap{ - p8080: []nat.PortBinding{{HostIP: "", HostPort: "18080"}}, - }, + HostConfig: &container.HostConfig{ + NetworkMode: "bridge", + CapAdd: []string{"NET_BIND_SERVICE"}, + CapDrop: []string{"ALL"}, + SecurityOpt: []string{"seccomp:unconfined"}, + Privileged: false, + RestartPolicy: container.RestartPolicy{Name: "unless-stopped"}, + Mounts: []mount.Mount{ + {Type: mount.TypeBind, Source: "/src1", Target: "/dst1", ReadOnly: true}, + {Type: mount.TypeBind, Source: "/src2", Target: "/dst2", ReadOnly: false}, + }, + PortBindings: network.PortMap{ + p8080: []network.PortBinding{{HostIP: netip.Addr{}, HostPort: "18080"}}, }, }, } diff --git a/pkg/container/docker/client_create_test.go b/pkg/container/docker/client_create_test.go index fad0e680b9..3d7515b43c 100644 --- a/pkg/container/docker/client_create_test.go +++ b/pkg/container/docker/client_create_test.go @@ -6,11 +6,12 @@ package docker import ( "context" "fmt" + "net/netip" "testing" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/network" - "github.com/docker/go-connections/nat" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" + mobyclient "github.com/moby/moby/client" v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -38,7 +39,7 @@ func TestCreateMcpContainer_Isolated_WiresConfigAndNetworks(t *testing.T) { assert.Equal(t, "app", name) return container.CreateResponse{ID: "cid-new"}, nil }, - startFunc: func(_ context.Context, id string, _ container.StartOptions) error { + startFunc: func(_ context.Context, id string, _ mobyclient.ContainerStartOptions) error { startCalled = true assert.Equal(t, "cid-new", id) return nil @@ -98,7 +99,7 @@ func TestCreateMcpContainer_Isolated_WiresConfigAndNetworks(t *testing.T) { assert.Equal(t, labels, gotConfig.Labels) // Exposed ports set - p8080, err := nat.NewPort("tcp", "8080") + p8080, err := network.ParsePort("8080") require.NoError(t, err) require.Contains(t, gotConfig.ExposedPorts, p8080) @@ -109,12 +110,12 @@ func TestCreateMcpContainer_Isolated_WiresConfigAndNetworks(t *testing.T) { assert.Equal(t, []string{"ALL"}, []string(gotHost.CapDrop)) assert.Equal(t, []string{"seccomp:unconfined"}, gotHost.SecurityOpt) assert.Equal(t, false, gotHost.Privileged) - assert.Equal(t, []string{"1.2.3.4"}, gotHost.DNS) + assert.Equal(t, []netip.Addr{netip.MustParseAddr("1.2.3.4")}, gotHost.DNS) // Port bindings wired require.Contains(t, gotHost.PortBindings, p8080) require.Len(t, gotHost.PortBindings[p8080], 1) - assert.Equal(t, "127.0.0.1", gotHost.PortBindings[p8080][0].HostIP) + assert.Equal(t, netip.MustParseAddr("127.0.0.1"), gotHost.PortBindings[p8080][0].HostIP) assert.Equal(t, "18080", gotHost.PortBindings[p8080][0].HostPort) // Networking config points to internal network when isolated @@ -134,7 +135,7 @@ func TestCreateMcpContainer_NonIsolated_UsesExternalNetwork(t *testing.T) { gotNet = netCfg return container.CreateResponse{ID: "cid-new"}, nil }, - startFunc: func(_ context.Context, _ string, _ container.StartOptions) error { + startFunc: func(_ context.Context, _ string, _ mobyclient.ContainerStartOptions) error { return nil }, } @@ -171,7 +172,7 @@ func TestCreateContainer_CreateAndStart_New(t *testing.T) { var started bool api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { // No existing container return []container.Summary{}, nil }, @@ -181,7 +182,7 @@ func TestCreateContainer_CreateAndStart_New(t *testing.T) { gotNet = netCfg return container.CreateResponse{ID: "cid-new"}, nil }, - startFunc: func(_ context.Context, id string, _ container.StartOptions) error { + startFunc: func(_ context.Context, id string, _ mobyclient.ContainerStartOptions) error { started = true assert.Equal(t, "cid-new", id) return nil @@ -218,7 +219,7 @@ func TestCreateContainer_ReuseExisting_WhenConfigMatchesAndStartIfStopped(t *tes api := &fakeDockerAPI{ // findExistingContainer will call ContainerList filtering by name - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return []container.Summary{ {ID: "cid-reuse", Names: []string{"/reuse"}}, }, nil @@ -227,18 +228,16 @@ func TestCreateContainer_ReuseExisting_WhenConfigMatchesAndStartIfStopped(t *tes require.Equal(t, "cid-reuse", id) // Existing matches desired (all zero-values) and is not running return container.InspectResponse{ - Config: &container.Config{}, - ContainerJSONBase: &container.ContainerJSONBase{ - HostConfig: &container.HostConfig{}, - State: &container.State{Status: "exited", Running: false}, - }, + Config: &container.Config{}, + HostConfig: &container.HostConfig{}, + State: &container.State{Status: "exited", Running: false}, }, nil }, createFunc: func(_ context.Context, _ *container.Config, _ *container.HostConfig, _ *network.NetworkingConfig, _ *v1.Platform, _ string) (container.CreateResponse, error) { createCalled = true return container.CreateResponse{ID: "cid-should-not"}, nil }, - startFunc: func(_ context.Context, id string, _ container.StartOptions) error { + startFunc: func(_ context.Context, id string, _ mobyclient.ContainerStartOptions) error { startCalled = true assert.Equal(t, "cid-reuse", id) return nil @@ -266,7 +265,7 @@ func TestCreateContainer_Mismatch_RemovesAndRecreates(t *testing.T) { hcfg := &container.HostConfig{} api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return []container.Summary{ {ID: "cid-old", Names: []string{"/app"}}, }, nil @@ -275,14 +274,12 @@ func TestCreateContainer_Mismatch_RemovesAndRecreates(t *testing.T) { require.Equal(t, "cid-old", id) // Existing image different -> mismatch path return container.InspectResponse{ - Config: &container.Config{Image: "different"}, - ContainerJSONBase: &container.ContainerJSONBase{ - HostConfig: &container.HostConfig{}, - State: &container.State{Status: "running", Running: true}, - }, + Config: &container.Config{Image: "different"}, + HostConfig: &container.HostConfig{}, + State: &container.State{Status: "running", Running: true}, }, nil }, - removeFunc: func(_ context.Context, id string, _ container.RemoveOptions) error { + removeFunc: func(_ context.Context, id string, _ mobyclient.ContainerRemoveOptions) error { removedID = id return nil }, @@ -290,7 +287,7 @@ func TestCreateContainer_Mismatch_RemovesAndRecreates(t *testing.T) { created = true return container.CreateResponse{ID: "cid-new"}, nil }, - startFunc: func(_ context.Context, id string, _ container.StartOptions) error { + startFunc: func(_ context.Context, id string, _ mobyclient.ContainerStartOptions) error { started = true assert.Equal(t, "cid-new", id) return nil @@ -316,7 +313,7 @@ func TestCreateMcpContainer_InvalidExposedPort_ReturnsError(t *testing.T) { t.Fatalf("ContainerCreate should not be called when exposed ports are invalid") return container.CreateResponse{}, nil }, - startFunc: func(_ context.Context, _ string, _ container.StartOptions) error { + startFunc: func(_ context.Context, _ string, _ mobyclient.ContainerStartOptions) error { t.Fatalf("ContainerStart should not be called when exposed ports are invalid") return nil }, @@ -356,7 +353,7 @@ func TestCreateMcpContainer_InvalidPortBinding_ReturnsError(t *testing.T) { t.Fatalf("ContainerCreate should not be called when port bindings are invalid") return container.CreateResponse{}, nil }, - startFunc: func(_ context.Context, _ string, _ container.StartOptions) error { + startFunc: func(_ context.Context, _ string, _ mobyclient.ContainerStartOptions) error { t.Fatalf("ContainerStart should not be called when port bindings are invalid") return nil }, @@ -400,7 +397,7 @@ func TestCreateMcpContainer_NoAdditionalDNS_DNSNotSet(t *testing.T) { gotHost = host return container.CreateResponse{ID: "cid-dns"}, nil }, - startFunc: func(_ context.Context, _ string, _ container.StartOptions) error { + startFunc: func(_ context.Context, _ string, _ mobyclient.ContainerStartOptions) error { return nil }, } @@ -432,7 +429,7 @@ func TestCreateContainer_ListError_Propagates(t *testing.T) { ctx := t.Context() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return nil, fmt.Errorf("list fail") }, } @@ -448,7 +445,7 @@ func TestCreateContainer_InspectError_Propagates(t *testing.T) { ctx := t.Context() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return []container.Summary{ {ID: "cid1", Names: []string{"/x"}}, }, nil @@ -469,21 +466,19 @@ func TestCreateContainer_StartExistingError_Wrapped(t *testing.T) { ctx := t.Context() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return []container.Summary{ {ID: "cid-exist", Names: []string{"/svc"}}, }, nil }, inspectFunc: func(_ context.Context, _ string) (container.InspectResponse, error) { return container.InspectResponse{ - Config: &container.Config{}, - ContainerJSONBase: &container.ContainerJSONBase{ - HostConfig: &container.HostConfig{}, - State: &container.State{Status: "exited", Running: false}, - }, + Config: &container.Config{}, + HostConfig: &container.HostConfig{}, + State: &container.State{Status: "exited", Running: false}, }, nil }, - startFunc: func(_ context.Context, _ string, _ container.StartOptions) error { + startFunc: func(_ context.Context, _ string, _ mobyclient.ContainerStartOptions) error { return fmt.Errorf("start fail") }, } @@ -500,7 +495,7 @@ func TestCreateContainer_CreateError_Wrapped(t *testing.T) { ctx := t.Context() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return []container.Summary{}, nil }, createFunc: func(_ context.Context, _ *container.Config, _ *container.HostConfig, _ *network.NetworkingConfig, _ *v1.Platform, _ string) (container.CreateResponse, error) { @@ -520,13 +515,13 @@ func TestCreateContainer_StartError_Wrapped(t *testing.T) { ctx := t.Context() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return []container.Summary{}, nil }, createFunc: func(_ context.Context, _ *container.Config, _ *container.HostConfig, _ *network.NetworkingConfig, _ *v1.Platform, _ string) (container.CreateResponse, error) { return container.CreateResponse{ID: "cid-new"}, nil }, - startFunc: func(_ context.Context, _ string, _ container.StartOptions) error { + startFunc: func(_ context.Context, _ string, _ mobyclient.ContainerStartOptions) error { return fmt.Errorf("start fail") }, } @@ -543,7 +538,7 @@ func TestCreateContainer_RemoveError_Propagates(t *testing.T) { ctx := t.Context() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return []container.Summary{ {ID: "cid-old", Names: []string{"/svc"}}, }, nil @@ -551,14 +546,12 @@ func TestCreateContainer_RemoveError_Propagates(t *testing.T) { inspectFunc: func(_ context.Context, _ string) (container.InspectResponse, error) { // Mismatch to force recreation return container.InspectResponse{ - Config: &container.Config{Image: "different"}, - ContainerJSONBase: &container.ContainerJSONBase{ - HostConfig: &container.HostConfig{}, - State: &container.State{Status: "running", Running: true}, - }, + Config: &container.Config{Image: "different"}, + HostConfig: &container.HostConfig{}, + State: &container.State{Status: "running", Running: true}, }, nil }, - removeFunc: func(_ context.Context, id string, _ container.RemoveOptions) error { + removeFunc: func(_ context.Context, id string, _ mobyclient.ContainerRemoveOptions) error { return fmt.Errorf("remove fail: %s", id) }, } diff --git a/pkg/container/docker/client_deploy_test.go b/pkg/container/docker/client_deploy_test.go index 271ab4645d..592bf51d7e 100644 --- a/pkg/container/docker/client_deploy_test.go +++ b/pkg/container/docker/client_deploy_test.go @@ -7,7 +7,7 @@ import ( "context" "testing" - "github.com/docker/docker/api/types/network" + "github.com/moby/moby/api/types/network" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/container/docker/client_info_test.go b/pkg/container/docker/client_info_test.go index 2a52503528..8f5eb331f5 100644 --- a/pkg/container/docker/client_info_test.go +++ b/pkg/container/docker/client_info_test.go @@ -5,11 +5,13 @@ package docker import ( "context" + "net/netip" "testing" "time" - "github.com/docker/docker/api/types/container" - "github.com/docker/go-connections/nat" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" + mobyclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -24,7 +26,7 @@ func TestGetWorkloadInfo_MapsInspectResponseToDomain(t *testing.T) { call := 0 api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { call++ if call == 1 { // First call: find by base name label -> return empty to force fallback @@ -42,20 +44,18 @@ func TestGetWorkloadInfo_MapsInspectResponseToDomain(t *testing.T) { }, inspectFunc: func(_ context.Context, id string) (container.InspectResponse, error) { require.Equal(t, "cid-123", id) - p8080, err := nat.NewPort("tcp", "8080") + p8080, err := network.ParsePort("8080") require.NoError(t, err) ns := &container.NetworkSettings{} - ns.Ports = nat.PortMap{ - p8080: []nat.PortBinding{{HostIP: "127.0.0.1", HostPort: "18080"}}, + ns.Ports = network.PortMap{ + p8080: []network.PortBinding{{HostIP: netip.MustParseAddr("127.0.0.1"), HostPort: "18080"}}, } return container.InspectResponse{ - ContainerJSONBase: &container.ContainerJSONBase{ - Name: "/mcp", - Created: createdStr, - State: &container.State{Status: "running", Running: true}, - }, + Name: "/mcp", + Created: createdStr, + State: &container.State{Status: "running", Running: true}, Config: &container.Config{ Image: "ghcr.io/example/mcp:latest", Labels: map[string]string{"toolhive": "true", "k": "v"}, @@ -86,7 +86,7 @@ func TestIsWorkloadRunning_TrueWhenDockerReportsRunning(t *testing.T) { call := 0 api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { call++ if call == 1 { // First call: base name lookup -> not found @@ -106,13 +106,11 @@ func TestIsWorkloadRunning_TrueWhenDockerReportsRunning(t *testing.T) { require.Equal(t, "cid-xyz", id) ns := &container.NetworkSettings{} - ns.Ports = nat.PortMap{} + ns.Ports = network.PortMap{} return container.InspectResponse{ - ContainerJSONBase: &container.ContainerJSONBase{ - Name: "/server", - State: &container.State{Status: "running", Running: true}, - }, + Name: "/server", + State: &container.State{Status: "running", Running: true}, Config: &container.Config{ Image: "img", }, @@ -134,7 +132,7 @@ func TestGetWorkloadInfo_PortParseAndCreatedFallback(t *testing.T) { call := 0 api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { call++ if call == 1 { // First attempt (label-based) -> none found @@ -154,19 +152,17 @@ func TestGetWorkloadInfo_PortParseAndCreatedFallback(t *testing.T) { require.Equal(t, "cid-badfields", id) // Non-numeric host port; GetWorkloadInfo should log a warning and fall back to 0 - p8080, err := nat.NewPort("tcp", "8080") + p8080, err := network.ParsePort("8080") require.NoError(t, err) ns := &container.NetworkSettings{} - ns.Ports = nat.PortMap{ - p8080: []nat.PortBinding{{HostIP: "127.0.0.1", HostPort: "abc"}}, + ns.Ports = network.PortMap{ + p8080: []network.PortBinding{{HostIP: netip.MustParseAddr("127.0.0.1"), HostPort: "abc"}}, } return container.InspectResponse{ - ContainerJSONBase: &container.ContainerJSONBase{ - Name: "/svc-bad", - State: &container.State{Status: "exited", Running: false}, - Created: "not-a-time", // invalid RFC3339 -> Created should be zero time - }, + Name: "/svc-bad", + State: &container.State{Status: "exited", Running: false}, + Created: "not-a-time", // invalid RFC3339 -> Created should be zero time Config: &container.Config{Image: "img", Labels: map[string]string{"toolhive": "true"}}, NetworkSettings: ns, }, nil diff --git a/pkg/container/docker/client_list_test.go b/pkg/container/docker/client_list_test.go index ecc5da869e..1cff90e9cb 100644 --- a/pkg/container/docker/client_list_test.go +++ b/pkg/container/docker/client_list_test.go @@ -8,7 +8,8 @@ import ( "testing" "time" - "github.com/docker/docker/api/types/container" + "github.com/moby/moby/api/types/container" + mobyclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -21,7 +22,7 @@ func TestListWorkloads_FiltersAuxiliaryAndMapsFields(t *testing.T) { created := time.Now().Add(-1 * time.Hour).Unix() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { return []container.Summary{ { ID: "aux1", @@ -30,7 +31,7 @@ func TestListWorkloads_FiltersAuxiliaryAndMapsFields(t *testing.T) { State: "running", Names: []string{"/aux-name"}, Labels: map[string]string{ToolhiveAuxiliaryWorkloadLabel: LabelValueTrue, "toolhive": "true"}, - Ports: []container.Port{{PrivatePort: 3128, PublicPort: 0, Type: "tcp"}}, + Ports: []container.PortSummary{{PrivatePort: 3128, PublicPort: 0, Type: "tcp"}}, Created: created, }, { @@ -40,7 +41,7 @@ func TestListWorkloads_FiltersAuxiliaryAndMapsFields(t *testing.T) { State: "running", Names: []string{"/mcp-name"}, Labels: map[string]string{"toolhive": "true", "custom": "x"}, - Ports: []container.Port{{PrivatePort: 8080, PublicPort: 18080, Type: "tcp"}}, + Ports: []container.PortSummary{{PrivatePort: 8080, PublicPort: 18080, Type: "tcp"}}, Created: created, }, }, nil diff --git a/pkg/container/docker/client_partial_match_test.go b/pkg/container/docker/client_partial_match_test.go index e3473a4987..2c9c26b453 100644 --- a/pkg/container/docker/client_partial_match_test.go +++ b/pkg/container/docker/client_partial_match_test.go @@ -7,7 +7,8 @@ import ( "context" "testing" - "github.com/docker/docker/api/types/container" + "github.com/moby/moby/api/types/container" + mobyclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -94,9 +95,9 @@ func TestFindExistingContainer_RejectsPartialMatches(t *testing.T) { t.Parallel() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, opts container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, opts mobyclient.ContainerListOptions) ([]container.Summary, error) { // Verify that the name filter is being used - nameFilters := opts.Filters.Get("name") + nameFilters := filterValues(opts.Filters, "name") if len(nameFilters) > 0 { assert.Equal(t, tt.searchName, nameFilters[0], "Expected name filter to be set correctly") } @@ -214,10 +215,10 @@ func TestFindContainerByExactName_RejectsPartialMatches(t *testing.T) { t.Parallel() api := &fakeDockerAPI{ - listFunc: func(_ context.Context, opts container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, opts mobyclient.ContainerListOptions) ([]container.Summary, error) { // Verify that both toolhive label filter and name filter are being used - toolhiveFilter := opts.Filters.Get("label") - nameFilter := opts.Filters.Get("name") + toolhiveFilter := filterValues(opts.Filters, "label") + nameFilter := filterValues(opts.Filters, "name") assert.Contains(t, toolhiveFilter, "toolhive=true", "Expected toolhive label filter") assert.Contains(t, nameFilter, tt.searchName, "Expected name filter to be set") @@ -258,7 +259,7 @@ func TestPartialMatchingPrevention_IntegrationScenarios(t *testing.T) { } api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { // Simulate that container runtime returned all containers due to partial matching return mockContainers, nil }, @@ -281,3 +282,14 @@ func TestPartialMatchingPrevention_IntegrationScenarios(t *testing.T) { require.NoError(t, err) assert.Empty(t, containerID, "Should not find anything for non-existent exact name") } + +// filterValues returns the set of values associated with a filter term in the +// redesigned client.Filters (map[string]map[string]bool), giving tests the same +// view the legacy filters.Args.Get helper used to provide. +func filterValues(f mobyclient.Filters, term string) []string { + values := make([]string, 0, len(f[term])) + for value := range f[term] { + values = append(values, value) + } + return values +} diff --git a/pkg/container/docker/client_stop_test.go b/pkg/container/docker/client_stop_test.go index 8e58780a33..41a42dd46b 100644 --- a/pkg/container/docker/client_stop_test.go +++ b/pkg/container/docker/client_stop_test.go @@ -8,8 +8,9 @@ import ( "testing" "github.com/containerd/errdefs" - "github.com/docker/docker/api/types/container" - "github.com/docker/go-connections/nat" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" + mobyclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -20,7 +21,7 @@ func TestStopWorkload_NotRunning_ReturnsNil(t *testing.T) { // Arrange: find by exact name and inspect -> not running call := 0 api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { call++ if call == 1 { // First call: base-name label lookup -> none found @@ -40,19 +41,17 @@ func TestStopWorkload_NotRunning_ReturnsNil(t *testing.T) { require.Equal(t, "cid-not-running", id) // Not running ns := &container.NetworkSettings{} - ns.Ports = nat.PortMap{} + ns.Ports = network.PortMap{} return container.InspectResponse{ - ContainerJSONBase: &container.ContainerJSONBase{ - Name: "/svc", - State: &container.State{Status: "exited", Running: false}, - }, + Name: "/svc", + State: &container.State{Status: "exited", Running: false}, Config: &container.Config{Image: "img", Labels: map[string]string{"toolhive": "true"}}, NetworkSettings: ns, ImageManifestDescriptor: nil, }, nil }, // stopFunc should not be called - stopFunc: func(_ context.Context, _ string, _ container.StopOptions) error { + stopFunc: func(_ context.Context, _ string, _ mobyclient.ContainerStopOptions) error { t.Fatalf("ContainerStop should not be called for not-running container") return nil }, @@ -72,7 +71,7 @@ func TestStopWorkload_Running_CallsContainerStop(t *testing.T) { called := false call := 0 api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { call++ if call == 1 { return []container.Summary{}, nil @@ -89,17 +88,15 @@ func TestStopWorkload_Running_CallsContainerStop(t *testing.T) { inspectFunc: func(_ context.Context, id string) (container.InspectResponse, error) { require.Equal(t, "cid-running", id) ns := &container.NetworkSettings{} - ns.Ports = nat.PortMap{} + ns.Ports = network.PortMap{} return container.InspectResponse{ - ContainerJSONBase: &container.ContainerJSONBase{ - Name: "/app", - State: &container.State{Status: "running", Running: true}, - }, + Name: "/app", + State: &container.State{Status: "running", Running: true}, Config: &container.Config{Image: "img", Labels: map[string]string{"toolhive": "true"}}, NetworkSettings: ns, }, nil }, - stopFunc: func(_ context.Context, id string, _ container.StopOptions) error { + stopFunc: func(_ context.Context, id string, _ mobyclient.ContainerStopOptions) error { // The implementation stops by workloadName (not ID), verify that assert.Equal(t, "app", id) called = true @@ -118,7 +115,7 @@ func TestStopWorkload_NotFound_ReturnsNil(t *testing.T) { // Simulate a case where a container appears in listing, but inspect returns NotFound api := &fakeDockerAPI{ - listFunc: func(_ context.Context, _ container.ListOptions) ([]container.Summary, error) { + listFunc: func(_ context.Context, _ mobyclient.ContainerListOptions) ([]container.Summary, error) { // Exact name lookup will find a candidate return []container.Summary{ { diff --git a/pkg/container/docker/mocks_test.go b/pkg/container/docker/mocks_test.go index 798c7c2b09..9ca117be99 100644 --- a/pkg/container/docker/mocks_test.go +++ b/pkg/container/docker/mocks_test.go @@ -7,64 +7,91 @@ import ( "context" "fmt" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/network" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" + mobyclient "github.com/moby/moby/client" v1 "github.com/opencontainers/image-spec/specs-go/v1" ) // fakeDockerAPI provides a minimal test double for dockerAPI used by Client. -// Centralized here for reuse across tests. +// Centralized here for reuse across tests. The hook signatures mirror the +// redesigned moby/moby client API that Client.api targets. type fakeDockerAPI struct { - listFunc func(ctx context.Context, options container.ListOptions) ([]container.Summary, error) + listFunc func(ctx context.Context, options mobyclient.ContainerListOptions) ([]container.Summary, error) inspectFunc func(ctx context.Context, id string) (container.InspectResponse, error) - stopFunc func(ctx context.Context, containerID string, options container.StopOptions) error + stopFunc func(ctx context.Context, containerID string, options mobyclient.ContainerStopOptions) error // additional hooks to satisfy extended dockerAPI for create/start/remove createFunc func(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *v1.Platform, containerName string) (container.CreateResponse, error) - startFunc func(ctx context.Context, containerID string, options container.StartOptions) error - removeFunc func(ctx context.Context, containerID string, options container.RemoveOptions) error + startFunc func(ctx context.Context, containerID string, options mobyclient.ContainerStartOptions) error + removeFunc func(ctx context.Context, containerID string, options mobyclient.ContainerRemoveOptions) error } -func (f *fakeDockerAPI) ContainerList(ctx context.Context, options container.ListOptions) ([]container.Summary, error) { +func (f *fakeDockerAPI) ContainerList( + ctx context.Context, + options mobyclient.ContainerListOptions, +) (mobyclient.ContainerListResult, error) { if f.listFunc != nil { - return f.listFunc(ctx, options) + items, err := f.listFunc(ctx, options) + return mobyclient.ContainerListResult{Items: items}, err } - return nil, nil + return mobyclient.ContainerListResult{}, nil } -func (f *fakeDockerAPI) ContainerInspect(ctx context.Context, id string) (container.InspectResponse, error) { +func (f *fakeDockerAPI) ContainerInspect( + ctx context.Context, + id string, + _ mobyclient.ContainerInspectOptions, +) (mobyclient.ContainerInspectResult, error) { if f.inspectFunc != nil { - return f.inspectFunc(ctx, id) + resp, err := f.inspectFunc(ctx, id) + return mobyclient.ContainerInspectResult{Container: resp}, err } - return container.InspectResponse{}, nil + return mobyclient.ContainerInspectResult{}, nil } -func (f *fakeDockerAPI) ContainerStop(ctx context.Context, containerID string, options container.StopOptions) error { +func (f *fakeDockerAPI) ContainerStop( + ctx context.Context, + containerID string, + options mobyclient.ContainerStopOptions, +) (mobyclient.ContainerStopResult, error) { if f.stopFunc != nil { - return f.stopFunc(ctx, containerID, options) + return mobyclient.ContainerStopResult{}, f.stopFunc(ctx, containerID, options) } - return nil + return mobyclient.ContainerStopResult{}, nil } -func (f *fakeDockerAPI) ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *v1.Platform, containerName string) (container.CreateResponse, error) { +func (f *fakeDockerAPI) ContainerCreate( + ctx context.Context, + options mobyclient.ContainerCreateOptions, +) (mobyclient.ContainerCreateResult, error) { if f.createFunc != nil { - return f.createFunc(ctx, config, hostConfig, networkingConfig, platform, containerName) + resp, err := f.createFunc(ctx, options.Config, options.HostConfig, options.NetworkingConfig, options.Platform, options.Name) + return mobyclient.ContainerCreateResult{ID: resp.ID, Warnings: resp.Warnings}, err } - return container.CreateResponse{}, nil + return mobyclient.ContainerCreateResult{}, nil } -func (f *fakeDockerAPI) ContainerStart(ctx context.Context, containerID string, options container.StartOptions) error { +func (f *fakeDockerAPI) ContainerStart( + ctx context.Context, + containerID string, + options mobyclient.ContainerStartOptions, +) (mobyclient.ContainerStartResult, error) { if f.startFunc != nil { - return f.startFunc(ctx, containerID, options) + return mobyclient.ContainerStartResult{}, f.startFunc(ctx, containerID, options) } - return nil + return mobyclient.ContainerStartResult{}, nil } -func (f *fakeDockerAPI) ContainerRemove(ctx context.Context, containerID string, options container.RemoveOptions) error { +func (f *fakeDockerAPI) ContainerRemove( + ctx context.Context, + containerID string, + options mobyclient.ContainerRemoveOptions, +) (mobyclient.ContainerRemoveResult, error) { if f.removeFunc != nil { - return f.removeFunc(ctx, containerID, options) + return mobyclient.ContainerRemoveResult{}, f.removeFunc(ctx, containerID, options) } - return nil + return mobyclient.ContainerRemoveResult{}, nil } // fakeImageManager provides a minimal test double for ImageManager diff --git a/pkg/container/docker/sdk/client_unix.go b/pkg/container/docker/sdk/client_unix.go index 2768869001..4a9cb56b0f 100644 --- a/pkg/container/docker/sdk/client_unix.go +++ b/pkg/container/docker/sdk/client_unix.go @@ -14,7 +14,7 @@ import ( "os" "path/filepath" - "github.com/docker/docker/client" + mobyclient "github.com/moby/moby/client" "github.com/stacklok/toolhive/pkg/container/runtime" ) @@ -23,7 +23,7 @@ import ( var ErrRuntimeNotFound = fmt.Errorf("container runtime not found") // newPlatformClient creates a Docker client using Unix sockets -func newPlatformClient(socketPath string) (*http.Client, []client.Opt) { +func newPlatformClient(socketPath string) (*http.Client, []mobyclient.Opt) { // Create a custom HTTP client that uses the Unix socket httpClient := &http.Client{ Transport: &http.Transport{ @@ -33,11 +33,12 @@ func newPlatformClient(socketPath string) (*http.Client, []client.Opt) { }, } - // Create Docker client options - opts := []client.Opt{ - client.WithAPIVersionNegotiation(), - client.WithHTTPClient(httpClient), - client.WithHost("unix://" + socketPath), + // Create Docker client options. API-version negotiation is enabled by + // default in the new client, so it no longer needs to be requested + // explicitly. + opts := []mobyclient.Opt{ + mobyclient.WithHTTPClient(httpClient), + mobyclient.WithHost("unix://" + socketPath), } return httpClient, opts diff --git a/pkg/container/docker/sdk/client_windows.go b/pkg/container/docker/sdk/client_windows.go index 851b1891ab..ca8b9692fc 100644 --- a/pkg/container/docker/sdk/client_windows.go +++ b/pkg/container/docker/sdk/client_windows.go @@ -15,7 +15,7 @@ import ( "time" "github.com/Microsoft/go-winio" - "github.com/docker/docker/client" + mobyclient "github.com/moby/moby/client" "github.com/stacklok/toolhive/pkg/container/runtime" ) @@ -35,7 +35,7 @@ const ( const pipeConnectionTimeout = 2 * time.Second // newPlatformClient creates a Docker client using Windows named pipes -func newPlatformClient(pipePath string) (*http.Client, []client.Opt) { +func newPlatformClient(pipePath string) (*http.Client, []mobyclient.Opt) { // Create a custom HTTP client that uses Windows named pipes httpClient := &http.Client{ Transport: &http.Transport{ @@ -48,11 +48,12 @@ func newPlatformClient(pipePath string) (*http.Client, []client.Opt) { }, } - // Create Docker client options - opts := []client.Opt{ - client.WithAPIVersionNegotiation(), - client.WithHTTPClient(httpClient), - client.WithHost("npipe://" + pipePath), + // Create Docker client options. API-version negotiation is enabled by + // default in the new client, so it no longer needs to be requested + // explicitly. + opts := []mobyclient.Opt{ + mobyclient.WithHTTPClient(httpClient), + mobyclient.WithHost("npipe://" + pipePath), } return httpClient, opts diff --git a/pkg/container/docker/sdk/factory.go b/pkg/container/docker/sdk/factory.go index bb56886e42..8d19c01dc2 100644 --- a/pkg/container/docker/sdk/factory.go +++ b/pkg/container/docker/sdk/factory.go @@ -12,7 +12,7 @@ import ( "log/slog" "strings" - "github.com/docker/docker/client" + mobyclient "github.com/moby/moby/client" "github.com/stacklok/toolhive/pkg/container/runtime" ) @@ -66,7 +66,7 @@ var supportedSocketPaths = []runtime.Type{runtime.TypePodman, runtime.TypeDocker // per-runtime fallback the first stat-OK socket short-circuits discovery and // we'd surface "no container runtime available" even though a usable Docker // daemon is reachable through a different socket. -func NewDockerClient(ctx context.Context) (*client.Client, string, runtime.Type, error) { +func NewDockerClient(ctx context.Context) (*mobyclient.Client, string, runtime.Type, error) { var lastErr error for _, sp := range supportedSocketPaths { @@ -120,18 +120,18 @@ func dockerPermissionHint(err error) string { } // NewClientWithSocketPath creates a new container client with a specific socket path -func newClientWithSocketPath(ctx context.Context, socketPath string) (*client.Client, error) { +func newClientWithSocketPath(ctx context.Context, socketPath string) (*mobyclient.Client, error) { // Create platform-specific client _, opts := newPlatformClient(socketPath) // Create Docker client with the custom HTTP client - dockerClient, err := client.NewClientWithOpts(opts...) + dockerClient, err := mobyclient.New(opts...) if err != nil { return nil, fmt.Errorf("failed to create client: %w", err) } // Make sure we can ping the server. - _, err = dockerClient.Ping(ctx) + _, err = dockerClient.Ping(ctx, mobyclient.PingOptions{}) if err != nil { return nil, fmt.Errorf("failed to ping Docker server at %s: %w", socketPath, err) } diff --git a/pkg/container/docker/squid.go b/pkg/container/docker/squid.go index 1d53143947..c23825e46b 100644 --- a/pkg/container/docker/squid.go +++ b/pkg/container/docker/squid.go @@ -11,8 +11,8 @@ import ( "strconv" "strings" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/network" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" "github.com/stacklok/toolhive-core/permissions" "github.com/stacklok/toolhive/pkg/container/runtime" diff --git a/pkg/container/docker/squid_test.go b/pkg/container/docker/squid_test.go index 984b61c0cf..341f630230 100644 --- a/pkg/container/docker/squid_test.go +++ b/pkg/container/docker/squid_test.go @@ -10,9 +10,10 @@ import ( "strings" "testing" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/mount" - "github.com/docker/docker/api/types/network" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/mount" + "github.com/moby/moby/api/types/network" + mobyclient "github.com/moby/moby/client" v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -37,7 +38,7 @@ func TestCreateSquidContainer_Basics(t *testing.T) { gotHost = host return container.CreateResponse{ID: "cid-new"}, nil }, - startFunc: func(_ context.Context, id string, _ container.StartOptions) error { + startFunc: func(_ context.Context, id string, _ mobyclient.ContainerStartOptions) error { startCalled = true assert.Equal(t, "cid-new", id) return nil diff --git a/pkg/container/images/registry.go b/pkg/container/images/registry.go index d00631200e..1080c790e5 100644 --- a/pkg/container/images/registry.go +++ b/pkg/container/images/registry.go @@ -15,8 +15,6 @@ import ( "path/filepath" "runtime" - "github.com/docker/docker/api/types/build" - "github.com/docker/docker/client" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -29,31 +27,20 @@ import ( // for direct registry operations without requiring a Docker daemon. // However, for building images from Dockerfiles, it still uses the Docker client. type RegistryImageManager struct { - keychain authn.Keychain - platform *v1.Platform - dockerClient *client.Client // Used for building images from Dockerfiles - daemonClient daemon.Client // Used for daemon.Image/daemon.Write (go-containerregistry) + keychain authn.Keychain + platform *v1.Platform + // dockerClient is used for building images from Dockerfiles and, because the + // moby/moby client already satisfies the go-containerregistry daemon.Client + // interface, for daemon.Image/daemon.Write operations as well. + dockerClient *mobyclient.Client } // NewRegistryImageManager creates a new RegistryImageManager instance -func NewRegistryImageManager(dockerClient *client.Client) *RegistryImageManager { - // Create a moby/moby client that satisfies the daemon.Client interface - // required by go-containerregistry, using the same host and HTTP client - // as the docker client. - mobyClient, err := mobyclient.New( - mobyclient.WithHost(dockerClient.DaemonHost()), - mobyclient.WithHTTPClient(dockerClient.HTTPClient()), - ) - if err != nil { - // Fall back: this should not happen since we already have a working docker client - slog.Warn("failed to create moby client for daemon operations, daemon image operations may fail", "error", err) - } - +func NewRegistryImageManager(dockerClient *mobyclient.Client) *RegistryImageManager { return &RegistryImageManager{ keychain: NewCompositeKeychain(), // Use composite keychain (env vars + default) platform: getDefaultPlatform(), // Use a default platform based on host architecture - dockerClient: dockerClient, // Used solely for building images from Dockerfiles - daemonClient: mobyClient, // Used for go-containerregistry daemon operations + dockerClient: dockerClient, } } @@ -75,7 +62,7 @@ func (r *RegistryImageManager) ImageExists(_ context.Context, imageName string) } // First check if image exists locally in daemon - if _, err := daemon.Image(ref, daemon.WithClient(r.daemonClient)); err != nil { + if _, err := daemon.Image(ref, daemon.WithClient(r.dockerClient)); err != nil { // Image does not exist locally return false, nil } @@ -121,7 +108,7 @@ func (r *RegistryImageManager) PullImage(ctx context.Context, imageName string) } // Save the image to the local daemon - response, err := daemon.Write(tag, img, daemon.WithClient(r.daemonClient)) + response, err := daemon.Write(tag, img, daemon.WithClient(r.dockerClient)) if err != nil { return fmt.Errorf("failed to write image to daemon: %w", err) } @@ -154,7 +141,7 @@ func (r *RegistryImageManager) WithPlatform(platform *v1.Platform) *RegistryImag } // buildDockerImage builds a Docker image using the Docker client API -func buildDockerImage(ctx context.Context, dockerClient *client.Client, contextDir, imageName string) error { +func buildDockerImage(ctx context.Context, dockerClient *mobyclient.Client, contextDir, imageName string) error { //nolint:gosec // G706: image name and context dir from config slog.Debug("building image", "image", imageName, "context_dir", contextDir) @@ -192,7 +179,7 @@ func buildDockerImage(ctx context.Context, dockerClient *client.Client, contextD } // Build the image - buildOptions := build.ImageBuildOptions{ + buildOptions := mobyclient.ImageBuildOptions{ Tags: []string{imageName}, Dockerfile: "Dockerfile", Remove: true,