httputil: remove Transport constructors aware of environment
Each of the servers has slightly different needs for configuring their
outbound HTTP transport. A little copying is better than a little
dependency.
Notably, the configuration logic in gddo-server is now consistent with
the rest of the configuration-gathering logic.
Change-Id: Ibe7d287a102e20e6e13d97e8a6483d9febab6442
Reviewed-on: https://go-review.googlesource.com/69291
Reviewed-by: Tuo Shan <shantuo@google.com>
diff --git a/gddo-server/client.go b/gddo-server/client.go
index af557a6..2761ac2 100644
--- a/gddo-server/client.go
+++ b/gddo-server/client.go
@@ -23,7 +23,7 @@
func newHTTPClient(v *viper.Viper) *http.Client {
requestTimeout := v.GetDuration(ConfigRequestTimeout)
- t := &http.Transport{
+ var t http.RoundTripper = &http.Transport{
Proxy: http.ProxyFromEnvironment,
Dial: (&net.Dialer{
Timeout: v.GetDuration(ConfigDialTimeout),
@@ -32,18 +32,22 @@
ResponseHeaderTimeout: requestTimeout / 2,
TLSHandshakeTimeout: requestTimeout / 2,
}
-
- var rt http.RoundTripper
if addr := v.GetString(ConfigMemcacheAddr); addr != "" {
ct := httpcache.NewTransport(memcache.New(addr))
ct.Transport = t
- rt = httputil.NewAuthTransport(ct)
- } else {
- rt = httputil.NewAuthTransport(t)
+ t = ct
}
+ t = &httputil.AuthTransport{
+ Base: t,
+
+ UserAgent: v.GetString(ConfigUserAgent),
+ GithubToken: v.GetString(ConfigGithubToken),
+ GithubClientID: v.GetString(ConfigGithubClientID),
+ GithubClientSecret: v.GetString(ConfigGithubClientSecret),
+ }
+ t = trace.Transport{Base: t}
return &http.Client{
- // Wrap the cached transport with GitHub authentication.
- Transport: trace.Transport{Base: rt},
+ Transport: t,
Timeout: requestTimeout,
}
}
diff --git a/gddo-server/config.go b/gddo-server/config.go
index a1d15a8..07845a2 100644
--- a/gddo-server/config.go
+++ b/gddo-server/config.go
@@ -18,6 +18,11 @@
const (
gaeProjectEnvVar = "GCLOUD_PROJECT"
gaAccountEnvVar = "GA_ACCOUNT"
+
+ userAgentEnvVar = "USER_AGENT"
+ githubTokenEnvVar = "GITHUB_TOKEN"
+ githubClientIDEnvVar = "GITHUB_CLIENT_ID"
+ githubClientSecretEnvVar = "GITHUB_CLIENT_SECRET"
)
const (
@@ -54,6 +59,12 @@
// Trace Config
ConfigTraceSamplerFraction = "trace_fraction"
ConfigTraceSamplerMaxQPS = "trace_max_qps"
+
+ // Outbound HTTP Config
+ ConfigUserAgent = "user_agent"
+ ConfigGithubToken = "github_token"
+ ConfigGithubClientID = "github_client_id"
+ ConfigGithubClientSecret = "github_client_secret"
)
func loadConfig(ctx context.Context, args []string) (*viper.Viper, error) {
@@ -67,6 +78,10 @@
if metadata.OnGCE() {
gceProjectAttributeDefault(ctx, v, ConfigGAAccount, "ga-account")
gceProjectAttributeDefault(ctx, v, ConfigGCELogName, "gce-log-name")
+ gceProjectAttributeDefault(ctx, v, ConfigUserAgent, "user-agent")
+ gceProjectAttributeDefault(ctx, v, ConfigGithubToken, "github-token")
+ gceProjectAttributeDefault(ctx, v, ConfigGithubClientID, "github-client-id")
+ gceProjectAttributeDefault(ctx, v, ConfigGithubClientSecret, "github-client-secret")
if id, err := metadata.ProjectID(); err != nil {
log.Warn(ctx, "failed to retrieve project ID", "error", err)
} else {
@@ -89,6 +104,10 @@
v.AutomaticEnv()
v.BindEnv(ConfigProject, gaeProjectEnvVar)
v.BindEnv(ConfigGAAccount, gaAccountEnvVar)
+ v.BindEnv(ConfigUserAgent, userAgentEnvVar)
+ v.BindEnv(ConfigGithubToken, githubTokenEnvVar)
+ v.BindEnv(ConfigGithubClientID, githubClientIDEnvVar)
+ v.BindEnv(ConfigGithubClientSecret, githubClientSecretEnvVar)
// Read from config.
if err := readViperConfig(ctx, v); err != nil {
diff --git a/gosrc/print.go b/gosrc/print.go
index 260592a..55e930f 100644
--- a/gosrc/print.go
+++ b/gosrc/print.go
@@ -16,6 +16,7 @@
"fmt"
"log"
"net/http"
+ "os"
"strings"
"github.com/golang/gddo/gosrc"
@@ -45,7 +46,13 @@
gosrc.SetLocalDevMode(*local)
}
c := &http.Client{
- Transport: httputil.NewAuthTransport(&http.Transport{}),
+ Transport: &httputil.AuthTransport{
+ Base: http.DefaultTransport,
+ UserAgent: os.Getenv("USER_AGENT"),
+ GithubToken: os.Getenv("GITHUB_TOKEN"),
+ GithubClientID: os.Getenv("GITHUB_CLIENT_ID"),
+ GithubClientSecret: os.Getenv("GITHUB_CLIENT_SECRET"),
+ },
}
dir, err := gosrc.Get(c, path, *etag)
if e, ok := err.(gosrc.NotFoundError); ok && e.Redirect != "" {
diff --git a/httputil/transport.go b/httputil/transport.go
index b41fe59..fdad3b4 100644
--- a/httputil/transport.go
+++ b/httputil/transport.go
@@ -10,12 +10,8 @@
package httputil
import (
- "log"
"net/http"
"net/url"
- "os"
-
- "cloud.google.com/go/compute/metadata"
)
// AuthTransport is an implementation of http.RoundTripper that authenticates
@@ -23,45 +19,11 @@
//
// When both a token and client credentials are set, the latter is preferred.
type AuthTransport struct {
- UserAgent string
- Token string
- ClientID string
- ClientSecret string
- Base http.RoundTripper
-}
-
-// NewAuthTransport gives new AuthTransport created with GitHub credentials
-// read from GCE metadata when the metadata server is accessible (we're on GCE)
-// or read from environment varialbes otherwise.
-func NewAuthTransport(base http.RoundTripper) *AuthTransport {
- if metadata.OnGCE() {
- return NewAuthTransportFromMetadata(base)
- }
- return NewAuthTransportFromEnvironment(base)
-}
-
-// NewAuthTransportFromEnvironment gives new AuthTransport created with GitHub
-// credentials read from environment variables.
-func NewAuthTransportFromEnvironment(base http.RoundTripper) *AuthTransport {
- return &AuthTransport{
- UserAgent: os.Getenv("USER_AGENT"),
- Token: os.Getenv("GITHUB_TOKEN"),
- ClientID: os.Getenv("GITHUB_CLIENT_ID"),
- ClientSecret: os.Getenv("GITHUB_CLIENT_SECRET"),
- Base: base,
- }
-}
-
-// NewAuthTransportFromMetadata gives new AuthTransport created with GitHub
-// credentials read from GCE metadata.
-func NewAuthTransportFromMetadata(base http.RoundTripper) *AuthTransport {
- return &AuthTransport{
- UserAgent: gceAttr("user-agent"),
- Token: gceAttr("github-token"),
- ClientID: gceAttr("github-client-id"),
- ClientSecret: gceAttr("github-client-secret"),
- Base: base,
- }
+ UserAgent string
+ GithubToken string
+ GithubClientID string
+ GithubClientSecret string
+ Base http.RoundTripper
}
// RoundTrip implements the http.RoundTripper interface.
@@ -73,20 +35,20 @@
}
if req.URL.Host == "api.github.com" && req.URL.Scheme == "https" {
switch {
- case t.ClientID != "" && t.ClientSecret != "":
+ case t.GithubClientID != "" && t.GithubClientSecret != "":
if reqCopy == nil {
reqCopy = copyRequest(req)
}
if reqCopy.URL.RawQuery == "" {
- reqCopy.URL.RawQuery = "client_id=" + t.ClientID + "&client_secret=" + t.ClientSecret
+ reqCopy.URL.RawQuery = "client_id=" + t.GithubClientID + "&client_secret=" + t.GithubClientSecret
} else {
- reqCopy.URL.RawQuery += "&client_id=" + t.ClientID + "&client_secret=" + t.ClientSecret
+ reqCopy.URL.RawQuery += "&client_id=" + t.GithubClientID + "&client_secret=" + t.GithubClientSecret
}
- case t.Token != "":
+ case t.GithubToken != "":
if reqCopy == nil {
reqCopy = copyRequest(req)
}
- reqCopy.Header.Set("Authorization", "token "+t.Token)
+ reqCopy.Header.Set("Authorization", "token "+t.GithubToken)
}
}
if reqCopy != nil {
@@ -112,15 +74,6 @@
return http.DefaultTransport
}
-func gceAttr(name string) string {
- s, err := metadata.ProjectAttributeValue(name)
- if err != nil {
- log.Printf("error querying metadata for %q: %s", name, err)
- return ""
- }
- return s
-}
-
func copyRequest(req *http.Request) *http.Request {
req2 := new(http.Request)
*req2 = *req
diff --git a/httputil/transport_test.go b/httputil/transport_test.go
index 2980d97..4b36118 100644
--- a/httputil/transport_test.go
+++ b/httputil/transport_test.go
@@ -87,9 +87,9 @@
query = r.URL.Query()
authHeader = r.Header.Get("Authorization")
}),
- Token: test.token,
- ClientID: test.clientID,
- ClientSecret: test.clientSecret,
+ GithubToken: test.token,
+ GithubClientID: test.clientID,
+ GithubClientSecret: test.clientSecret,
},
}
_, err := client.Get(test.url)
diff --git a/lintapp/main.go b/lintapp/main.go
index f4b686b..e77de4a 100644
--- a/lintapp/main.go
+++ b/lintapp/main.go
@@ -49,7 +49,6 @@
"timeago": timeagoFn,
"contactEmail": contactEmailFn,
}
- github = httputil.NewAuthTransportFromEnvironment(nil)
)
func parseTemplate(fnames ...string) *template.Template {
@@ -114,11 +113,11 @@
c := appengine.NewContext(r)
return &http.Client{
Transport: &httputil.AuthTransport{
- Token: github.Token,
- ClientID: github.ClientID,
- ClientSecret: github.ClientSecret,
- Base: &urlfetch.Transport{Context: c},
- UserAgent: fmt.Sprintf("%s (+http://%s/-/bot)", appengine.AppID(c), r.Host),
+ GithubToken: os.Getenv("GITHUB_TOKEN"),
+ GithubClientID: os.Getenv("GITHUB_CLIENT_ID"),
+ GithubClientSecret: os.Getenv("GITHUB_CLIENT_SECRET"),
+ Base: &urlfetch.Transport{Context: c},
+ UserAgent: fmt.Sprintf("%s (+http://%s/-/bot)", appengine.AppID(c), r.Host),
},
}
}
diff --git a/talksapp/main.go b/talksapp/main.go
index 5b15445..1cf85ac 100644
--- a/talksapp/main.go
+++ b/talksapp/main.go
@@ -41,6 +41,10 @@
// used for mocking in tests
getPresentation = gosrc.GetPresentation
playCompileURL = "https://play.golang.org/compile"
+
+ githubToken = os.Getenv("GITHUB_TOKEN")
+ githubClientID = os.Getenv("GITHUB_CLIENT_ID")
+ githubClientSecret = os.Getenv("GITHUB_CLIENT_SECRET")
)
func init() {
@@ -52,12 +56,13 @@
contactEmail = s
}
- if appengine.IsDevAppServer() {
- return
- }
- github := httputil.NewAuthTransportFromEnvironment(nil)
- if github.Token == "" || github.ClientID == "" || github.ClientSecret == "" {
- panic("missing GitHub metadata, follow the instructions on README.md")
+ if !appengine.IsDevAppServer() {
+ if githubToken == "" && githubClientID == "" && githubClientSecret == "" {
+ panic("missing GitHub metadata, follow the instructions on README.md")
+ }
+ if githubToken != "" && (githubClientID != "" || githubClientSecret != "") {
+ panic("GitHub token and client secret given, follow the instructions on README.md")
+ }
}
}
@@ -126,15 +131,14 @@
func httpClient(r *http.Request) *http.Client {
ctx, _ := context.WithTimeout(appengine.NewContext(r), 10*time.Second)
- github := httputil.NewAuthTransportFromEnvironment(nil)
return &http.Client{
Transport: &httputil.AuthTransport{
- Token: github.Token,
- ClientID: github.ClientID,
- ClientSecret: github.ClientSecret,
- Base: &urlfetch.Transport{Context: ctx},
- UserAgent: fmt.Sprintf("%s (+http://%s/-/bot)", appengine.AppID(ctx), r.Host),
+ GithubToken: githubToken,
+ GithubClientID: githubClientID,
+ GithubClientSecret: githubClientSecret,
+ Base: &urlfetch.Transport{Context: ctx},
+ UserAgent: fmt.Sprintf("%s (+http://%s/-/bot)", appengine.AppID(ctx), r.Host),
},
}
}
