diff --git a/internal/builds/register.go b/internal/builds/register.go index 00ea4f1e013758b7728fa0bb448b1b101840965a..4c902912c1883c7b197678d41ff6c392a6a50893 100644 --- a/internal/builds/register.go +++ b/internal/builds/register.go @@ -31,15 +31,11 @@ var ( }, []string{"state"}, ) -) -var ( registerHandlerOpenAtReading = registerHandlerOpen.WithLabelValues("reading") registerHandlerOpenAtProxying = registerHandlerOpen.WithLabelValues("proxying") registerHandlerOpenAtWatching = registerHandlerOpen.WithLabelValues("watching") -) -var ( registerHandlerBodyReadErrors = registerHandlerHits.WithLabelValues("body-read-error") registerHandlerBodyParseErrors = registerHandlerHits.WithLabelValues("body-parse-error") registerHandlerMissingValues = registerHandlerHits.WithLabelValues("missing-values") @@ -74,19 +70,18 @@ func readRunnerBody(w http.ResponseWriter, r *http.Request) ([]byte, error) { return helper.ReadRequestBody(w, r, maxRegisterBodySize) } -func readRunnerRequest(r *http.Request, body []byte) (runnerRequest, error) { - var runnerRequest runnerRequest - +func readRunnerRequest(r *http.Request, body []byte) (*runnerRequest, error) { if !helper.IsApplicationJson(r) { - return runnerRequest, errors.New("invalid content-type received") + return nil, errors.New("invalid content-type received") } + var runnerRequest runnerRequest err := json.Unmarshal(body, &runnerRequest) if err != nil { - return runnerRequest, err + return nil, err } - return runnerRequest, nil + return &runnerRequest, nil } func proxyRegisterRequest(h http.Handler, w http.ResponseWriter, r *http.Request) { @@ -141,7 +136,7 @@ func RegisterHandler(h http.Handler, watchHandler WatchKeyHandler, pollingDurati switch result { // It means that we detected a change before starting watching on change, - // We proxy request to Rails, to see whether we can receive the build + // We proxy request to Rails, to see whether we have a build to receive case redis.WatchKeyStatusAlreadyChanged: registerHandlerAlreadyChangedHits.Inc() proxyRegisterRequest(h, w, newRequest) @@ -150,7 +145,7 @@ func RegisterHandler(h http.Handler, watchHandler WatchKeyHandler, pollingDurati // We could potentially proxy request to Rails, but... // We can end-up with unreliable responses, // as don't really know whether ResponseWriter is still in a sane state, - // whether the connection is not dead + // for example the connection is dead case redis.WatchKeyStatusSeenChange: registerHandlerSeenChangeHits.Inc() w.WriteHeader(http.StatusNoContent) diff --git a/internal/builds/register_test.go b/internal/builds/register_test.go index f28068d1b35444ad2c66478320fcb67a9e0fd131..519738a31bbc99731e169c7d2e543fb584bb9b2f 100644 --- a/internal/builds/register_test.go +++ b/internal/builds/register_test.go @@ -20,8 +20,6 @@ func echoRequest(rw http.ResponseWriter, req *http.Request) { var echoRequestFunc = http.HandlerFunc(echoRequest) -const applicationJson = "application/json" - func expectHandlerWithWatcher(t *testing.T, watchHandler WatchKeyHandler, data string, contentType string, expectedHttpStatus int, msgAndArgs ...interface{}) { h := RegisterHandler(echoRequestFunc, watchHandler, time.Second) @@ -40,7 +38,7 @@ func expectHandler(t *testing.T, data string, contentType string, expectedHttpSt func TestRegisterHandlerLargeBody(t *testing.T) { data := strings.Repeat(".", maxRegisterBodySize+5) - expectHandler(t, data, applicationJson, http.StatusRequestEntityTooLarge, + expectHandler(t, data, "application/json", http.StatusRequestEntityTooLarge, "rejects body with entity too large") } @@ -50,20 +48,20 @@ func TestRegisterHandlerInvalidRunnerRequest(t *testing.T) { } func TestRegisterHandlerInvalidJsonPayload(t *testing.T) { - expectHandler(t, "{[", applicationJson, http.StatusOK, + expectHandler(t, `{[`, "application/json", http.StatusOK, "fails on parsing body and proxies request to upstream") } func TestRegisterHandlerMissingData(t *testing.T) { - dataList := []string{"{\"token\":\"token\"}", "{\"last_update\":\"data\"}"} + dataList := []string{`{"token":"token"}`, `{"last_update":"data"}`} for _, data := range dataList { - expectHandler(t, data, applicationJson, http.StatusOK, + expectHandler(t, data, "application/json", http.StatusOK, "fails on argument validation and proxies request to upstream") } } -func exceptWatcherToBeExecuted(t *testing.T, watchKeyStatus redis.WatchKeyStatus, watchKeyError error, +func expectWatcherToBeExecuted(t *testing.T, watchKeyStatus redis.WatchKeyStatus, watchKeyError error, httpStatus int, msgAndArgs ...interface{}) { executed := false watchKeyHandler := func(key, value string, timeout time.Duration) (redis.WatchKeyStatus, error) { @@ -71,33 +69,33 @@ func exceptWatcherToBeExecuted(t *testing.T, watchKeyStatus redis.WatchKeyStatus return watchKeyStatus, watchKeyError } - parsableData := "{\"token\":\"token\",\"last_update\":\"last_update\"}" + parsableData := `{"token":"token","last_update":"last_update"}` - expectHandlerWithWatcher(t, watchKeyHandler, parsableData, applicationJson, httpStatus, msgAndArgs...) + expectHandlerWithWatcher(t, watchKeyHandler, parsableData, "application/json", httpStatus, msgAndArgs...) assert.True(t, executed, msgAndArgs...) } func TestRegisterHandlerWatcherError(t *testing.T) { - exceptWatcherToBeExecuted(t, redis.WatchKeyStatusNoChange, errors.New("redis connection"), + expectWatcherToBeExecuted(t, redis.WatchKeyStatusNoChange, errors.New("redis connection"), http.StatusOK, "proxies data to upstream") } func TestRegisterHandlerWatcherAlreadyChanged(t *testing.T) { - exceptWatcherToBeExecuted(t, redis.WatchKeyStatusAlreadyChanged, nil, + expectWatcherToBeExecuted(t, redis.WatchKeyStatusAlreadyChanged, nil, http.StatusOK, "proxies data to upstream") } func TestRegisterHandlerWatcherSeenChange(t *testing.T) { - exceptWatcherToBeExecuted(t, redis.WatchKeyStatusSeenChange, nil, + expectWatcherToBeExecuted(t, redis.WatchKeyStatusSeenChange, nil, http.StatusNoContent) } func TestRegisterHandlerWatcherTimeout(t *testing.T) { - exceptWatcherToBeExecuted(t, redis.WatchKeyStatusTimeout, nil, + expectWatcherToBeExecuted(t, redis.WatchKeyStatusTimeout, nil, http.StatusNoContent) } func TestRegisterHandlerWatcherNoChange(t *testing.T) { - exceptWatcherToBeExecuted(t, redis.WatchKeyStatusNoChange, nil, + expectWatcherToBeExecuted(t, redis.WatchKeyStatusNoChange, nil, http.StatusNoContent) } diff --git a/internal/helper/helpers.go b/internal/helper/helpers.go index d039d49bda6f8c785123e9f9ccddea97266a2e65..72c37c8a1d3ef3dd117777e7abb26ce5a8635699 100644 --- a/internal/helper/helpers.go +++ b/internal/helper/helpers.go @@ -197,6 +197,7 @@ func ReadRequestBody(w http.ResponseWriter, r *http.Request, maxBodySize int64) func CloneRequestWithNewBody(r *http.Request, body []byte) *http.Request { newReq := *r newReq.Body = ioutil.NopCloser(bytes.NewReader(body)) + newReq.Header = HeaderClone(r.Header) newReq.ContentLength = int64(len(body)) return &newReq }