diff --git a/server/etcdserver/api/etcdhttp/health.go b/server/etcdserver/api/etcdhttp/health.go index 4d66987867c..20e8836c270 100644 --- a/server/etcdserver/api/etcdhttp/health.go +++ b/server/etcdserver/api/etcdhttp/health.go @@ -40,24 +40,25 @@ type ServerHealth interface { Leader() types.ID Range(context.Context, *pb.RangeRequest) (*pb.RangeResponse, error) Config() config.ServerConfig + AuthStore() auth.AuthStore } // HandleHealth registers metrics and health handlers. it checks health by using v3 range request // and its corresponding timeout. func HandleHealth(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) { - mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool) Health { + mux.Handle(PathHealth, NewHealthHandler(lg, func(ctx context.Context, excludedAlarms AlarmSet, serializable bool) Health { if h := checkAlarms(lg, srv, excludedAlarms); h.Health != "true" { return h } if h := checkLeader(lg, srv, serializable); h.Health != "true" { return h } - return checkAPI(lg, srv, serializable) + return checkAPI(ctx, lg, srv, serializable) })) } // NewHealthHandler handles '/health' requests. -func NewHealthHandler(lg *zap.Logger, hfunc func(excludedAlarms AlarmSet, Serializable bool) Health) http.HandlerFunc { +func NewHealthHandler(lg *zap.Logger, hfunc func(ctx context.Context, excludedAlarms AlarmSet, Serializable bool) Health) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { w.Header().Set("Allow", http.MethodGet) @@ -71,7 +72,7 @@ func NewHealthHandler(lg *zap.Logger, hfunc func(excludedAlarms AlarmSet, Serial // This is useful for probes attempting to validate the liveness of // the etcd process vs readiness of the cluster to serve requests. serializableFlag := getSerializableFlag(r) - h := hfunc(excludedAlarms, serializableFlag) + h := hfunc(r.Context(), excludedAlarms, serializableFlag) defer func() { if h.Health == "true" { healthSuccess.Inc() @@ -178,13 +179,14 @@ func checkLeader(lg *zap.Logger, srv ServerHealth, serializable bool) Health { return h } -func checkAPI(lg *zap.Logger, srv ServerHealth, serializable bool) Health { +func checkAPI(ctx context.Context, lg *zap.Logger, srv ServerHealth, serializable bool) Health { h := Health{Health: "true"} cfg := srv.Config() - ctx, cancel := context.WithTimeout(context.Background(), cfg.ReqTimeout()) - _, err := srv.Range(ctx, &pb.RangeRequest{KeysOnly: true, Limit: 1, Serializable: serializable}) + ctx = srv.AuthStore().WithRoot(ctx) + cctx, cancel := context.WithTimeout(ctx, cfg.ReqTimeout()) + _, err := srv.Range(cctx, &pb.RangeRequest{KeysOnly: true, Limit: 1, Serializable: serializable}) cancel() - if err != nil && err != auth.ErrUserEmpty && err != auth.ErrPermissionDenied { + if err != nil { h.Health = "false" h.Reason = fmt.Sprintf("RANGE ERROR:%s", err) lg.Warn("serving /health false; Range fails", zap.Error(err)) diff --git a/server/etcdserver/api/etcdhttp/health_test.go b/server/etcdserver/api/etcdhttp/health_test.go index e3f19c16128..1955ffef380 100644 --- a/server/etcdserver/api/etcdhttp/health_test.go +++ b/server/etcdserver/api/etcdhttp/health_test.go @@ -25,22 +25,26 @@ import ( "go.uber.org/zap/zaptest" + "go.etcd.io/raft/v3" + pb "go.etcd.io/etcd/api/v3/etcdserverpb" "go.etcd.io/etcd/client/pkg/v3/testutil" "go.etcd.io/etcd/client/pkg/v3/types" "go.etcd.io/etcd/server/v3/auth" "go.etcd.io/etcd/server/v3/config" "go.etcd.io/etcd/server/v3/etcdserver" - "go.etcd.io/raft/v3" + betesting "go.etcd.io/etcd/server/v3/storage/backend/testing" + "go.etcd.io/etcd/server/v3/storage/schema" ) type fakeHealthServer struct { fakeServer - health string - apiError error + health string + apiError error + authStore auth.AuthStore } -func (s *fakeHealthServer) Range(ctx context.Context, request *pb.RangeRequest) (*pb.RangeResponse, error) { +func (s *fakeHealthServer) Range(_ context.Context, _ *pb.RangeRequest) (*pb.RangeResponse, error) { return nil, s.apiError } @@ -54,12 +58,13 @@ func (s *fakeHealthServer) Leader() types.ID { } return types.ID(raft.None) } -func (s *fakeHealthServer) Do(ctx context.Context, r pb.Request) (etcdserver.Response, error) { +func (s *fakeHealthServer) Do(_ context.Context, _ pb.Request) (etcdserver.Response, error) { if s.health == "true" { return etcdserver.Response{}, nil } return etcdserver.Response{}, fmt.Errorf("fail health check") } +func (s *fakeHealthServer) AuthStore() auth.AuthStore { return s.authStore } func (s *fakeHealthServer) ClientCertAuthEnabled() bool { return false } func TestHealthHandler(t *testing.T) { @@ -123,20 +128,6 @@ func TestHealthHandler(t *testing.T) { expectStatusCode: http.StatusOK, expectHealth: "true", }, - { - name: "Healthy even if authentication failed", - healthCheckURL: "/health", - apiError: auth.ErrUserEmpty, - expectStatusCode: http.StatusOK, - expectHealth: "true", - }, - { - name: "Healthy even if authorization failed", - healthCheckURL: "/health", - apiError: auth.ErrPermissionDenied, - expectStatusCode: http.StatusOK, - expectHealth: "true", - }, { name: "Unhealthy if api is not available", healthCheckURL: "/health", @@ -149,10 +140,14 @@ func TestHealthHandler(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mux := http.NewServeMux() + lg := zaptest.NewLogger(t) + be, _ := betesting.NewDefaultTmpBackend(t) + defer betesting.Close(t, be) HandleHealth(zaptest.NewLogger(t), mux, &fakeHealthServer{ fakeServer: fakeServer{alarms: tt.alarms}, health: tt.expectHealth, apiError: tt.apiError, + authStore: auth.NewAuthStore(lg, schema.NewAuthBackend(lg, be), nil, 0), }) ts := httptest.NewServer(mux) defer ts.Close() diff --git a/server/proxy/grpcproxy/health.go b/server/proxy/grpcproxy/health.go index ec9781bfe27..456564f1047 100644 --- a/server/proxy/grpcproxy/health.go +++ b/server/proxy/grpcproxy/health.go @@ -32,7 +32,9 @@ func HandleHealth(lg *zap.Logger, mux *http.ServeMux, c *clientv3.Client) { if lg == nil { lg = zap.NewNop() } - mux.Handle(etcdhttp.PathHealth, etcdhttp.NewHealthHandler(lg, func(excludedAlarms etcdhttp.AlarmSet, serializable bool) etcdhttp.Health { return checkHealth(c) })) + mux.Handle(etcdhttp.PathHealth, etcdhttp.NewHealthHandler(lg, func(ctx context.Context, excludedAlarms etcdhttp.AlarmSet, serializable bool) etcdhttp.Health { + return checkHealth(c) + })) } // HandleProxyHealth registers health handler on '/proxy/health'. @@ -40,7 +42,9 @@ func HandleProxyHealth(lg *zap.Logger, mux *http.ServeMux, c *clientv3.Client) { if lg == nil { lg = zap.NewNop() } - mux.Handle(etcdhttp.PathProxyHealth, etcdhttp.NewHealthHandler(lg, func(excludedAlarms etcdhttp.AlarmSet, serializable bool) etcdhttp.Health { return checkProxyHealth(c) })) + mux.Handle(etcdhttp.PathProxyHealth, etcdhttp.NewHealthHandler(lg, func(ctx context.Context, excludedAlarms etcdhttp.AlarmSet, serializable bool) etcdhttp.Health { + return checkProxyHealth(c) + })) } func checkHealth(c *clientv3.Client) etcdhttp.Health {