diff --git a/internal/proxy/oauthproxy.go b/internal/proxy/oauthproxy.go index 61078e25..668567c9 100755 --- a/internal/proxy/oauthproxy.go +++ b/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"} diff --git a/internal/proxy/proxy_config.go b/internal/proxy/proxy_config.go index 38d84c19..9eaeb64b 100644 --- a/internal/proxy/proxy_config.go +++ b/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), "_") // +} diff --git a/internal/proxy/proxy_config_test.go b/internal/proxy/proxy_config_test.go index 4a16d234..98cc5d09 100644 --- a/internal/proxy/proxy_config_test.go +++ b/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") + } + }) } }