diff --git a/pkg/agent/protocol/http/proxy.go b/pkg/agent/protocol/http/proxy.go index e6a4c290..df78ecf5 100644 --- a/pkg/agent/protocol/http/proxy.go +++ b/pkg/agent/protocol/http/proxy.go @@ -75,17 +75,10 @@ func NewProxy(c ProxyConfig, d Disruption) (protocol.Proxy, error) { }, nil } -// httpClient defines the method for executing HTTP requests. It is used to allow mocking -// the client in tests -type httpClient interface { - Do(req *http.Request) (*http.Response, error) -} - // httpHandler implements a http.Handler for disrupting request to a upstream server type httpHandler struct { upstreamURL url.URL disruption Disruption - client httpClient metrics *protocol.MetricMap } @@ -111,7 +104,7 @@ func (h *httpHandler) forward(rw http.ResponseWriter, req *http.Request, delay t upstreamReq.URL.Scheme = h.upstreamURL.Scheme upstreamReq.RequestURI = "" // It is an error to set this field in an HTTP client request. - response, err := h.client.Do(upstreamReq) + response, err := http.DefaultClient.Do(upstreamReq) <-timer if err != nil { rw.WriteHeader(http.StatusBadGateway) @@ -183,7 +176,6 @@ func (p *proxy) Start() error { handler := &httpHandler{ upstreamURL: *upstreamURL, disruption: p.disruption, - client: http.DefaultClient, metrics: &p.metrics, } diff --git a/pkg/agent/protocol/http/proxy_test.go b/pkg/agent/protocol/http/proxy_test.go index c3f40018..079b5260 100644 --- a/pkg/agent/protocol/http/proxy_test.go +++ b/pkg/agent/protocol/http/proxy_test.go @@ -2,41 +2,16 @@ package http import ( "bytes" - "fmt" "io" "net/http" "net/http/httptest" "net/url" - "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/grafana/xk6-disruptor/pkg/agent/protocol" ) -// fakeHTTPClient mocks the execution of a request returning the predefines -// status and body -type fakeHTTPClient struct { - status int - headers http.Header - body []byte -} - -func (f *fakeHTTPClient) Do(_ *http.Request) (*http.Response, error) { - resp := &http.Response{ - Proto: "HTTP/1.1", - ProtoMajor: 1, - ProtoMinor: 1, - StatusCode: f.status, - Status: http.StatusText(f.status), - Header: f.headers, - Body: io.NopCloser(strings.NewReader(string(f.body))), - ContentLength: int64(len(f.body)), - } - - return resp, nil -} - func Test_Validations(t *testing.T) { t.Parallel() @@ -195,12 +170,11 @@ func Test_ProxyHandler(t *testing.T) { type TestCase struct { title string disruption Disruption - config ProxyConfig method string path string statusCode int - headers http.Header - body []byte + upstreamHeaders http.Header + upstreamBody []byte expectedStatus int expectedHeaders http.Header expectedBody []byte @@ -216,13 +190,9 @@ func Test_ProxyHandler(t *testing.T) { ErrorCode: 0, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":9080", - UpstreamAddress: "http://127.0.0.1:8080", - }, path: "", statusCode: 200, - body: []byte("content body"), + upstreamBody: []byte("content body"), expectedStatus: 200, expectedBody: []byte("content body"), }, @@ -235,13 +205,9 @@ func Test_ProxyHandler(t *testing.T) { ErrorCode: 500, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":9080", - UpstreamAddress: "http://127.0.0.1:8080", - }, path: "", statusCode: 200, - body: []byte("content body"), + upstreamBody: []byte("content body"), expectedStatus: 500, expectedBody: []byte(""), }, @@ -254,13 +220,9 @@ func Test_ProxyHandler(t *testing.T) { ErrorCode: 500, Excluded: []string{"/excluded/path"}, }, - config: ProxyConfig{ - ListenAddress: ":9080", - UpstreamAddress: "http://127.0.0.1:8080", - }, path: "/excluded/path", statusCode: 200, - body: []byte("content body"), + upstreamBody: []byte("content body"), expectedStatus: 200, expectedBody: []byte("content body"), }, @@ -273,13 +235,9 @@ func Test_ProxyHandler(t *testing.T) { ErrorCode: 500, Excluded: []string{"/excluded/path"}, }, - config: ProxyConfig{ - ListenAddress: ":9080", - UpstreamAddress: "http://127.0.0.1:8080", - }, path: "/non-excluded/path", statusCode: 200, - body: []byte("content body"), + upstreamBody: []byte("content body"), expectedStatus: 500, expectedBody: []byte(""), }, @@ -293,13 +251,9 @@ func Test_ProxyHandler(t *testing.T) { ErrorBody: "{\"error\": 500, \"message\":\"internal server error\"}", Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":9080", - UpstreamAddress: "http://127.0.0.1:8080", - }, path: "", statusCode: 200, - body: []byte("content body"), + upstreamBody: []byte("content body"), expectedStatus: 500, expectedBody: []byte("{\"error\": 500, \"message\":\"internal server error\"}"), }, @@ -308,16 +262,12 @@ func Test_ProxyHandler(t *testing.T) { disruption: Disruption{ Excluded: []string{"/excluded"}, }, - config: ProxyConfig{ - ListenAddress: ":9080", - UpstreamAddress: "http://127.0.0.1:8080", - }, path: "/excluded", statusCode: 200, - headers: http.Header{ + upstreamHeaders: http.Header{ "X-Test-Header": []string{"A-Test"}, }, - body: []byte("content body"), + upstreamBody: []byte("content body"), expectedStatus: 200, expectedHeaders: http.Header{ "X-Test-Header": []string{"A-Test"}, @@ -329,15 +279,11 @@ func Test_ProxyHandler(t *testing.T) { disruption: Disruption{ ErrorRate: 0, }, - config: ProxyConfig{ - ListenAddress: ":9080", - UpstreamAddress: "http://127.0.0.1:8080", - }, statusCode: 200, - headers: http.Header{ + upstreamHeaders: http.Header{ "X-Test-Header": []string{"A-Test"}, }, - body: []byte("content body"), + upstreamBody: []byte("content body"), expectedStatus: 200, expectedHeaders: http.Header{ "X-Test-Header": []string{"A-Test"}, @@ -350,15 +296,11 @@ func Test_ProxyHandler(t *testing.T) { ErrorRate: 1.0, ErrorCode: 500, }, - config: ProxyConfig{ - ListenAddress: ":9080", - UpstreamAddress: "http://127.0.0.1:8080", - }, statusCode: 200, - headers: http.Header{ + upstreamHeaders: http.Header{ "X-Test-Header": []string{"A-Test"}, }, - body: []byte("content body"), + upstreamBody: []byte("content body"), expectedStatus: 500, expectedHeaders: http.Header{}, expectedBody: nil, @@ -371,48 +313,64 @@ func Test_ProxyHandler(t *testing.T) { t.Run(tc.title, func(t *testing.T) { t.Parallel() - client := &fakeHTTPClient{ - body: tc.body, - status: tc.expectedStatus, - headers: tc.headers, - } + upstreamServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + for k, values := range tc.upstreamHeaders { + for _, v := range values { + rw.Header().Add(k, v) + } + } + rw.WriteHeader(tc.statusCode) - upstreamURL, err := url.Parse(tc.config.UpstreamAddress) + _, err := rw.Write(tc.upstreamBody) + if err != nil { + t.Fatalf("writing upstream body: %v", err) + } + })) + + upstreamURL, err := url.Parse(upstreamServer.URL) if err != nil { - t.Errorf("error parsing upstream address %v", err) - return + t.Fatalf("error parsing httptest url") } + handler := &httpHandler{ upstreamURL: *upstreamURL, - client: client, disruption: tc.disruption, metrics: &protocol.MetricMap{}, } - reqURL := fmt.Sprintf("http://%s%s", tc.config.ListenAddress, tc.path) - req := httptest.NewRequest(tc.method, reqURL, strings.NewReader(string(tc.body))) - recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, req) - resp := recorder.Result() + proxyServer := httptest.NewServer(handler) + + req, err := http.NewRequest(tc.method, proxyServer.URL+tc.path, nil) + if err != nil { + t.Fatalf("building request to proxy: %v", err) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("making request to proxy: %v", err) + } if tc.expectedStatus != resp.StatusCode { - t.Errorf("expected status code '%d' but '%d' received ", tc.expectedStatus, resp.StatusCode) - return + t.Fatalf("expected status code '%d' but '%d' received ", tc.expectedStatus, resp.StatusCode) } + // Remove standard response headers so we don't need to specify them on every test case. + resp.Header.Del("content-length") + resp.Header.Del("content-type") + resp.Header.Del("date") + // Compare headers only if either expected or returned have items. // We have to check for length explicitly as otherwise a nil map would not be equal to an empty map. - if len(tc.headers) > 0 || len(tc.expectedHeaders) > 0 { + if len(tc.upstreamHeaders) > 0 || len(tc.expectedHeaders) > 0 { if diff := cmp.Diff(tc.expectedHeaders, resp.Header); diff != "" { - t.Errorf("Expected headers did not match returned:\n%s", diff) + t.Fatalf("Expected headers did not match returned:\n%s", diff) } } var body bytes.Buffer _, _ = io.Copy(&body, resp.Body) if !bytes.Equal(tc.expectedBody, body.Bytes()) { - t.Errorf("expected body '%s' but '%s' received ", tc.expectedBody, body.Bytes()) - return + t.Fatalf("expected body '%s' but '%s' received ", tc.expectedBody, body.Bytes()) } }) } @@ -466,7 +424,6 @@ func Test_Metrics(t *testing.T) { handler := &httpHandler{ upstreamURL: *upstreamURL, disruption: tc.config, - client: http.DefaultClient, metrics: &metrics, }