Skip to content

Commit c22a08e

Browse files
committed
fix(docker): address review feedback for #605
- Extract resolveDockerHost() helper so the config -> env -> default precedence is documented and independently testable, instead of inlined inside createHTTPClient. - Add TestResolveDockerHost to pin the precedence contract. - Extend TestCreateHTTPClient_DockerHostEnvVar with an HTTPS case that also asserts ForceAttemptHTTP2 is opt-in for https:// hosts. - Add a CHANGELOG entry under [Unreleased]. - Document the failure mode and the recommended docker-socket-proxy configuration in docs/TROUBLESHOOTING.md. Refs #605, #606. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
1 parent 7f5a333 commit c22a08e

4 files changed

Lines changed: 124 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- `DOCKER_HOST` is now honored when selecting the HTTP transport dialer. Previously, with `DOCKER_HOST=tcp://...` (e.g. a Docker socket proxy) the client silently dialed `unix:///var/run/docker.sock` and startup failed with a misleading "Cannot connect to the Docker daemon at tcp://…" error ([#606](https://github.com/netresearch/ofelia/pull/606), fixes [#605](https://github.com/netresearch/ofelia/issues/605))
13+
1014
## [0.24.0] - 2026-05-10
1115

1216
### Changed

core/adapters/docker/client.go

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -124,23 +124,28 @@ func newClientFromSDK(sdk *client.Client) *Client {
124124
return c
125125
}
126126

127+
// resolveDockerHost mirrors the SDK's client.FromEnv host resolution so the
128+
// custom HTTP transport's dialer ends up targeting the same endpoint as the
129+
// SDK. Precedence: explicit config > DOCKER_HOST > client.DefaultDockerHost.
130+
// Without this, an empty config.Host plus DOCKER_HOST=tcp://... silently
131+
// routes every request through a unix socket dialer pinned to
132+
// /var/run/docker.sock. See issue #605.
133+
func resolveDockerHost(configHost string) string {
134+
if configHost != "" {
135+
return configHost
136+
}
137+
if envHost := os.Getenv("DOCKER_HOST"); envHost != "" {
138+
return envHost
139+
}
140+
return client.DefaultDockerHost
141+
}
142+
127143
// createHTTPClient creates an HTTP client with connection pooling.
128144
func createHTTPClient(config *ClientConfig) *http.Client {
129145
// Determine if we should use HTTP/2
130146
// Docker daemon only supports HTTP/2 over TLS (ALPN negotiation)
131147
// For Unix sockets and plain TCP, we use HTTP/1.1
132-
//
133-
// Host resolution must mirror what the SDK's client.FromEnv option does
134-
// (read DOCKER_HOST when not set explicitly); otherwise the dialer here
135-
// can fall back to a unix socket while the SDK is trying to reach a TCP
136-
// endpoint such as a Docker socket proxy. See issue #605.
137-
host := config.Host
138-
if host == "" {
139-
host = os.Getenv("DOCKER_HOST")
140-
}
141-
if host == "" {
142-
host = client.DefaultDockerHost
143-
}
148+
host := resolveDockerHost(config.Host)
144149

145150
transport := &http.Transport{
146151
MaxIdleConns: config.MaxIdleConns,

core/adapters/docker/client_mutation_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,33 +158,50 @@ func TestCreateHTTPClient_HostConditions(t *testing.T) {
158158
// Previously the dialer always fell back to a Unix socket, breaking TCP setups
159159
// (e.g. Docker socket proxy) even though the SDK itself read DOCKER_HOST via
160160
// client.FromEnv.
161+
//
162+
// Invariant under test: only the unix:// branch of createHTTPClient installs
163+
// a transport.DialContext closure. For TCP/HTTPS we rely on net.Dial via the
164+
// standard transport, so DialContext==nil is the signal that "this transport
165+
// will route to the URL's host" rather than to a hard-coded unix socket.
161166
func TestCreateHTTPClient_DockerHostEnvVar(t *testing.T) {
162167
testCases := []struct {
163168
name string
164169
configHost string
165170
envHost string
166171
wantUnixDialer bool
172+
wantForceHTTP2 bool
167173
desc string
168174
}{
169175
{
170176
name: "tcp_via_env_with_empty_config",
171177
configHost: "",
172178
envHost: "tcp://docker-proxy:2375",
173179
wantUnixDialer: false,
180+
wantForceHTTP2: false,
174181
desc: "DOCKER_HOST=tcp://... must produce a TCP-capable transport (no unix dialer)",
175182
},
176183
{
177184
name: "unix_via_env_with_empty_config",
178185
configHost: "",
179186
envHost: "unix:///custom/docker.sock",
180187
wantUnixDialer: true,
188+
wantForceHTTP2: false,
181189
desc: "DOCKER_HOST=unix://... must produce a unix dialer",
182190
},
191+
{
192+
name: "https_via_env_with_empty_config",
193+
configHost: "",
194+
envHost: "https://docker.example.com:2376",
195+
wantUnixDialer: false,
196+
wantForceHTTP2: true,
197+
desc: "DOCKER_HOST=https://... must opt into HTTP/2 and use no unix dialer",
198+
},
183199
{
184200
name: "config_host_wins_over_env",
185201
configHost: "tcp://from-config:2375",
186202
envHost: "unix:///should-be-ignored.sock",
187203
wantUnixDialer: false,
204+
wantForceHTTP2: false,
188205
desc: "Explicit config.Host must take precedence over DOCKER_HOST",
189206
},
190207
}
@@ -214,10 +231,37 @@ func TestCreateHTTPClient_DockerHostEnvVar(t *testing.T) {
214231
if !tc.wantUnixDialer && hasDialer {
215232
t.Errorf("%s: expected no unix dialer (DialContext nil) but it was set", tc.desc)
216233
}
234+
if transport.ForceAttemptHTTP2 != tc.wantForceHTTP2 {
235+
t.Errorf("%s: ForceAttemptHTTP2 = %v, want %v", tc.desc, transport.ForceAttemptHTTP2, tc.wantForceHTTP2)
236+
}
217237
})
218238
}
219239
}
220240

241+
// TestResolveDockerHost pins the precedence contract: explicit config wins,
242+
// otherwise DOCKER_HOST, otherwise the SDK default. This is the contract the
243+
// createHTTPClient dialer relies on staying aligned with client.FromEnv.
244+
func TestResolveDockerHost(t *testing.T) {
245+
t.Run("config_wins", func(t *testing.T) {
246+
t.Setenv("DOCKER_HOST", "unix:///ignored.sock")
247+
if got := resolveDockerHost("tcp://explicit:2375"); got != "tcp://explicit:2375" {
248+
t.Errorf("got %q, want explicit config value", got)
249+
}
250+
})
251+
t.Run("env_when_config_empty", func(t *testing.T) {
252+
t.Setenv("DOCKER_HOST", "tcp://from-env:2375")
253+
if got := resolveDockerHost(""); got != "tcp://from-env:2375" {
254+
t.Errorf("got %q, want DOCKER_HOST value", got)
255+
}
256+
})
257+
t.Run("default_when_both_empty", func(t *testing.T) {
258+
t.Setenv("DOCKER_HOST", "")
259+
if got := resolveDockerHost(""); got == "" {
260+
t.Error("resolveDockerHost returned empty when no config and no env — expected SDK default")
261+
}
262+
})
263+
}
264+
221265
func TestClientConfig_PoolingOptions(t *testing.T) {
222266
// Test various connection pooling configurations
223267
testCases := []struct {

docs/TROUBLESHOOTING.md

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,65 @@ docker exec ofelia id
181181
- TCP socket should only bind to localhost, not 0.0.0.0
182182
- Consider the security implications before disabling userns-remap in production
183183

184+
### DOCKER_HOST with TCP Socket Proxy (v0.12.0 – v0.24.0)
185+
186+
**Symptoms**:
187+
188+
```
189+
SDK provider failed to connect to Docker: pinging docker: Cannot connect to the Docker daemon at tcp://<host>:2375. Is the docker daemon running?
190+
```
191+
192+
The error names the configured `DOCKER_HOST`, but the connection never actually reaches it.
193+
194+
**Affected Versions**: v0.12.0 – v0.24.0 (fixed in v0.24.1+).
195+
196+
**Affected Configurations**:
197+
198+
- `DOCKER_HOST=tcp://…` pointing at a Docker socket proxy (e.g. [tecnativa/docker-socket-proxy](https://github.com/Tecnativa/docker-socket-proxy)).
199+
- `DOCKER_HOST=tcp://…` pointing at a remote Docker daemon over plain TCP.
200+
- Not affected: `DOCKER_HOST=unix://…` and the default `unix:///var/run/docker.sock`.
201+
202+
**Root Cause**:
203+
The custom HTTP transport built inside the Docker SDK adapter chose its dialer from `ClientConfig.Host` only, which was empty in the production path. It therefore fell back to a Unix-socket dialer pinned to `/var/run/docker.sock`, even though the SDK itself correctly read `DOCKER_HOST` and pointed at the TCP endpoint. Every request was silently routed to a non-existent Unix socket, producing the misleading `Cannot connect to the Docker daemon at tcp://…` error.
204+
205+
**Solutions**:
206+
207+
1. **Upgrade to v0.24.1+** (recommended).
208+
2. **Workaround for older versions**: bind-mount `/var/run/docker.sock` directly into the container instead of using a TCP socket proxy. This loses the proxy's security restrictions.
209+
210+
**Working example after the fix** (Docker Compose with `tecnativa/docker-socket-proxy`):
211+
212+
```yaml
213+
services:
214+
ofelia:
215+
image: ghcr.io/netresearch/ofelia:0.24.1
216+
environment:
217+
DOCKER_HOST: tcp://ofelia-socket-proxy:2375
218+
networks: [ofelia-socket-proxy]
219+
depends_on:
220+
ofelia-socket-proxy:
221+
condition: service_healthy
222+
223+
ofelia-socket-proxy:
224+
image: ghcr.io/tecnativa/docker-socket-proxy:v0.4.2
225+
networks: [ofelia-socket-proxy]
226+
volumes:
227+
- /var/run/docker.sock:/var/run/docker.sock:ro
228+
environment:
229+
CONTAINERS: 1
230+
POST: 1
231+
EXEC: 1
232+
healthcheck:
233+
test: wget -t1 -T4 -qO- http://127.0.0.1:2375/_ping | grep -q "OK" || exit 1
234+
235+
networks:
236+
ofelia-socket-proxy:
237+
```
238+
239+
**References**:
240+
- Issue: [#605](https://github.com/netresearch/ofelia/issues/605)
241+
- Fix: [#606](https://github.com/netresearch/ofelia/pull/606)
242+
184243
### HTTP/2 Protocol Errors (v0.11.0 Only)
185244
186245
**Symptoms**:

0 commit comments

Comments
 (0)