Override http.Client.Transport

[InPermutation]

In #245, support was proposed for overriding the http.Client. However, I only want to modify the http.Transport, so I have to duplicate the entire http.Client configuration code inside my module, like so:

func newChefHttpClient(cfg *chef.Config) *chef.Config {
       // This is copied directly from go-chef/chef/http.go
       tlsConfig := &tls.Config{InsecureSkipVerify: cfg.SkipSSL}
       if cfg.RootCAs != nil {
               tlsConfig.RootCAs = cfg.RootCAs
       }
       tr := &http.Transport{
               Proxy: http.ProxyFromEnvironment,
               Dial: (&net.Dialer{
                       Timeout:   30 * time.Second,
                       KeepAlive: 30 * time.Second,
               }).Dial,
               TLSClientConfig:     tlsConfig,
               TLSHandshakeTimeout: 10 * time.Second,
       }

       if cfg.Proxy != nil {
               tr.Proxy = cfg.Proxy
       }

       cfg.Client = &http.Client{
               Transport: tr,
               Timeout:   time.Duration(cfg.Timeout) * time.Second,
       }

       return cfg
}

Then, I set the cfg.Client.Transport after calling this function.

This is brittle, because go-chef/chef's implementation may change, and I'd like to get those changes too whenever the rest of the package is updated.

I'd prefer a way to set the Transport on the constructed *http.Client. However, I still need to be able to chain to the old Transport, for the same reason as above: I don't want to duplicate all the code to set Proxy Dial TLSClientConfig TLSHandshakeTimeout, and risk it drifting away from this implementation.

So I added the Config field RoundTripper func(http.RoundTripper) http.RoundTripper which can modify or replace the existing Transport.

I wrote tests similar to the Client tests from #246. They don't actually test whether the override is used, only whether it is set.

Alternate approaches

I don't know which approach is best, so I wrote a PR for you to help me decide. Please share your thoughts!

1. A func which modifies http.Client.Transport. (This PR).
2. A func which modifies http.Client - a caller could replace, save a pointer, or mutate the client at will, without being so tightly coupled to the Transport specifically.
3. Export else { tlsConfig := .... c.client = &http.Client{ ... } } code as a constructor function. The consumer can mutate/replace and pass it back as cfg.Client.
4. Export the Client *http.Client field on the type Client struct. (Why not? Every other field is exported!)

[MarkGibbons]

Thanks for the PR. I'll take a look.

[MarkGibbons]

I was stuck upgrading code to get #249 merged. I'll start working on this and the next PR.

[MarkGibbons]

I finally have integration tests working. I'll finish writing the tests and get this merged. I'm going with your options #1 allowing the config option and #4 exporting the Client field of the Client struct. School and a vacation got in the way of spending enough time on this. Sorry.

[MarkGibbons]

This PR was merged as part of #255.