From 4e84900cf861800fc71beee0366f33b42238b880 Mon Sep 17 00:00:00 2001 From: protolambda Date: Thu, 27 Feb 2025 23:52:43 +0100 Subject: [PATCH 1/4] op-service: check if TLS is enabled --- op-service/httputil/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-service/httputil/server.go b/op-service/httputil/server.go index 727201bc7cc44..b17d5f8017510 100644 --- a/op-service/httputil/server.go +++ b/op-service/httputil/server.go @@ -74,7 +74,7 @@ func (s *HTTPServer) Start() error { }, } - if s.config.tls != nil { + if s.config.tls != nil && s.config.tls.CLIConfig.Enabled { srv.TLSConfig = s.config.tls.Config } From e1f68841e409976f6660498481a50eb4b8b3e454 Mon Sep 17 00:00:00 2001 From: protolambda Date: Fri, 28 Feb 2025 01:44:49 +0100 Subject: [PATCH 2/4] op-service: apply RPC middlewares globally --- op-service/rpc/handler.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/op-service/rpc/handler.go b/op-service/rpc/handler.go index 40563a25ad329..a4469d11c6de2 100644 --- a/op-service/rpc/handler.go +++ b/op-service/rpc/handler.go @@ -72,6 +72,10 @@ func NewHandler(appVersion string, opts ...Option) *Handler { var handler http.Handler handler = bs.mux + // Apply user middlewares globally + for _, middleware := range bs.middlewares { + handler = middleware(handler) + } // Outer-most middlewares: logging, metrics, TLS handler = optls.NewPeerTLSMiddleware(handler) handler = opmetrics.NewHTTPRecordingMiddleware(bs.httpRecorder, handler) @@ -163,10 +167,6 @@ func (b *Handler) AddRPC(route string) error { handler = b.newWsMiddleWare(srv, handler) } - // Apply user middlewares - for _, middleware := range b.middlewares { - handler = middleware(handler) - } b.rpcRoutes[route] = srv b.mux.Handle(route+"/", http.StripPrefix(route+"/", handler)) From d0b744ae17badc83d7e67a67cee28bd1a31d7059 Mon Sep 17 00:00:00 2001 From: protolambda Date: Fri, 28 Feb 2025 02:06:02 +0100 Subject: [PATCH 3/4] op-service: apply RPC middlewares before RPC but after health --- op-service/rpc/handler.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/op-service/rpc/handler.go b/op-service/rpc/handler.go index a4469d11c6de2..361dce26f1d64 100644 --- a/op-service/rpc/handler.go +++ b/op-service/rpc/handler.go @@ -72,10 +72,6 @@ func NewHandler(appVersion string, opts ...Option) *Handler { var handler http.Handler handler = bs.mux - // Apply user middlewares globally - for _, middleware := range bs.middlewares { - handler = middleware(handler) - } // Outer-most middlewares: logging, metrics, TLS handler = optls.NewPeerTLSMiddleware(handler) handler = opmetrics.NewHTTPRecordingMiddleware(bs.httpRecorder, handler) @@ -152,13 +148,9 @@ func (b *Handler) AddRPC(route string) error { // default to 404 not-found handler = http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { - b.log.Info("oh no!") http.NotFound(writer, request) }) - // Health endpoint is lowest priority. - handler = b.newHealthMiddleware(handler) - // serve RPC on configured RPC path (but not on arbitrary paths) handler = b.newHttpRPCMiddleware(srv, handler) @@ -167,6 +159,14 @@ func (b *Handler) AddRPC(route string) error { handler = b.newWsMiddleWare(srv, handler) } + // Apply user middlewares + for _, middleware := range b.middlewares { + handler = middleware(handler) + } + + // Health endpoint applies before user middleware + handler = b.newHealthMiddleware(handler) + b.rpcRoutes[route] = srv b.mux.Handle(route+"/", http.StripPrefix(route+"/", handler)) From e453fdff5b4fb2c1d283ec4f026e371ba35e7012 Mon Sep 17 00:00:00 2001 From: protolambda Date: Fri, 28 Feb 2025 02:14:26 +0100 Subject: [PATCH 4/4] op-service: test health before middleware --- op-service/rpc/server_test.go | 51 +++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/op-service/rpc/server_test.go b/op-service/rpc/server_test.go index 900ebbba26d52..8774f93642283 100644 --- a/op-service/rpc/server_test.go +++ b/op-service/rpc/server_test.go @@ -114,6 +114,57 @@ func testServer(t *testing.T, endpoint string, appVersion string, namespace stri }) } +// TestUserMiddlewareBeforeHealth tests that the health endpoint is always available, in front of user-middleware. +func TestUserMiddlewareBeforeHealth(t *testing.T) { + appVersion := "test" + logger := testlog.Logger(t, log.LevelTrace) + server := ServerFromConfig(&ServerConfig{ + HttpOptions: nil, + RpcOptions: []Option{ + WithLogger(logger), + WithWebsocketEnabled(), + WithMiddleware(func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusTeapot) + }) + }), + }, + Host: "127.0.0.1", + Port: 0, + AppVersion: appVersion, + }) + server.AddAPI(rpc.API{ + Namespace: "test", + Service: new(testAPI), + }) + require.NoError(t, server.Start(), "must start") + + t.Cleanup(func() { + err := server.Stop() + if err != nil { + panic(err) + } + }) + + t.Run("does not support other GET /foobar", func(t *testing.T) { + res, err := http.Get(server.httpServer.HTTPEndpoint() + "/foobar") + require.NoError(t, err) + defer res.Body.Close() + require.Equal(t, http.StatusTeapot, res.StatusCode) + }) + + t.Run("supports GET /healthz", func(t *testing.T) { + res, err := http.Get(server.httpServer.HTTPEndpoint() + "/healthz") + require.NoError(t, err) + defer res.Body.Close() + require.Equal(t, http.StatusOK, res.StatusCode) + body, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.EqualValues(t, fmt.Sprintf("{\"version\":\"%s\"}\n", appVersion), string(body)) + }) + +} + func TestAuthServer(t *testing.T) { secret := [32]byte{0: 4} badSecret := [32]byte{0: 5}