fix(docker): guard nil NetworkSettings in Executor network methods - #61
Merged
Conversation
InspectResponse.NetworkSettings is a pointer and the daemon leaves it nil for containers without networking (--network=none, and some podman responses). MappedPort, GetAllPorts, GetNetworks and GetIPAddress dereferenced it unconditionally and panicked instead of returning an error. ContainerTarget already tolerates this (target.go State); these four methods did not. Move the projection out of the client calls into pure functions of an inspect response, which is what makes the nil case unit-testable without a live daemon, and guard there. GetAllPorts and GetNetworks now return an empty map/slice rather than panicking, since callers range over the result; MappedPort and GetIPAddress return their existing not-found errors. Also stop dereferencing nil EndpointSettings values inside the Networks map.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
container.InspectResponse.NetworkSettingsis a pointer, and the daemon leaves itnilfor containers without networking (--network=none, and some podman responses).Four exported
Executormethods dereferenced it unconditionally and panicked:MappedPortnetwork.go:86GetAllPortsnetwork.go:147GetNetworksnetwork.go:175GetIPAddressnetwork.go:205,212ContainerTarget.State(target.go:87) already guards this exact field — these four were the gap. Recorded as "docker NetworkSettings parity" indocs/plans/2026-07-22-v3-audit-backlog.md.Change
The projection logic was entangled with the client call, which is why the nil case had never been tested — it needs a live daemon to reach. Extracted four pure functions of an inspect response (
portBinding,allPortBindings,networkNames,networkIPAddress) and guarded there, so the nil path is unit-testable without Docker.Behaviour:
GetAllPorts/GetNetworksreturn an empty map/slice instead of panicking — callers range over the result, so nil would be a second footgun.MappedPort/GetIPAddressreturn their existing not-found errors unchanged.nilEndpointSettingsvalues inside theNetworksmap, which was a second latent panic.No public signature changes.
Tests (written first, observed failing)
New
docker/network_test.go— the first run failed to compile (undefined: portBinding, …), driving the extraction. 17 cases covering bound/unbound/missing ports, named vs first-available network lookup, nilNetworkSettings, and nilEndpointSettings.These raise docker's unit coverage without needing a daemon, which matters since the package sat at 41% precisely because most paths require one.
Verification
go test ./docker/passes;golangci-lint run ./docker/...0 issues. Fulltask ci:checkgreen on this change.