Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unix transport used when HTTP host is specified #99

Closed
jamesmoessis opened this issue Jul 19, 2022 · 2 comments · Fixed by #103
Closed

Unix transport used when HTTP host is specified #99

jamesmoessis opened this issue Jul 19, 2022 · 2 comments · Fixed by #103

Comments

@jamesmoessis
Copy link

Here is a demo repo that demonstrates the bug. You can toggle on/off the replace in the go.mod in order to switch the bug on and off. The example uses the docker client. This problem doesn't exist in 0.4.0.

There's a problem in master currently where it overrides the proto and doesn't reset it when a new scheme is applied. This had the effect of making request to a unix socket even though an http endpoint was specified for the docker engine.

A debugging session indicated the problem was from a removal of piece of code in sockets.go, which was changed here: https://github.com/docker/go-connections/pull/61/files#r923998601

The issue seems possibly related to the following which also had trouble making TCP connections:

@nicks
Copy link

nicks commented Oct 27, 2023

I can repro this as well -

Repro Steps

  1. Create the following test case:
package main

import (
	"context"
	"log"

	"github.com/docker/docker/client"
)

func main() {
	cli, _ := client.NewClientWithOpts(
		client.WithHost("tcp://localhost:5000"))

	info, err := cli.Info(context.Background())
	log.Printf("INFO %s %v\n", info.ServerVersion, err)
}
  1. Use the following go.mod
module github.com/docker/bug

go 1.21.3

require github.com/docker/docker v24.0.7+incompatible

require (
	github.com/Microsoft/go-winio v0.6.1 // indirect
	github.com/distribution/reference v0.5.0 // indirect
	github.com/docker/distribution v2.8.3+incompatible // indirect
	github.com/docker/go-connections v0.4.1-0.20210727194412-58542c764a11 // indirect
	github.com/docker/go-units v0.5.0 // indirect
	github.com/gogo/protobuf v1.3.2 // indirect
	github.com/moby/term v0.5.0 // indirect
	github.com/morikuni/aec v1.0.0 // indirect
	github.com/opencontainers/go-digest v1.0.0 // indirect
	github.com/opencontainers/image-spec v1.0.2 // indirect
	github.com/pkg/errors v0.9.1 // indirect
	golang.org/x/mod v0.8.0 // indirect
	golang.org/x/sys v0.5.0 // indirect
	golang.org/x/time v0.3.0 // indirect
	golang.org/x/tools v0.6.0 // indirect
	gotest.tools/v3 v3.5.1 // indirect
)
  1. Run go run main.go

Expected results:
The Docker API client will send a request to localhost:5000, or fail if docker isn't running on that port.

Actual results:
The Docker API client will send a request to unix://var/run/docker.sock

the problem goes away if you change the go.mod to go-connections v0.4.0

@nicks
Copy link

nicks commented Oct 27, 2023

I think the fix is to update the TCP codepath to set DialContext to nil and DisableCompression to false -- effectively to "undo" http.Transport changes made by the Unix socket and npipe codepaths.

But I also don't 100% understand the contract of this function or how it's used

andrzej-stencel added a commit to andrzej-stencel/opentelemetry-collector-releases that referenced this issue Sep 25, 2024
…nnections` package

The package `github.com/docker/go-connections` only exists in contrib's generated `go.mod` file
in version `v0.5.0`, and does not exist in the replaced version `v0.4.1`.
The distro also builds and runs fine without the replace directive.

I believe the [reason](open-telemetry/opentelemetry-collector-contrib#12322 (comment))
for the replace directive to have been added is out of date.
To be sure, I have run the repro scenario described [here](docker/go-connections#99 (comment))
and the behavior with go-connections v0.5.0 was correct - the Docker client tried to connect to the TCP port.
codeboten added a commit to open-telemetry/opentelemetry-collector-releases that referenced this issue Oct 2, 2024
…nnections` package (#677)

The package `github.com/docker/go-connections` only exists in contrib's generated `go.mod` file
in version `v0.5.0`, and does not exist in the replaced version `v0.4.1`.
The distro also builds and runs fine without the replace directive.

I believe the [reason](open-telemetry/opentelemetry-collector-contrib#12322 (comment))
for the replace directive to have been added is out of date.
To be sure, I have run the repro scenario described [here](docker/go-connections#99 (comment))
and the behavior with go-connections v0.5.0 was correct - the Docker client tried to connect to the TCP port.

Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants