transport/http: use ipv4 explicitly for dialers if ipv6 is not supported #2549
Conversation
|
The conventionalcommits linter conflicts with https://github.com/googleapis/google-api-go-client/blob/main/CONTRIBUTING.md - which should I follow? |
|
I'll assume conventionalcommits for now. |
…upported This is a workaround for golang/go#25321, and is related to hashicorp/terraform-provider-google#6782
melinath
force-pushed
the
ipv4-only-resolution
branch
from
4b8b91e
to
ddec111
Compare
|
Related to hashicorp/terraform-provider-google#6782 |
| trans.DialContext = func(ctx context.Context, network string, addr string) (net.Conn, error) { | ||
| // Don't try IPv6 if it's not supported. | ||
| // https://github.com/golang/go/issues/25321 | ||
| if !nettest.SupportsIPv6() { |
There was a problem hiding this comment.
I'm a bit hesitant to use a package meant for testing in a production context. Is this what the Go team recommends? Is there any other package we can use that is meant for critical path usage?
There was a problem hiding this comment.
I think this package would be fine to use as it is meant to test the network and not necessary unit tests. This does feel like code that should happen in terraform though maybe and not here. I think the reason for putting it here is that our transport function is not handling some of the mTLS bits that our NewHTTPClient function does. Is that correct @melinath? If so maybe we can move that logic ad NewHTTPClient calls into NewTransport.
There was a problem hiding this comment.
and not necessary unit tests.
Some of the APIs in that package take testing.T which gave me pause.
meant to test the network
The package description is Package nettest provides utilities for network testing. which is sufficiently vague..."testing aspects of the network setup" vs. "network testing" vs. "for us in tests relating to the network" are all reasonable interpretations of this.
There was a problem hiding this comment.
Some of the APIs in that package take testing.T which gave me pause.
I had not noticed that at first glance. That does give me more pause.
I'll fix this, thanks for the shout out. |
This is a workaround for golang/go#25321, and is related to hashicorp/terraform-provider-google#6782
For additional context, see yaqs/5034757202074664960 and yaqs/47302089738551296.