Use Custom Transport

[adrianliechti]

Currently, the New-Constructor throws an error, if a client uses a wrapped transport.
Maybe you could consider to allow this optionally

In my case, I would love to override the transport to allow tunneled connections. Other use cases might be to include OpenTelemetry Tracing Wrappers or similar

transport, ok := c.client.Transport.(*http.Transport)
	if !ok {
		return nil, fmt.Errorf("the configured base client's transport (%T) is not of type *http.Transport", c.client.Transport)
	}

My current workaround is to set a dummy client, construct the client and then override the transport afterwards.
But I would prefer a more official way :)

vaultClient := &http.Client{
  Transport: &http.Transport{},
}

v, err := vault.New(
		vault.WithHTTPClient(vaultClient),
		vault.WithAddress(address),
		vault.WithRequestTimeout(30*time.Second))

vaultClient.Transport := tunnelTransport(xxxx)

Cheers, and thank you for this new client!

[averche]

Thanks for opening the issue, @adrianliechti!

The recommended way for injecting a custom HTTP client is to start from vault.DefaultConfiguration().HTTPClient and modify it accordingly (since the default HTTP client has some vault-specific settings).

vaultClient.Transport := tunnelTransport(xxxx)

Is the transport that you are trying to inject not of type *http.Transport? Otherwise it should be possible to inject it by initializing it in the HTTPClient object that you pass in through vault.WithHTTPClient(vaultClient).

[adrianliechti]

Ciao @averche,

Thanks for hinting me to the DefaultConfiguration Builder.
But this seems not to change much, since also the, newClient() requires a http.Transport

Could you think of something like this:

if transport, ok := c.client.Transport.(*http.Transport); ok {
	// Adjust the dial context for unix domain socket addresses.
	if strings.HasPrefix(configuration.Address, "unix://") {
		transport.DialContext = func(context.Context, string, string) (net.Conn, error) {
			return net.Dial("unix", strings.TrimPrefix(configuration.Address, "unix://"))
		}
	}

	if err := configuration.TLS.applyTo(transport.TLSClientConfig); err != nil {
		return nil, err
	}
}

I think that supporting e.g. http.RoundTripper instead of http.Transport would allow more flexible use of this library.

I will optimize my hack but keep it for the moment while excitingly watching the progress of this repo

[averche]

We could definitely consider this though I'm a bit concerned that it would silently turn off TLS & unix socket support. We would need to properly error out in the cases where these settings are provided in conjunction with a non-standard transport.

I'll be away on vacation for a bit but will think about this a bit more :)

[astromechza]

We could definitely consider this though I'm a bit concerned that it would silently turn off TLS & unix socket support. We would need to properly error out in the cases where these settings are provided in conjunction with a non-standard transport.

I'd be happy with this kind of option - if folks override the transport then they should apply the same settings to it if the existing client can no longer do it.

[adrianliechti]

amazing! thanks a ton!!