diff --git a/CHANGELOG.md b/CHANGELOG.md index d2bee7a..f13ef1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). -## [1.1.2] - 2017-08-16 +## [Unreleased] ### Changed - If InfluxDB database doesn't exist when writing, it will be created. -- Fixed bug causing `http: multiple response.WriteHeader calls` error if `io.Copy` failed when writing the response. +- Raw URL path will be used as the primary path when proxying to upstream services. ## [1.1.1] - 2017-08-15 ### Changed diff --git a/handlers/handler.go b/handlers/handler.go index 625a53a..e280ed1 100644 --- a/handlers/handler.go +++ b/handlers/handler.go @@ -48,7 +48,7 @@ func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) { // start a global trace sp := opentracing.StartSpan(fmt.Sprintf("%s %s", r.Method, - r.URL.Path, + r.URL.EscapedPath(), )) closer, str, err := utils.DupReaderAndString(r.Body) @@ -60,7 +60,7 @@ func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) { r.Body = closer sp.SetTag("http.request_body", str) - sp.SetTag("http.url", r.RequestURI) + sp.SetTag("http.url", r.URL.EscapedPath()) sp.SetTag("http.method", r.Method) jsonHeaders, err := json.Marshal(utils.FlattenHTTPHeaders(utils.OmitHeaderValues(r.Header, viper.GetString(config.FlagHeaderMaskValue.GetLong()), viper.GetString(config.FlagApikeyHeaderKey.GetLong())))) @@ -89,10 +89,10 @@ func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) { // log error logrus.WithFields(logrus.Fields{ "method": r.Method, - "uri": r.RequestURI, + "uri": r.URL.EscapedPath(), }).Error(e.Error()) - h.Metrics.Add(metrics.Metric{"http_response_code", strconv.Itoa(e.Status()), true}) + h.Metrics.Add(metrics.Metric{Name: "http_response_code", Value: strconv.Itoa(e.Status()), Index: true}) errStatus, err := json.Marshal(utils.JSONErr{Code: e.Status(), Msg: e.Error()}) if err != nil { @@ -116,10 +116,10 @@ func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) { // log error logrus.WithFields(logrus.Fields{ "method": r.Method, - "uri": r.RequestURI, + "uri": r.URL.EscapedPath(), }).Error("unknown error") - h.Metrics.Add(metrics.Metric{"http_response_code", strconv.Itoa(http.StatusInternalServerError), true}) + h.Metrics.Add(metrics.Metric{Name: "http_response_code", Value: strconv.Itoa(http.StatusInternalServerError), Index: true}) errStatus, err := json.Marshal(utils.JSONErr{Code: http.StatusInternalServerError, Msg: "unknown error"}) if err != nil { diff --git a/handlers/logger.go b/handlers/logger.go index 2bf20cd..0e76966 100644 --- a/handlers/logger.go +++ b/handlers/logger.go @@ -46,15 +46,15 @@ func Logger(influxCtlr *monitor.InfluxController, inner Handler) http.Handler { logrus.WithFields(logrus.Fields{ "client ip": strings.Split(r.RemoteAddr, ":")[0], "method": r.Method, - "uri": r.RequestURI, + "uri": r.URL.EscapedPath(), "totalTime": int(t1.Sub(t0) / time.Millisecond), }).Info("request details") inner.Metrics.Add( - metrics.Metric{"total_time", int(t1.Sub(t0) / time.Millisecond), false}, - metrics.Metric{"http_method", r.Method, true}, - metrics.Metric{"http_uri", r.RequestURI, false}, - metrics.Metric{"client_ip", strings.Split(r.RemoteAddr, ":")[0], false}, + metrics.Metric{Name: "total_time", Value: int(t1.Sub(t0) / time.Millisecond), Index: false}, + metrics.Metric{Name: "http_method", Value: r.Method, Index: true}, + metrics.Metric{Name: "http_uri", Value: r.URL.EscapedPath(), Index: false}, + metrics.Metric{Name: "client_ip", Value: strings.Split(r.RemoteAddr, ":")[0], Index: false}, ) if err := influxCtlr.WriteRequestData(inner.Metrics); err != nil { diff --git a/monitor/influxdb_test.go b/monitor/influxdb_test.go index f004f0f..3eb0830 100644 --- a/monitor/influxdb_test.go +++ b/monitor/influxdb_test.go @@ -68,8 +68,8 @@ func (c *mockClient) Close() error { func TestWriteRequestData(t *testing.T) { ctlr := &InfluxController{Client: &mockClient{}} m := &metrics.Metrics{ - metrics.Metric{"metric-one", "value-one", true}, - metrics.Metric{"metric-two", "value-two", false}, + metrics.Metric{Name: "metric-one", Value: "value-one", Index: true}, + metrics.Metric{Name: "metric-two", Value: "value-two", Index: false}, } assert.Equal(t, ctlr.WriteRequestData(m).Error(), "no database name") viper.SetDefault(config.FlagInfluxdbDatabase.GetLong(), "mydb") @@ -98,8 +98,8 @@ func TestCreateDatabase(t *testing.T) { func TestGetFields(t *testing.T) { assert.Equal(t, getFields(&metrics.Metrics{ - metrics.Metric{"metric-one", "value-one", true}, - metrics.Metric{"metric-two", "value-two", false}, + metrics.Metric{Name: "metric-one", Value: "value-one", Index: true}, + metrics.Metric{Name: "metric-two", Value: "value-two", Index: false}, }), map[string]interface{}{ "metric-one": "value-one", "metric-two": "value-two", @@ -108,16 +108,16 @@ func TestGetFields(t *testing.T) { func TestGetTags(t *testing.T) { tags, err := getTags(&metrics.Metrics{ - metrics.Metric{"metric-one", "value-one", true}, - metrics.Metric{"metric-two", "value-two", false}, + metrics.Metric{Name: "metric-one", Value: "value-one", Index: true}, + metrics.Metric{Name: "metric-two", Value: "value-two", Index: false}, }) assert.Nil(t, err) assert.Equal(t, tags, map[string]string{ "metric-one": "value-one", }) tags, err = getTags(&metrics.Metrics{ - metrics.Metric{"metric-one", 5, true}, - metrics.Metric{"metric-two", "value-two", false}, + metrics.Metric{Name: "metric-one", Value: 5, Index: true}, + metrics.Metric{Name: "metric-two", Value: "value-two", Index: false}, }) assert.Nil(t, tags) assert.Equal(t, err.Error(), "InfluxDB requires that the indexed field metric-one be of type string") diff --git a/steps/mockservice.go b/steps/mockservice.go index 2e16cc3..e0694ee 100644 --- a/steps/mockservice.go +++ b/steps/mockservice.go @@ -61,7 +61,7 @@ func (step MockServiceStep) GetName() string { func (step MockServiceStep) Do(ctx context.Context, m *metrics.Metrics, c *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error { // incoming proxy - untypedProxy, err := spec.ProxyStore.Get(r.URL.Path) + untypedProxy, err := spec.ProxyStore.Get(r.URL.EscapedPath()) if err != nil || untypedProxy == nil { return utils.StatusError{Code: http.StatusNotFound, Err: errors.New("proxy not found")} } @@ -122,7 +122,7 @@ func (step MockServiceStep) Do(ctx context.Context, m *metrics.Metrics, c *contr // alright so have an incoming path and we have a target path, if defined. // figure out with the diff is and search for that in the map. - targetPath := utils.ComputeTargetPath(proxy.Spec.Path, proxy.Spec.Target, r.URL.Path) + targetPath := utils.ComputeTargetPath(proxy.Spec.Path, proxy.Spec.Target, r.URL.EscapedPath()) // this variable will hold the index in the array // of the matching mock response, if any @@ -143,7 +143,7 @@ func (step MockServiceStep) Do(ctx context.Context, m *metrics.Metrics, c *contr if mockRespIndex < 0 { return utils.StatusError{Code: http.StatusNotFound, Err: fmt.Errorf("no mock response defined for the incoming path %s and target path %s", - r.URL.Path, + r.URL.EscapedPath(), targetPath, )} @@ -178,7 +178,7 @@ func (step MockServiceStep) Do(ctx context.Context, m *metrics.Metrics, c *contr HeaderMap: upstreamHeaders, } - m.Add(metrics.Metric{"http_response_code", strconv.Itoa(mok[mockRespIndex].Code), true}) + m.Add(metrics.Metric{Name: "http_response_code", Value: strconv.Itoa(mok[mockRespIndex].Code), Index: true}) *resp = *responseRecorder.Result() diff --git a/steps/pluginsonrequest.go b/steps/pluginsonrequest.go index 4df6377..9f0d3bf 100644 --- a/steps/pluginsonrequest.go +++ b/steps/pluginsonrequest.go @@ -46,7 +46,7 @@ func (step PluginsOnRequestStep) GetName() string { // Do executes the logic of the PluginsOnRequestStep step func (step PluginsOnRequestStep) Do(ctx context.Context, m *metrics.Metrics, c *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error { - untypedProxy, err := spec.ProxyStore.Get(r.URL.Path) + untypedProxy, err := spec.ProxyStore.Get(r.URL.EscapedPath()) if err != nil || untypedProxy == nil { return utils.StatusError{Code: http.StatusNotFound, Err: errors.New("proxy not found")} } diff --git a/steps/pluginsonresponse.go b/steps/pluginsonresponse.go index 6182fbd..e20f07c 100644 --- a/steps/pluginsonresponse.go +++ b/steps/pluginsonresponse.go @@ -46,7 +46,7 @@ func (step PluginsOnResponseStep) GetName() string { // Do executes the logic of the PluginsOnResponseStep step func (step PluginsOnResponseStep) Do(ctx context.Context, m *metrics.Metrics, c *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error { - untypedProxy, err := spec.ProxyStore.Get(r.URL.Path) + untypedProxy, err := spec.ProxyStore.Get(r.URL.EscapedPath()) if err != nil || untypedProxy == nil { return utils.StatusError{Code: http.StatusNotFound, Err: errors.New("proxy not found")} } diff --git a/steps/proxypass.go b/steps/proxypass.go index ccb4bd5..a84389b 100644 --- a/steps/proxypass.go +++ b/steps/proxypass.go @@ -82,7 +82,7 @@ func (step ProxyPassStep) Do(ctx context.Context, m *metrics.Metrics, c *control Target: typedProxy, // shouldn't be nil (unless the proxy is removed within the microseconds it takes to get to this code) } - up := create(p).setUpstreamURL(p, c).configureTLS(p).setUpstreamHeaders(p).performProxy(trace) + up := create(p).setUpstreamURL(p).configureTLS(p).setUpstreamHeaders(p).performProxy(trace) if up.Error != (utils.StatusError{}) { logrus.Errorf("oops: %s", up.Error) @@ -196,7 +196,7 @@ func (up *upstream) configureTLS(p *proxy) *upstream { } -func (up *upstream) setUpstreamURL(p *proxy, c *controller.Controller) *upstream { +func (up *upstream) setUpstreamURL(p *proxy) *upstream { if up.Error != (utils.StatusError{}) { return up @@ -212,8 +212,8 @@ func (up *upstream) setUpstreamURL(p *proxy, c *controller.Controller) *upstream Err: err, } } else { - // set path - u.Path = utils.ComputeTargetPath(p.Target.Spec.Path, p.Target.Spec.Target, p.Source.URL.Path) + u.Path = utils.ComputeTargetPath(p.Target.Spec.Path, p.Target.Spec.Target, p.Source.URL.EscapedPath()) + u.RawPath = u.EscapedPath() u.ForceQuery = p.Source.URL.ForceQuery u.RawQuery = p.Source.URL.RawQuery u.Fragment = p.Source.URL.Fragment diff --git a/steps/proxypass_test.go b/steps/proxypass_test.go index 77ecb94..b7308d2 100644 --- a/steps/proxypass_test.go +++ b/steps/proxypass_test.go @@ -50,6 +50,20 @@ func TestCreate(t *testing.T) { } +func TestSetUpstreamURL(t *testing.T) { + spec.ServiceStore.Set(spec.Service{ + Name: "my-service", + Namespace: "foo", + ClusterIP: "1.2.3.4", + Port: 8080, + }) + + p := getTestInternalProxies()[3] + result := create(p).setUpstreamURL(p) + assert.Equal(t, result.Request.URL.Path, "/foo/bar/https%3A%2F%2Ffoo.bar.com") + assert.Equal(t, result.Request.URL.EscapedPath(), "/foo/bar/https%253A%252F%252Ffoo.bar.com") +} + func TestSetK8sDiscoveredURI(t *testing.T) { assert := assert.New(t) @@ -97,6 +111,7 @@ func getTestInternalProxies() []*proxy { one, _ := url.Parse("http://www.foo.bar.com/api/v1/accounts") two, _ := url.Parse("http://www.foo.bar.com/api/v1/accounts/one/two") + three, _ := url.Parse("http://www.foo.bar.com/api/v1/accounts/https%3A%2F%2Ffoo.bar.com") return []*proxy{ { @@ -203,6 +218,27 @@ func getTestInternalProxies() []*proxy { }, }, }, + { + Source: &http.Request{ + URL: three, + }, + Target: spec.APIProxy{ + TypeMeta: unversioned.TypeMeta{}, + ObjectMeta: api.ObjectMeta{ + Name: "exampleAPIProxyOne", + Namespace: "foo", + }, + Spec: spec.APIProxySpec{ + Path: "/api/v1/accounts", + Target: "/foo/bar", + Service: spec.Service{ + Name: "my-service", + Namespace: "foo", + Port: 8080, + }, + }, + }, + }, } } diff --git a/steps/validateproxy.go b/steps/validateproxy.go index bf7c652..c27b3d3 100644 --- a/steps/validateproxy.go +++ b/steps/validateproxy.go @@ -46,7 +46,7 @@ func (step ValidateProxyStep) GetName() string { // Do executes the logic of the ValidateProxyStep step func (step ValidateProxyStep) Do(ctx context.Context, m *metrics.Metrics, c *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error { - untypedProxy, err := spec.ProxyStore.Get(r.URL.Path) + untypedProxy, err := spec.ProxyStore.Get(r.URL.EscapedPath()) if err != nil || untypedProxy == nil { if err != nil { logrus.Error(err.Error()) @@ -56,8 +56,8 @@ func (step ValidateProxyStep) Do(ctx context.Context, m *metrics.Metrics, c *con trace.SetTag("kanali.proxy_namespace", "unknown") m.Add( - metrics.Metric{"proxy_name", "unknown", true}, - metrics.Metric{"proxy_namespace", "unknown", true}, + metrics.Metric{Name: "proxy_name", Value: "unknown", Index: true}, + metrics.Metric{Name: "proxy_namespace", Value: "unknown", Index: true}, ) return utils.StatusError{Code: http.StatusNotFound, Err: errors.New("proxy not found")} @@ -70,8 +70,8 @@ func (step ValidateProxyStep) Do(ctx context.Context, m *metrics.Metrics, c *con trace.SetTag("kanali.proxy_namespace", "unknown") m.Add( - metrics.Metric{"proxy_name", "unknown", true}, - metrics.Metric{"proxy_namespace", "unknown", true}, + metrics.Metric{Name: "proxy_name", Value: "unknown", Index: true}, + metrics.Metric{Name: "proxy_namespace", Value: "unknown", Index: true}, ) return utils.StatusError{Code: http.StatusNotFound, Err: errors.New("proxy not found")} @@ -81,8 +81,8 @@ func (step ValidateProxyStep) Do(ctx context.Context, m *metrics.Metrics, c *con trace.SetTag("kanali.proxy_namespace", proxy.ObjectMeta.Namespace) m.Add( - metrics.Metric{"proxy_name", proxy.ObjectMeta.Name, true}, - metrics.Metric{"proxy_namespace", proxy.ObjectMeta.Namespace, true}, + metrics.Metric{Name: "proxy_name", Value: proxy.ObjectMeta.Name, Index: true}, + metrics.Metric{Name: "proxy_namespace", Value: proxy.ObjectMeta.Namespace, Index: true}, ) return nil diff --git a/steps/writeresponse.go b/steps/writeresponse.go index c828011..8d21140 100644 --- a/steps/writeresponse.go +++ b/steps/writeresponse.go @@ -51,7 +51,7 @@ func (step WriteResponseStep) Do(ctx context.Context, m *metrics.Metrics, c *con } } - m.Add(metrics.Metric{"http_response_code", strconv.Itoa(resp.StatusCode), true}) + m.Add(metrics.Metric{Name: "http_response_code", Value: strconv.Itoa(resp.StatusCode), Index: true}) closer, str, err := utils.DupReaderAndString(resp.Body) if err != nil { @@ -64,7 +64,7 @@ func (step WriteResponseStep) Do(ctx context.Context, m *metrics.Metrics, c *con w.WriteHeader(resp.StatusCode) if _, err := io.Copy(w, closer); err != nil { - return nil + return err } return nil