Classify invalid docker DNS/MAC address as ConfigurationError
Why
Follow-up from review discussion on
!6902 (merged)
(migrate Docker SDK to moby/moby v29). That MR introduced
dnsServerAddrs()/parseMACAddress(), which now reject invalid
DNS/MAC entries in [runners.docker] config at container-create
time instead of silently forwarding them to the daemon. Axel raised
that this is effectively a breaking change for anyone relying on the
old permissive behavior, and asked that it at least be documented
for support:
!6902 (comment 2637081234)
Digging into it further: today these errors bubble up unwrapped and
fall through to the default RunnerSystemFailure classification,
even though this is entirely a user configuration mistake (the value
lives in the runner's own config.toml, set by whoever administers
the runner) that only the runner can ever detect. ConfigurationError
exists specifically for this case, and the same file already uses it
for other invalid user-supplied docker config values (an invalid
service image name, an unparseable volumes entry).
What this does
Wraps the errors from dnsServerAddrs()/parseMACAddress() in
&common.BuildError{FailureReason: common.ConfigurationError} at
their three call sites in executors/docker/docker.go
(createHostConfigForService, networkConfig, createHostConfig),
and adds unit tests asserting the failure reason.
Why this is a separate MR, targeting main, in draft
dnsServerAddrs()/parseMACAddress() don't exist on main yet —
they're introduced by !6902 (merged). This branch is stacked on top of
!6902 (merged)'s branch so it can be built/tested/reviewed independently, but
targets main since that's where it ultimately belongs once !6902 (merged)
merges. It's marked Draft because the diff will show all of !6902 (merged)'s
commits until that MR merges and this one is rebased onto main.
Verification
go build ./executors/docker/...go test ./executors/docker/...(all packages pass)make lint- Manually confirmed via
run-singleagainst real Docker daemons (Rancher Desktop, Docker Desktop) that invalid DNS/MAC still fail fast with the same error message, just now classified correctly.