diff options
author | Jaime Martinez <jmartinez@gitlab.com> | 2022-04-26 22:54:52 +0300 |
---|---|---|
committer | Jaime Martinez <jmartinez@gitlab.com> | 2022-04-26 22:54:52 +0300 |
commit | 46b4706ea917e4e5018c455d6737893d9426aada (patch) | |
tree | 807f7fbfaae9ad444e74cf37165669246e537312 | |
parent | 4a8a5d61f87e2aa69630ccf4aacef8cf836e934a (diff) | |
parent | cce3706f57ec6424ea7c62ebffd19a3ccbe49c0e (diff) |
Merge branch 'healthcheck/middleware' into 'master'
refactor: move healthcheck middleware early in the pipeline
See merge request gitlab-org/gitlab-pages!616
-rw-r--r-- | app.go | 32 | ||||
-rw-r--r-- | app_test.go | 65 | ||||
-rw-r--r-- | internal/healthcheck/middleware.go | 19 | ||||
-rw-r--r-- | internal/healthcheck/middleware_test.go | 50 |
4 files changed, 71 insertions, 95 deletions
@@ -30,6 +30,7 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/domain" "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/handlers" + health "gitlab.com/gitlab-org/gitlab-pages/internal/healthcheck" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/netutil" @@ -59,10 +60,6 @@ type theApp struct { CustomHeaders http.Header } -func (a *theApp) isReady() bool { - return true -} - func (a *theApp) GetCertificate(ch *cryptotls.ClientHelloInfo) (*cryptotls.Certificate, error) { if ch.ServerName == "" { return nil, nil @@ -121,11 +118,6 @@ func (a *theApp) tryAuxiliaryHandlers(w http.ResponseWriter, r *http.Request, ht return true } - if !a.isReady() { - httperrors.Serve503(w) - return true - } - if _, err := domain.GetLookupPath(r); err != nil { if errors.Is(err, gitlab.ErrDiskDisabled) { errortracking.CaptureErrWithReqAndStackTrace(err, r) @@ -146,26 +138,6 @@ func (a *theApp) tryAuxiliaryHandlers(w http.ResponseWriter, r *http.Request, ht return false } -// healthCheckMiddleware is serving the application status check -func (a *theApp) healthCheckMiddleware(handler http.Handler) http.Handler { - healthCheck := http.HandlerFunc(func(w http.ResponseWriter, _r *http.Request) { - if a.isReady() { - w.Write([]byte("success\n")) - } else { - http.Error(w, "not yet ready", http.StatusServiceUnavailable) - } - }) - - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.RequestURI == a.config.General.StatusPath { - healthCheck.ServeHTTP(w, r) - return - } - - handler.ServeHTTP(w, r) - }) -} - // auxiliaryMiddleware will handle status updates, not-ready requests and other // not static-content responses func (a *theApp) auxiliaryMiddleware(handler http.Handler) http.Handler { @@ -247,7 +219,7 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) { handler = handlers.Ratelimiter(handler, &a.config.RateLimit) // Health Check - handler = a.healthCheckMiddleware(handler) + handler = health.NewMiddleware(handler, a.config.General.StatusPath) // Custom response headers handler = customheaders.NewMiddleware(handler, a.CustomHeaders) diff --git a/app_test.go b/app_test.go index 3b1d6697..75747a2d 100644 --- a/app_test.go +++ b/app_test.go @@ -3,18 +3,14 @@ package main import ( "crypto/tls" "fmt" - "io" "net/http" "net/http/httptest" "testing" - "time" "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/request" - "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab" "gitlab.com/gitlab-org/gitlab-pages/metrics" ) @@ -66,67 +62,6 @@ func newGetRequestWithScheme(t *testing.T, scheme string, withTLS bool) *http.Re return req } -func TestHealthCheckMiddleware(t *testing.T) { - tests := []struct { - name string - path string - status int - body string - }{ - { - name: "Not a healthcheck request", - path: "/foo/bar", - status: http.StatusOK, - body: "Hello from inner handler", - }, - { - name: "Healthcheck request", - path: "/-/healthcheck", - status: http.StatusOK, - body: "success\n", - }, - } - - validCfg := config.GitLab{ - InternalServer: "server", - APISecretKey: []byte("secret"), - ClientHTTPTimeout: time.Second, - JWTTokenExpiration: time.Second, - } - - source, err := gitlab.New(&validCfg) - require.NoError(t, err) - - cfg := config.Config{ - General: config.General{ - StatusPath: "/-/healthcheck", - }, - } - - app := theApp{ - config: &cfg, - source: source, - } - - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - io.WriteString(w, "Hello from inner handler") - }) - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - r := httptest.NewRequest("GET", tc.path, nil) - rr := httptest.NewRecorder() - - middleware := app.healthCheckMiddleware(handler) - middleware.ServeHTTP(rr, r) - - require.Equal(t, tc.status, rr.Code) - require.Equal(t, tc.body, rr.Body.String()) - }) - } -} - func TestHandlePanicMiddleware(t *testing.T) { next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { panic("on purpose") diff --git a/internal/healthcheck/middleware.go b/internal/healthcheck/middleware.go new file mode 100644 index 00000000..2ddd35b7 --- /dev/null +++ b/internal/healthcheck/middleware.go @@ -0,0 +1,19 @@ +package healthcheck + +import ( + "net/http" +) + +// NewMiddleware is serving the application status check +func NewMiddleware(handler http.Handler, statusPath string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == statusPath { + w.Header().Set("Cache-Control", "no-store") + w.Write([]byte("success\n")) + + return + } + + handler.ServeHTTP(w, r) + }) +} diff --git a/internal/healthcheck/middleware_test.go b/internal/healthcheck/middleware_test.go new file mode 100644 index 00000000..124bb0ff --- /dev/null +++ b/internal/healthcheck/middleware_test.go @@ -0,0 +1,50 @@ +package healthcheck_test + +import ( + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-pages/internal/config" + "gitlab.com/gitlab-org/gitlab-pages/internal/healthcheck" +) + +func TestHealthCheckMiddleware(t *testing.T) { + tests := map[string]struct { + path string + body string + }{ + "Not a healthcheck request": { + path: "/foo/bar", + body: "Hello from inner handler", + }, + "Healthcheck request": { + path: "/-/healthcheck", + body: "success\n", + }, + } + + cfg := config.Config{ + General: config.General{ + StatusPath: "/-/healthcheck", + }, + } + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + io.WriteString(w, "Hello from inner handler") + }) + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + middleware := healthcheck.NewMiddleware(handler, cfg.General.StatusPath) + + u := "https://example.com" + tc.path + + require.HTTPStatusCode(t, middleware.ServeHTTP, http.MethodGet, u, nil, http.StatusOK) + require.HTTPBodyContains(t, middleware.ServeHTTP, http.MethodGet, u, nil, tc.body) + }) + } +} |