diff --git a/README.md b/README.md index f6c4f06b..42059baa 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,10 @@ matches are exact: Excluded requests are still logged; only the Prometheus counters and in-flight gauge are skipped. +Paths are specified as the upstream receives them. Services deployed using +stripped path prefixes should specify their excluded paths in the un-prefixed +form. + ### Automatic TLS diff --git a/internal/server/router.go b/internal/server/router.go index 9f42c2a9..3b891b6a 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -35,6 +35,17 @@ func RoutingContext(r *http.Request) *routingContext { return rc } +func RoutedTargetPath(r *http.Request) string { + path := r.URL.Path + if rc := RoutingContext(r); rc != nil { + path = strings.TrimPrefix(path, rc.MatchedPrefix) + if path == "" { + path = rootPath + } + } + return path +} + type Router struct { statePath string services *ServiceMap diff --git a/internal/server/router_test.go b/internal/server/router_test.go index df7c484c..7a1a55c5 100644 --- a/internal/server/router_test.go +++ b/internal/server/router_test.go @@ -563,6 +563,38 @@ func TestRouter_PathBasedRoutingStripPrefix(t *testing.T) { assert.Equal(t, "/app", body) } +func TestRouter_HealthCheckWhilePausedWithPathPrefix(t *testing.T) { + router := testRouter(t) + _, backend := testBackend(t, "ok", http.StatusOK) + + serviceOptions := defaultServiceOptions + serviceOptions.PathPrefixes = []string{"/api"} + serviceOptions.StripPrefix = true + require.NoError(t, router.DeployService("service1", []string{backend}, defaultEmptyReaders, serviceOptions, defaultTargetOptions, defaultDeploymentOptions)) + + serviceOptions = defaultServiceOptions + serviceOptions.PathPrefixes = []string{"/admin"} + serviceOptions.StripPrefix = false + targetOptions := defaultTargetOptions + targetOptions.HealthCheckConfig.Path = "/admin/up" + require.NoError(t, router.DeployService("service2", []string{backend}, defaultEmptyReaders, serviceOptions, targetOptions, defaultDeploymentOptions)) + + require.NoError(t, router.PauseService("service1", time.Second, time.Millisecond*10)) + require.NoError(t, router.PauseService("service2", time.Second, time.Millisecond*10)) + + // Health checks succeed while paused, with the health check path matched + // against the target's view of the path + statusCode, _ := sendGETRequest(router, "http://example.com/api/up") + assert.Equal(t, http.StatusOK, statusCode) + + statusCode, _ = sendGETRequest(router, "http://example.com/admin/up") + assert.Equal(t, http.StatusOK, statusCode) + + // Other requests are still paused + statusCode, _ = sendGETRequest(router, "http://example.com/api/other") + assert.Equal(t, http.StatusGatewayTimeout, statusCode) +} + func TestRouter_PathBasedRoutingWithHosts(t *testing.T) { router := testRouter(t) _, first := testBackend(t, "first", http.StatusOK) diff --git a/internal/server/service.go b/internal/server/service.go index bb20edcb..1407f4c1 100644 --- a/internal/server/service.go +++ b/internal/server/service.go @@ -97,7 +97,7 @@ type ServiceOptions struct { } func (so *ServiceOptions) ShouldExcludeMetrics(r *http.Request) bool { - return slices.Contains(so.ExcludeMetricsPaths, r.URL.Path) + return slices.Contains(so.ExcludeMetricsPaths, RoutedTargetPath(r)) } func (so *ServiceOptions) Normalize() { diff --git a/internal/server/service_test.go b/internal/server/service_test.go index 0523c18f..8893a706 100644 --- a/internal/server/service_test.go +++ b/internal/server/service_test.go @@ -175,6 +175,11 @@ func TestServiceOptions_ShouldExcludeMetrics(t *testing.T) { assert.False(t, options.ShouldExcludeMetrics(httptest.NewRequest(http.MethodGet, "/api/users", nil))) assert.False(t, options.ShouldExcludeMetrics(httptest.NewRequest(http.MethodGet, "/up/nested", nil))) + // When a path prefix is due to be stripped, match against the target's view of the path + assert.True(t, options.ShouldExcludeMetrics(testRequestWithMatchedPrefix(httptest.NewRequest(http.MethodGet, "/api/up", nil), "/api"))) + assert.False(t, options.ShouldExcludeMetrics(testRequestWithMatchedPrefix(httptest.NewRequest(http.MethodGet, "/api/users", nil), "/api"))) + assert.False(t, options.ShouldExcludeMetrics(httptest.NewRequest(http.MethodGet, "/api/up", nil))) + empty := ServiceOptions{} assert.False(t, empty.ShouldExcludeMetrics(httptest.NewRequest(http.MethodGet, "/up", nil))) } diff --git a/internal/server/target.go b/internal/server/target.go index 09084a05..b1bd627b 100644 --- a/internal/server/target.go +++ b/internal/server/target.go @@ -12,7 +12,6 @@ import ( "net/http/httputil" "net/url" "regexp" - "strings" "sync" "time" ) @@ -77,7 +76,7 @@ type TargetOptions struct { } func (to *TargetOptions) IsHealthCheckRequest(r *http.Request) bool { - return (r.Method == http.MethodGet || r.Method == http.MethodHead) && r.URL.Path == to.HealthCheckConfig.Path + return (r.Method == http.MethodGet || r.Method == http.MethodHead) && RoutedTargetPath(r) == to.HealthCheckConfig.Path } func (to *TargetOptions) canonicalizeLogHeaders() { @@ -221,7 +220,8 @@ func (t *Target) BeginHealthChecks(stateConsumer TargetStateConsumer) { t.withInflightLock(func() { healthCheckURL := t.buildHealthCheckURL() - t.healthcheck = NewHealthCheck(t, + t.healthcheck = NewHealthCheck( + t, healthCheckURL, t.options.HealthCheckConfig.Interval, t.options.HealthCheckConfig.Timeout, @@ -310,10 +310,7 @@ func (t *Target) rewrite(req *httputil.ProxyRequest) { req.SetURL(t.targetURL) req.Out.Host = req.In.Host - routingContext := RoutingContext(req.In) - if routingContext != nil { - req.Out.URL.Path = strings.TrimPrefix(req.Out.URL.Path, routingContext.MatchedPrefix) - } + req.Out.URL.Path = RoutedTargetPath(req.In) // Ensure query params are preserved exactly, including those we could not // parse. diff --git a/internal/server/target_test.go b/internal/server/target_test.go index bfcaa4b8..dbcc99dd 100644 --- a/internal/server/target_test.go +++ b/internal/server/target_test.go @@ -285,6 +285,11 @@ func TestTarget_IsHealthCheckRequest(t *testing.T) { assert.False(t, target.options.IsHealthCheckRequest(httptest.NewRequest(http.MethodGet, "/up/other", nil))) assert.False(t, target.options.IsHealthCheckRequest(httptest.NewRequest(http.MethodGet, "/health", nil))) + + // When a path prefix is due to be stripped, match against the target's view of the path + assert.True(t, target.options.IsHealthCheckRequest(testRequestWithMatchedPrefix(httptest.NewRequest(http.MethodGet, "/api/up", nil), "/api"))) + assert.False(t, target.options.IsHealthCheckRequest(testRequestWithMatchedPrefix(httptest.NewRequest(http.MethodGet, "/api/health", nil), "/api"))) + assert.False(t, target.options.IsHealthCheckRequest(httptest.NewRequest(http.MethodGet, "/api/up", nil))) } func TestTarget_AddedTargetBecomesHealthy(t *testing.T) { diff --git a/internal/server/testing.go b/internal/server/testing.go index 15b11478..61576bf1 100644 --- a/internal/server/testing.go +++ b/internal/server/testing.go @@ -1,6 +1,7 @@ package server import ( + "context" "net/http" "net/http/httptest" "net/url" @@ -48,6 +49,11 @@ func testTargetWithOptions(t testing.TB, targetOptions TargetOptions, handler ht return target } +func testRequestWithMatchedPrefix(req *http.Request, prefix string) *http.Request { + ctx := context.WithValue(req.Context(), contextKeyRoutingContext, &routingContext{MatchedPrefix: prefix}) + return req.WithContext(ctx) +} + func testBackend(t testing.TB, body string, statusCode int) (*httptest.Server, string) { t.Helper()