smutel / terraform-provider-netbox

Terraform provider for Netbox
ISC License
58 stars 19 forks source link

fix: Enable http transport connection reuse #207

Closed amhn closed 1 year ago

amhn commented 1 year ago

fixes: #206

Reasons to remove transport struct and simplify client setup

This allowed to use the default client with the default transport and. if insecure is set, with a simplified tls.Config. I did not check the behaviour with insecure = true, as I do not have a setup for this. http and https with a trusted cert is confirmed working.

If you think this is too big of a change, the following should work too:

diff --git a/netbox/provider.go b/netbox/provider.go
index 3a984b20..a8e578fa 100644
--- a/netbox/provider.go
+++ b/netbox/provider.go
@@ -251,9 +251,10 @@ func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) {
        base := t.base
        if base == nil {
                // init an http.Transport with TLSOptions
-               customTransport := http.DefaultTransport.(*http.Transport).Clone()
+               customTransport := http.DefaultTransport.(*http.Transport)
                customTransport.TLSClientConfig = t.TLSClientConfig
                base = customTransport
+               t.base = base
        }
        return base.RoundTrip(req)
 }
smutel commented 1 year ago

Hello @amhn, could you please rebase and we will see if the linter is ok.

amhn commented 1 year ago

Done. Lint error stays. Is NETBOX_INSECURE set in the CI-Variables? On my machine I do not get this error.

(Sorry. Took me a while to react, occupied with other things.)

amhn commented 1 year ago

Changing the way to handle insecure seems to have helped. I.e. not creating a new TLSConfig but setting InsecureSkipVerify on the existing.

I have NOT tested this change with an invalid certificate.