From 344e8fca05852e281a901a39d47c431eff095306 Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Wed, 28 Feb 2018 11:40:17 -0500 Subject: [PATCH 1/2] Relax typing to allow for http.RoundTripper --- registry/client.go | 2 +- svchost/disco/disco.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/registry/client.go b/registry/client.go index 6525c5410..f778bfa4f 100644 --- a/registry/client.go +++ b/registry/client.go @@ -55,7 +55,7 @@ func NewClient(services *disco.Disco, creds auth.CredentialsSource, client *http client.Timeout = requestTimeout } - services.Transport = client.Transport.(*http.Transport) + services.Transport = client.Transport return &Client{ client: client, diff --git a/svchost/disco/disco.go b/svchost/disco/disco.go index db4046371..90f056025 100644 --- a/svchost/disco/disco.go +++ b/svchost/disco/disco.go @@ -40,9 +40,9 @@ type Disco struct { hostCache map[svchost.Hostname]Host credsSrc auth.CredentialsSource - // Transport is a custom http.Transport to use. + // Transport is a custom http.RoundTripper to use. // A package default is used if this is nil. - Transport *http.Transport + Transport http.RoundTripper } func NewDisco() *Disco { From c868092d2dd04fdde891619460a25e6a3cf6b42c Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Wed, 28 Feb 2018 11:40:43 -0500 Subject: [PATCH 2/2] Standardize http.Client creation with User-Agent --- backend/remote-state/gcs/backend.go | 4 +- httpclient/client.go | 18 +++++++ httpclient/client_test.go | 77 +++++++++++++++++++++++++++++ httpclient/useragent.go | 26 ++++++++++ plugin/discovery/get.go | 16 +++++- registry/client.go | 4 +- state/remote/gcs.go | 9 +--- svchost/disco/disco.go | 7 --- terraform/user_agent.go | 13 ++--- 9 files changed, 146 insertions(+), 28 deletions(-) create mode 100644 httpclient/client.go create mode 100644 httpclient/client_test.go create mode 100644 httpclient/useragent.go diff --git a/backend/remote-state/gcs/backend.go b/backend/remote-state/gcs/backend.go index cf11c4361..fc5109264 100644 --- a/backend/remote-state/gcs/backend.go +++ b/backend/remote-state/gcs/backend.go @@ -13,7 +13,7 @@ import ( "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/helper/pathorcontents" "github.com/hashicorp/terraform/helper/schema" - "github.com/hashicorp/terraform/terraform" + "github.com/hashicorp/terraform/httpclient" "golang.org/x/oauth2/jwt" "google.golang.org/api/option" ) @@ -156,7 +156,7 @@ func (b *gcsBackend) configure(ctx context.Context) error { opts = append(opts, option.WithScopes(storage.ScopeReadWrite)) } - opts = append(opts, option.WithUserAgent(terraform.UserAgentString())) + opts = append(opts, option.WithUserAgent(httpclient.UserAgentString())) client, err := storage.NewClient(b.storageContext, opts...) if err != nil { return fmt.Errorf("storage.NewClient() failed: %v", err) diff --git a/httpclient/client.go b/httpclient/client.go new file mode 100644 index 000000000..bb06beb47 --- /dev/null +++ b/httpclient/client.go @@ -0,0 +1,18 @@ +package httpclient + +import ( + "net/http" + + cleanhttp "github.com/hashicorp/go-cleanhttp" +) + +// New returns the DefaultPooledClient from the cleanhttp +// package that will also send a Terraform User-Agent string. +func New() *http.Client { + cli := cleanhttp.DefaultPooledClient() + cli.Transport = &userAgentRoundTripper{ + userAgent: UserAgentString(), + inner: cli.Transport, + } + return cli +} diff --git a/httpclient/client_test.go b/httpclient/client_test.go new file mode 100644 index 000000000..8a748f7c2 --- /dev/null +++ b/httpclient/client_test.go @@ -0,0 +1,77 @@ +package httpclient + +import ( + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/hashicorp/terraform/version" +) + +func TestUserAgent(t *testing.T) { + var actualUserAgent string + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + actualUserAgent = req.UserAgent() + })) + defer ts.Close() + + tsURL, err := url.Parse(ts.URL) + if err != nil { + t.Fatal(err) + } + + for i, c := range []struct { + expected string + request func(c *http.Client) error + }{ + { + fmt.Sprintf("Terraform/%s", version.Version), + func(c *http.Client) error { + _, err := c.Get(ts.URL) + return err + }, + }, + { + "foo/1", + func(c *http.Client) error { + req := &http.Request{ + Method: "GET", + URL: tsURL, + Header: http.Header{ + "User-Agent": []string{"foo/1"}, + }, + } + _, err := c.Do(req) + return err + }, + }, + { + "", + func(c *http.Client) error { + req := &http.Request{ + Method: "GET", + URL: tsURL, + Header: http.Header{ + "User-Agent": []string{""}, + }, + } + _, err := c.Do(req) + return err + }, + }, + } { + t.Run(fmt.Sprintf("%d %s", i, c.expected), func(t *testing.T) { + actualUserAgent = "" + cli := New() + err := c.request(cli) + if err != nil { + t.Fatal(err) + } + if actualUserAgent != c.expected { + t.Fatalf("actual User-Agent '%s' is not '%s'", actualUserAgent, c.expected) + } + }) + } +} diff --git a/httpclient/useragent.go b/httpclient/useragent.go new file mode 100644 index 000000000..d8db087cf --- /dev/null +++ b/httpclient/useragent.go @@ -0,0 +1,26 @@ +package httpclient + +import ( + "fmt" + "net/http" + + "github.com/hashicorp/terraform/version" +) + +const userAgentFormat = "Terraform/%s" + +func UserAgentString() string { + return fmt.Sprintf(userAgentFormat, version.Version) +} + +type userAgentRoundTripper struct { + inner http.RoundTripper + userAgent string +} + +func (rt *userAgentRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + if _, ok := req.Header["User-Agent"]; !ok { + req.Header.Set("User-Agent", rt.userAgent) + } + return rt.inner.RoundTrip(req) +} diff --git a/plugin/discovery/get.go b/plugin/discovery/get.go index f523c3608..815640f14 100644 --- a/plugin/discovery/get.go +++ b/plugin/discovery/get.go @@ -15,9 +15,9 @@ import ( "golang.org/x/net/html" - cleanhttp "github.com/hashicorp/go-cleanhttp" getter "github.com/hashicorp/go-getter" multierror "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform/httpclient" "github.com/mitchellh/cli" ) @@ -33,7 +33,19 @@ const protocolVersionHeader = "x-terraform-protocol-version" var releaseHost = "https://releases.hashicorp.com" -var httpClient = cleanhttp.DefaultPooledClient() +var httpClient *http.Client + +func init() { + httpClient = httpclient.New() + + httpGetter := &getter.HttpGetter{ + Client: httpClient, + Netrc: true, + } + + getter.Getters["http"] = httpGetter + getter.Getters["https"] = httpGetter +} // An Installer maintains a local cache of plugins by downloading plugins // from an online repository. diff --git a/registry/client.go b/registry/client.go index f778bfa4f..aefed84af 100644 --- a/registry/client.go +++ b/registry/client.go @@ -11,7 +11,7 @@ import ( "strings" "time" - cleanhttp "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/terraform/httpclient" "github.com/hashicorp/terraform/registry/regsrc" "github.com/hashicorp/terraform/registry/response" "github.com/hashicorp/terraform/svchost" @@ -51,7 +51,7 @@ func NewClient(services *disco.Disco, creds auth.CredentialsSource, client *http services.SetCredentialsSource(creds) if client == nil { - client = cleanhttp.DefaultPooledClient() + client = httpclient.New() client.Timeout = requestTimeout } diff --git a/state/remote/gcs.go b/state/remote/gcs.go index bfd5118cd..deaaef139 100644 --- a/state/remote/gcs.go +++ b/state/remote/gcs.go @@ -8,18 +8,16 @@ import ( "log" "net/http" "os" - "runtime" "strings" "github.com/hashicorp/terraform/helper/pathorcontents" + "github.com/hashicorp/terraform/httpclient" "golang.org/x/net/context" "golang.org/x/oauth2" "golang.org/x/oauth2/google" "golang.org/x/oauth2/jwt" "google.golang.org/api/googleapi" "google.golang.org/api/storage/v1" - - version "github.com/hashicorp/terraform/version" ) // accountFile represents the structure of the credentials JSON @@ -100,16 +98,13 @@ func gcsFactory(conf map[string]string) (Client, error) { return nil, err } } - versionString := version.Version - userAgent := fmt.Sprintf( - "(%s %s) Terraform/%s", runtime.GOOS, runtime.GOARCH, versionString) log.Printf("[INFO] Instantiating Google Storage Client...") clientStorage, err := storage.New(client) if err != nil { return nil, err } - clientStorage.UserAgent = userAgent + clientStorage.UserAgent = httpclient.UserAgentString() return &GCSClient{ clientStorage: clientStorage, diff --git a/svchost/disco/disco.go b/svchost/disco/disco.go index 90f056025..144384e04 100644 --- a/svchost/disco/disco.go +++ b/svchost/disco/disco.go @@ -8,7 +8,6 @@ package disco import ( "encoding/json" "errors" - "fmt" "io" "io/ioutil" "log" @@ -20,7 +19,6 @@ import ( cleanhttp "github.com/hashicorp/go-cleanhttp" "github.com/hashicorp/terraform/svchost" "github.com/hashicorp/terraform/svchost/auth" - "github.com/hashicorp/terraform/version" ) const ( @@ -30,7 +28,6 @@ const ( maxDiscoDocBytes = 1 * 1024 * 1024 // 1MB - to prevent abusive services from using loads of our memory ) -var userAgent = fmt.Sprintf("Terraform/%s (service discovery)", version.String()) var httpTransport = cleanhttp.DefaultPooledTransport() // overridden during tests, to skip TLS verification // Disco is the main type in this package, which allows discovery on given @@ -142,13 +139,9 @@ func (d *Disco) discover(host svchost.Hostname) Host { }, } - var header = http.Header{} - header.Set("User-Agent", userAgent) - req := &http.Request{ Method: "GET", URL: discoURL, - Header: header, } if d.credsSrc != nil { diff --git a/terraform/user_agent.go b/terraform/user_agent.go index 76bdae099..a42613e85 100644 --- a/terraform/user_agent.go +++ b/terraform/user_agent.go @@ -1,16 +1,13 @@ package terraform import ( - "fmt" - "runtime" - - "github.com/hashicorp/terraform/version" + "github.com/hashicorp/terraform/httpclient" ) -// The standard Terraform User-Agent format -const UserAgent = "Terraform %s (%s)" - // Generate a UserAgent string +// +// Deprecated: Use httpclient.UserAgentString if you are setting your +// own User-Agent header. func UserAgentString() string { - return fmt.Sprintf(UserAgent, version.String(), runtime.Version()) + return httpclient.UserAgentString() }