From b8a705d9c2aefcb0089fc1484e09e3c2399577ea Mon Sep 17 00:00:00 2001 From: Brad Davidson Date: Fri, 18 Apr 2025 07:55:58 +0000 Subject: [PATCH] Fix handler panic when bootstrapper returned empty peer list Panic gets rescued by the http server, and was only visible when running in debug mode, but should be handled properly. Signed-off-by: Brad Davidson --- pkg/agent/https/https.go | 2 +- pkg/server/auth/auth.go | 14 +++++++++----- pkg/server/handlers/router.go | 2 +- pkg/spegel/bootstrap.go | 2 +- pkg/spegel/spegel.go | 7 ++++++- 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/agent/https/https.go b/pkg/agent/https/https.go index c531716b53a..a688cca00dd 100644 --- a/pkg/agent/https/https.go +++ b/pkg/agent/https/https.go @@ -56,7 +56,7 @@ func Start(ctx context.Context, nodeConfig *config.Node, runtime *config.Control runtime.Handler = router } - router.Use(auth.Delegated(nodeConfig.AgentConfig.ClientCA, nodeConfig.AgentConfig.KubeConfigKubelet, config)) + router.Use(auth.RequestInfo(), auth.Delegated(nodeConfig.AgentConfig.ClientCA, nodeConfig.AgentConfig.KubeConfigKubelet, config)) if config.SecureServing != nil { _, _, err = config.SecureServing.Serve(router, 0, ctx.Done()) diff --git a/pkg/server/auth/auth.go b/pkg/server/auth/auth.go index 17994aa04b0..c125908c26b 100644 --- a/pkg/server/auth/auth.go +++ b/pkg/server/auth/auth.go @@ -124,19 +124,23 @@ func Delegated(clientCA, kubeConfig string, config *server.Config) mux.Middlewar return func(handler http.Handler) http.Handler { handler = genericapifilters.WithAuthorization(handler, config.Authorization.Authorizer, scheme.Codecs) handler = genericapifilters.WithAuthentication(handler, config.Authentication.Authenticator, failedHandler, nil, nil) - handler = genericapifilters.WithRequestInfo(handler, requestInfoResolver) handler = genericapifilters.WithCacheControl(handler) return handler } } +// RequestInfo returns a middleware function that adds verb/resource/gvk/etc info to the request context. +// This must be set for other filters to function, but only needs to be in each middleware chain once. +func RequestInfo() mux.MiddlewareFunc { + return func(handler http.Handler) http.Handler { + return genericapifilters.WithRequestInfo(handler, requestInfoResolver) + } +} + // MaxInFlight returns a middleware function that limits the number of requests that are executed concurrently. // This is not strictly auth related, but it also uses the core Kubernetes request filters. func MaxInFlight(nonMutatingLimit, mutatingLimit int) mux.MiddlewareFunc { return func(handler http.Handler) http.Handler { - handler = genericfilters.WithMaxInFlightLimit(handler, nonMutatingLimit, mutatingLimit, nil) - handler = genericapifilters.WithRequestInfo(handler, requestInfoResolver) - handler = genericapifilters.WithCacheControl(handler) - return handler + return genericfilters.WithMaxInFlightLimit(handler, nonMutatingLimit, mutatingLimit, nil) } } diff --git a/pkg/server/handlers/router.go b/pkg/server/handlers/router.go index a787aa08f93..45abb7598dc 100644 --- a/pkg/server/handlers/router.go +++ b/pkg/server/handlers/router.go @@ -33,7 +33,7 @@ func NewHandler(ctx context.Context, control *config.Control, cfg *cmds.Server) prefix := "/v1-{program}" authed := mux.NewRouter().SkipClean(true) authed.NotFoundHandler = APIServer(control, cfg) - authed.Use(auth.HasRole(control, version.Program+":agent", user.NodesGroup, bootstrapapi.BootstrapDefaultGroup), auth.MaxInFlight(maxNonMutatingAgentRequests, maxMutatingAgentRequests)) + authed.Use(auth.HasRole(control, version.Program+":agent", user.NodesGroup, bootstrapapi.BootstrapDefaultGroup), auth.RequestInfo(), auth.MaxInFlight(maxNonMutatingAgentRequests, maxMutatingAgentRequests)) authed.Handle(prefix+"/serving-kubelet.crt", ServingKubeletCert(control, nodeAuth)) authed.Handle(prefix+"/client-kubelet.crt", ClientKubeletCert(control, nodeAuth)) authed.Handle(prefix+"/client-kube-proxy.crt", ClientKubeProxyCert(control)) diff --git a/pkg/spegel/bootstrap.go b/pkg/spegel/bootstrap.go index ef9765ca0c7..c25ed0dfdeb 100644 --- a/pkg/spegel/bootstrap.go +++ b/pkg/spegel/bootstrap.go @@ -241,7 +241,7 @@ func (c *chainingBootstrapper) Get(ctx context.Context) ([]peer.AddrInfo, error) as, err := b.Get(ctx) if err != nil { errs = append(errs, err) - } else { + } else if len(as) != 0 { return as, nil } } diff --git a/pkg/spegel/spegel.go b/pkg/spegel/spegel.go index f657827d1a3..016f95eb5fb 100644 --- a/pkg/spegel/spegel.go +++ b/pkg/spegel/spegel.go @@ -256,7 +256,7 @@ func (c *Config) Start(ctx context.Context, nodeConfig *config.Node) error { func (c *Config) peerInfo() http.HandlerFunc { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { info, err := c.Bootstrapper.Get(req.Context()) - if err != nil || len(info) == 0 { + if err != nil { http.Error(resp, err.Error(), http.StatusInternalServerError) return } @@ -268,6 +268,11 @@ func (c *Config) peerInfo() http.HandlerFunc { } } + if len(addrs) == 0 { + http.Error(resp, "no peer addresses available", http.StatusServiceUnavailable) + return + } + client, _, _ := net.SplitHostPort(req.RemoteAddr) if req.Header.Get("Accept") == "application/json" { b, err := json.Marshal(addrs)