Merge branch 'master' into static-assets

internal/proxy/oauthproxy.go

@@ -93,9 +93,11 @@ type StateParameter struct {
 
 // UpstreamProxy stores information necessary for proxying the request back to the upstream.
 type UpstreamProxy struct {
-	cookieName string
-	handler    http.Handler
-	auth       hmacauth.HmacAuth
+	name         string
+	cookieName   string
+	handler      http.Handler
+	auth         hmacauth.HmacAuth
+	statsdClient *statsd.Client
 }
 
 // deleteSSOCookieHeader deletes the session cookie from the request header string.
@@ -116,7 +118,15 @@ func (u *UpstreamProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if u.auth != nil {
 		u.auth.SignRequest(r)
 	}
+
+	start := time.Now()
 	u.handler.ServeHTTP(w, r)
+	duration := time.Now().Sub(start)
+
+	tags := []string{
+		fmt.Sprintf("service_name:%s", u.name),
+	}
+	u.statsdClient.Timing("proxy_request", duration, tags, 1.0)
 }
 
 // upstreamTransport is used to ensure that upstreams cannot override the
@@ -183,9 +193,11 @@ func NewRewriteReverseProxy(route *RewriteRoute) *httputil.ReverseProxy {
 // NewReverseProxyHandler creates a new http.Handler given a httputil.ReverseProxy
 func NewReverseProxyHandler(reverseProxy *httputil.ReverseProxy, opts *Options, config *UpstreamConfig) (http.Handler, []string) {
 	upstreamProxy := &UpstreamProxy{
-		handler:    reverseProxy,
-		auth:       config.HMACAuth,
-		cookieName: opts.CookieName,
+		name:         config.Service,
+		handler:      reverseProxy,
+		auth:         config.HMACAuth,
+		cookieName:   opts.CookieName,
+		statsdClient: opts.StatsdClient,
 	}
 	if config.FlushInterval != 0 {
 		return NewStreamingHandler(upstreamProxy, opts, config), []string{"handler:streaming"}

internal/proxy/proxy_config.go

@@ -17,6 +17,10 @@ const (
 	rewrite = "rewrite"
 )
 
+var (
+	space = regexp.MustCompile(`\s+`)
+)
+
 // ServiceConfig represents the configuration for a given service
 type ServiceConfig struct {
 	Service        string                     `yaml:"service"`
@@ -302,7 +306,7 @@ func resolveUpstreamConfig(service *ServiceConfig, override string) (*UpstreamCo
 		return nil, err
 	}
 
-	dst.Service = service.Service
+	dst.Service = cleanWhiteSpace(service.Service)
 	return dst, nil
 }
 
@@ -363,3 +367,8 @@ func parseOptionsConfig(proxy *UpstreamConfig) error {
 
 	return nil
 }
+
+func cleanWhiteSpace(s string) string {
+	// This trims all white space from a service name and collapses all remaining space to `_`
+	return space.ReplaceAllString(strings.TrimSpace(s), "_") //
+}

internal/proxy/proxy_config_test.go

@@ -1,7 +1,6 @@
 package proxy
 
 import (
-	"fmt"
 	"net/url"
 	"reflect"
 	"regexp"
@@ -10,47 +9,102 @@ import (
 )
 
 func TestUpstreamConfigParsing(t *testing.T) {
-	serviceName := "foo"
-	defaultField := "foo"
-	from := "foo.dev.sso"
-	to := "foo.internal.dev.sso"
-	data := []byte(fmt.Sprintf(`
-- service: %s
-  %s:
-    from: %s
-    to: %s
-`, serviceName, defaultField, from, to))
-
-	serviceConfigs, err := parseServiceConfigs(data)
-	if err != nil {
-		t.Fatalf("expected to parse error, got error:%v", err)
-	}
-
-	if serviceConfigs == nil {
-		t.Fatalf("expected to get non-nil config")
-	}
-
-	if len(serviceConfigs) != 1 {
-		t.Fatalf("expected to get a config with one item, got:%v", serviceConfigs)
-	}
-
-	serviceConfig := serviceConfigs[0]
-
-	if serviceConfig.Service != serviceName {
-		t.Errorf("expected config to have service:%q, got:%q", serviceName, serviceConfig.Service)
-	}
+	testCases := []struct {
+		name      string
+		rawConfig []byte
 
-	upstreamConfig, ok := serviceConfig.ClusterConfigs[defaultField]
-	if !ok {
-		t.Fatalf("expected config to have cluster field %q but has none", defaultField)
+		wantErr     *ErrParsingConfig
+		wantConfigs []*UpstreamConfig
+	}{
+		{
+			name: "basic parsing",
+			rawConfig: []byte(`
+- service: bar
+  default:
+    from: bar.{{cluster}}.{{root_domain}}
+    to: bar-internal.{{cluster}}.{{root_domain}}
+`),
+			wantConfigs: []*UpstreamConfig{
+				{
+					Service: "bar",
+					RouteConfig: RouteConfig{
+						From: "bar.sso.dev",
+						To:   "bar-internal.sso.dev",
+					},
+					Route: &SimpleRoute{
+						FromURL: &url.URL{
+							Scheme: "http",
+							Host:   "bar.sso.dev",
+						},
+						ToURL: &url.URL{
+							Scheme: "http",
+							Host:   "bar-internal.sso.dev",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "parse white space in service name correctly",
+			rawConfig: []byte(`
+- service: bar 	  service 	 
+  default:
+    from: bar.{{cluster}}.{{root_domain}}
+    to: bar-internal.{{cluster}}.{{root_domain}}
+`),
+			wantConfigs: []*UpstreamConfig{
+				{
+					Service: "bar_service",
+					RouteConfig: RouteConfig{
+						From: "bar.sso.dev",
+						To:   "bar-internal.sso.dev",
+					},
+					Route: &SimpleRoute{
+						FromURL: &url.URL{
+							Scheme: "http",
+							Host:   "bar.sso.dev",
+						},
+						ToURL: &url.URL{
+							Scheme: "http",
+							Host:   "bar-internal.sso.dev",
+						},
+					},
+				},
+			},
+		},
 	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			templateVars := map[string]string{
+				"root_domain": "dev",
+				"cluster":     "sso",
+			}
+			gotConfigs, err := loadServiceConfigs(tc.rawConfig, "sso", "http", templateVars)
+			if tc.wantErr != nil {
+				if err != tc.wantErr {
+					t.Logf("got err %v", err)
+					t.Logf("want err %v", tc.wantErr)
+					t.Fatalf("got unexpected error load service configs")
+				}
+				return
+			}
 
-	if upstreamConfig.RouteConfig.From != from {
-		t.Errorf("expected upstream config to have `from` %q but got %q", from, upstreamConfig.RouteConfig.From)
-	}
+			if err != nil {
+				t.Logf("got err %v", err)
+				t.Logf("want err %v", tc.wantErr)
+				t.Fatalf("got unexpected error load service configs")
+			}
 
-	if upstreamConfig.RouteConfig.To != to {
-		t.Errorf("expected upstream config to have `to` %q but got %q", to, upstreamConfig.RouteConfig.To)
+			if !reflect.DeepEqual(gotConfigs, tc.wantConfigs) {
+				for _, gotConfig := range gotConfigs {
+					t.Logf("got  %#v", gotConfig)
+				}
+				for _, wantConfig := range tc.wantConfigs {
+					t.Logf("want %#v", wantConfig)
+				}
+				t.Fatalf("expected configs to be equal")
+			}
+		})
 	}
 }