Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 11 additions & 0 deletions internal/server/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions internal/server/router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/server/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
5 changes: 5 additions & 0 deletions internal/server/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
Expand Down
11 changes: 4 additions & 7 deletions internal/server/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"net/http/httputil"
"net/url"
"regexp"
"strings"
"sync"
"time"
)
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions internal/server/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions internal/server/testing.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package server

import (
"context"
"net/http"
"net/http/httptest"
"net/url"
Expand Down Expand Up @@ -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()

Expand Down
Loading