diff --git a/pkg/checks/checkhttp.go b/pkg/checks/checkhttp.go index d40e272d9c..9293a2c108 100644 --- a/pkg/checks/checkhttp.go +++ b/pkg/checks/checkhttp.go @@ -3,16 +3,22 @@ package checks import ( "context" "fmt" + "net" "net/http" + "net/netip" "strings" "time" "github.com/opencloud-eu/opencloud/pkg/handlers" ) -// NewHTTPCheck checks the reachability of a http server. +// HealthCheckUserAgent is the user agent of the requests NewHTTPCheck sends. +const HealthCheckUserAgent = "opencloud-health-check" + +// NewHTTPCheck checks the reachability of a http server. Any answer counts, +// a server that doesn't answer within the timeout fails the check. func NewHTTPCheck(url string) func(context.Context) error { - return func(_ context.Context) error { + return func(ctx context.Context) error { url, err := handlers.FailSaveAddress(url) if err != nil { return err @@ -22,10 +28,16 @@ func NewHTTPCheck(url string) func(context.Context) error { url = "http://" + url } + req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) + if err != nil { + return err + } + req.Header.Set("User-Agent", HealthCheckUserAgent) + c := http.Client{ Timeout: 3 * time.Second, } - resp, err := c.Get(url) + resp, err := c.Do(req) if err != nil { return fmt.Errorf("could not connect to http server: %v", err) } @@ -33,3 +45,18 @@ func NewHTTPCheck(url string) func(context.Context) error { return nil } } + +// IsLocalHealthCheck reports whether r is a request of NewHTTPCheck, so an +// access log can leave it out. The user agent alone would let any client hide +// its requests, so the request must also come from the loopback address. +func IsLocalHealthCheck(r *http.Request) bool { + if r.UserAgent() != HealthCheckUserAgent { + return false + } + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return false + } + a, err := netip.ParseAddr(host) + return err == nil && a.Unmap().IsLoopback() +} diff --git a/pkg/checks/checkhttp_test.go b/pkg/checks/checkhttp_test.go new file mode 100644 index 0000000000..735d7ddb56 --- /dev/null +++ b/pkg/checks/checkhttp_test.go @@ -0,0 +1,42 @@ +package checks + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewHTTPCheck(t *testing.T) { + var got *http.Request + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r + w.WriteHeader(http.StatusBadGateway) + })) + defer srv.Close() + + require.NoError(t, NewHTTPCheck(srv.URL)(context.Background()), "any answer counts") + assert.Equal(t, http.MethodHead, got.Method) + assert.True(t, IsLocalHealthCheck(got)) +} + +func TestIsLocalHealthCheck(t *testing.T) { + for name, tc := range map[string]struct { + remoteAddr, userAgent string + want bool + }{ + "check from ipv4 loopback": {"127.0.0.1:51234", HealthCheckUserAgent, true}, + "check from ipv6 loopback": {"[::1]:51234", HealthCheckUserAgent, true}, + "client sending the agent": {"203.0.113.7:51234", HealthCheckUserAgent, false}, + "proxy in front": {"10.0.0.5:51234", HealthCheckUserAgent, false}, + "local browser": {"127.0.0.1:51234", "Mozilla/5.0", false}, + } { + r := httptest.NewRequest(http.MethodHead, "/", nil) + r.RemoteAddr = tc.remoteAddr + r.Header.Set("User-Agent", tc.userAgent) + assert.Equal(t, tc.want, IsLocalHealthCheck(r), name) + } +} diff --git a/services/collaboration/pkg/middleware/accesslog.go b/services/collaboration/pkg/middleware/accesslog.go index 8ebd05a1b5..fd89cc4d31 100644 --- a/services/collaboration/pkg/middleware/accesslog.go +++ b/services/collaboration/pkg/middleware/accesslog.go @@ -5,6 +5,7 @@ import ( "time" "github.com/go-chi/chi/v5/middleware" + "github.com/opencloud-eu/opencloud/pkg/checks" "github.com/opencloud-eu/opencloud/pkg/log" ocmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware" "go.opentelemetry.io/otel/trace" @@ -21,6 +22,10 @@ func AccessLog(logger log.Logger) func(http.Handler) http.Handler { wrap := middleware.NewWrapResponseWriter(w, r.ProtoMajor) next.ServeHTTP(wrap, r) + if checks.IsLocalHealthCheck(r) { + return + } + spanContext := trace.SpanContextFromContext(r.Context()) logger.Info(). Str("proto", r.Proto). diff --git a/services/proxy/pkg/middleware/accesslog.go b/services/proxy/pkg/middleware/accesslog.go index 2638ff2178..00695229d1 100644 --- a/services/proxy/pkg/middleware/accesslog.go +++ b/services/proxy/pkg/middleware/accesslog.go @@ -5,6 +5,7 @@ import ( "time" "github.com/go-chi/chi/v5/middleware" + "github.com/opencloud-eu/opencloud/pkg/checks" "github.com/opencloud-eu/opencloud/pkg/log" ocmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware" "go.opentelemetry.io/otel/trace" @@ -21,6 +22,10 @@ func AccessLog(logger log.Logger) func(http.Handler) http.Handler { wrap := middleware.NewWrapResponseWriter(w, r.ProtoMajor) next.ServeHTTP(wrap, r) + if checks.IsLocalHealthCheck(r) { + return + } + spanContext := trace.SpanContextFromContext(r.Context()) logger.Info(). Str("proto", r.Proto).