Skip to content

Commit 2f15b04

Browse files
authored
op-service: check if TLS is enabled and move middleware (#14571)
* op-service: check if TLS is enabled * op-service: apply RPC middlewares globally * op-service: apply RPC middlewares before RPC but after health * op-service: test health before middleware
1 parent 3f56e4b commit 2f15b04

File tree

3 files changed

+56
-5
lines changed

3 files changed

+56
-5
lines changed

op-service/httputil/server.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func (s *HTTPServer) Start() error {
7474
},
7575
}
7676

77-
if s.config.tls != nil {
77+
if s.config.tls != nil && s.config.tls.CLIConfig.Enabled {
7878
srv.TLSConfig = s.config.tls.Config
7979
}
8080

op-service/rpc/handler.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -148,13 +148,9 @@ func (b *Handler) AddRPC(route string) error {
148148

149149
// default to 404 not-found
150150
handler = http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
151-
b.log.Info("oh no!")
152151
http.NotFound(writer, request)
153152
})
154153

155-
// Health endpoint is lowest priority.
156-
handler = b.newHealthMiddleware(handler)
157-
158154
// serve RPC on configured RPC path (but not on arbitrary paths)
159155
handler = b.newHttpRPCMiddleware(srv, handler)
160156

@@ -167,6 +163,10 @@ func (b *Handler) AddRPC(route string) error {
167163
for _, middleware := range b.middlewares {
168164
handler = middleware(handler)
169165
}
166+
167+
// Health endpoint applies before user middleware
168+
handler = b.newHealthMiddleware(handler)
169+
170170
b.rpcRoutes[route] = srv
171171

172172
b.mux.Handle(route+"/", http.StripPrefix(route+"/", handler))

op-service/rpc/server_test.go

+51
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,57 @@ func testServer(t *testing.T, endpoint string, appVersion string, namespace stri
114114
})
115115
}
116116

117+
// TestUserMiddlewareBeforeHealth tests that the health endpoint is always available, in front of user-middleware.
118+
func TestUserMiddlewareBeforeHealth(t *testing.T) {
119+
appVersion := "test"
120+
logger := testlog.Logger(t, log.LevelTrace)
121+
server := ServerFromConfig(&ServerConfig{
122+
HttpOptions: nil,
123+
RpcOptions: []Option{
124+
WithLogger(logger),
125+
WithWebsocketEnabled(),
126+
WithMiddleware(func(next http.Handler) http.Handler {
127+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
128+
w.WriteHeader(http.StatusTeapot)
129+
})
130+
}),
131+
},
132+
Host: "127.0.0.1",
133+
Port: 0,
134+
AppVersion: appVersion,
135+
})
136+
server.AddAPI(rpc.API{
137+
Namespace: "test",
138+
Service: new(testAPI),
139+
})
140+
require.NoError(t, server.Start(), "must start")
141+
142+
t.Cleanup(func() {
143+
err := server.Stop()
144+
if err != nil {
145+
panic(err)
146+
}
147+
})
148+
149+
t.Run("does not support other GET /foobar", func(t *testing.T) {
150+
res, err := http.Get(server.httpServer.HTTPEndpoint() + "/foobar")
151+
require.NoError(t, err)
152+
defer res.Body.Close()
153+
require.Equal(t, http.StatusTeapot, res.StatusCode)
154+
})
155+
156+
t.Run("supports GET /healthz", func(t *testing.T) {
157+
res, err := http.Get(server.httpServer.HTTPEndpoint() + "/healthz")
158+
require.NoError(t, err)
159+
defer res.Body.Close()
160+
require.Equal(t, http.StatusOK, res.StatusCode)
161+
body, err := io.ReadAll(res.Body)
162+
require.NoError(t, err)
163+
require.EqualValues(t, fmt.Sprintf("{\"version\":\"%s\"}\n", appVersion), string(body))
164+
})
165+
166+
}
167+
117168
func TestAuthServer(t *testing.T) {
118169
secret := [32]byte{0: 4}
119170
badSecret := [32]byte{0: 5}

0 commit comments

Comments
 (0)