Inlined addRequestID to HTTPProxy code. Moved test to integration tests.

fabiolb · May 16, 2017 · fb309ef · fb309ef
1 parent 0e76f0e
commit fb309ef
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 64 deletions.
diff --git a/proxy/http_headers.go b/proxy/http_headers.go
@@ -7,7 +7,6 @@ import (
 	"strings"
 
 	"github.com/fabiolb/fabio/config"
-	"github.com/fabiolb/fabio/uuid"
 )
 
 // addHeaders adds/updates headers in request
@@ -73,20 +72,6 @@ func addHeaders(r *http.Request, cfg config.Proxy) error {
 	return nil
 }
 
-// addRequestID Set header with name from `cfg.RequestID` to UUID value
-//
-func addRequestID(r *http.Request, p *HTTPProxy) {
-	if p.Config.RequestID != "" {
-		var id string
-		if p.UUID == nil {
-			id = uuid.NewUUID()
-		} else {
-			id = p.UUID()
-		}
-		r.Header.Set(p.Config.RequestID, id)
-	}
-}
-
 func scheme(r *http.Request) string {
 	ws := r.Header.Get("Upgrade") == "websocket"
 	switch {

diff --git a/proxy/http_headers_test.go b/proxy/http_headers_test.go
@@ -264,54 +264,6 @@ func TestAddHeaders(t *testing.T) {
 	}
 }
 
-func TestAddRequestID(t *testing.T) {
-	tests := []struct {
-		desc string
-		r    *http.Request
-		p    *HTTPProxy
-		hdrs http.Header
-		err  string
-	}{
-		{
-			"set requestid header",
-			&http.Request{RemoteAddr: "1.2.3.4:5555", Host: "5.6.7.8:1234"},
-			&HTTPProxy{
-				Config: config.Proxy{RequestID: "X-Request-Id"},
-				UUID:   func() string { return "f47ac10b-58cc-0372-8567-0e02b2c3d479" },
-			},
-			http.Header{
-				"X-Request-Id": []string{"f47ac10b-58cc-0372-8567-0e02b2c3d479"},
-			},
-			"",
-		},
-		{
-			"not set requestid header",
-			&http.Request{RemoteAddr: "1.2.3.4:5555", Host: "5.6.7.8:1234"},
-			&HTTPProxy{
-				Config: config.Proxy{},
-				UUID:   func() string { return "f47ac10b-58cc-0372-8567-0e02b2c3d479" },
-			},
-			http.Header{},
-			"",
-		},
-	}
-
-	for _, tt := range tests {
-		tt := tt // capture loop var
-
-		t.Run(tt.desc, func(t *testing.T) {
-			if tt.r.Header == nil {
-				tt.r.Header = http.Header{}
-			}
-
-			addRequestID(tt.r, tt.p)
-
-			got, want := tt.r.Header, tt.hdrs
-			verify.Values(t, "", got, want)
-		})
-	}
-}
-
 func TestLocalPort(t *testing.T) {
 	tests := []struct {
 		r    *http.Request

diff --git a/proxy/http_integration_test.go b/proxy/http_integration_test.go
@@ -50,6 +50,31 @@ func TestProxyProducesCorrectXffHeader(t *testing.T) {
 	}
 }
 
+func TestProxyRequestIDHeader(t *testing.T) {
+	got := "not called"
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		got = r.Header.Get("X-Request-ID")
+	}))
+	defer server.Close()
+
+	proxy := httptest.NewServer(&HTTPProxy{
+		Config:    config.Proxy{RequestID: "X-Request-Id"},
+		Transport: http.DefaultTransport,
+		UUID:      func() string { return "f47ac10b-58cc-0372-8567-0e02b2c3d479" },
+		Lookup: func(r *http.Request) *route.Target {
+			return &route.Target{URL: mustParse(server.URL)}
+		},
+	})
+	defer proxy.Close()
+
+	req, _ := http.NewRequest("GET", proxy.URL, nil)
+	mustDo(req)
+
+	if want := "f47ac10b-58cc-0372-8567-0e02b2c3d479"; got != want {
+		t.Errorf("got %v, but want %v", got, want)
+	}
+}
+
 func TestProxyNoRouteStaus(t *testing.T) {
 	proxy := httptest.NewServer(&HTTPProxy{
 		Config:    config.Proxy{NoRouteStatus: 999},

diff --git a/proxy/http_proxy.go b/proxy/http_proxy.go
@@ -15,6 +15,7 @@ import (
 	"github.com/fabiolb/fabio/metrics"
 	"github.com/fabiolb/fabio/proxy/gzip"
 	"github.com/fabiolb/fabio/route"
+	"github.com/fabiolb/fabio/uuid"
 )
 
 // HTTPProxy is a dynamic reverse proxy for HTTP and HTTPS protocols.
@@ -69,7 +70,16 @@ func (p *HTTPProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "cannot parse "+r.RemoteAddr, http.StatusInternalServerError)
 		return
 	}
-	addRequestID(r, p)
+
+	if p.Config.RequestID != "" {
+		var id string
+		if p.UUID == nil {
+			id = uuid.NewUUID()
+		} else {
+			id = p.UUID()
+		}
+		r.Header.Set(p.Config.RequestID, id)
+	}
 
 	// build the request url since r.URL will get modified
 	// by the reverse proxy and contains only the RequestURI anyway