From 10d853c0048b0d400864d31cd236e34e3aca8118 Mon Sep 17 00:00:00 2001 From: henrygd Date: Tue, 17 Feb 2026 16:12:29 -0500 Subject: [PATCH] heartbeat: tweaks and tests (#1729) --- internal/hub/heartbeat/heartbeat.go | 75 +++-- internal/hub/heartbeat/heartbeat_test.go | 258 ++++++++++++++++++ internal/hub/hub.go | 26 +- internal/hub/hub_test.go | 52 ++++ .../components/routes/settings/heartbeat.tsx | 30 +- 5 files changed, 385 insertions(+), 56 deletions(-) create mode 100644 internal/hub/heartbeat/heartbeat_test.go diff --git a/internal/hub/heartbeat/heartbeat.go b/internal/hub/heartbeat/heartbeat.go index f577529e..8f79c9db 100644 --- a/internal/hub/heartbeat/heartbeat.go +++ b/internal/hub/heartbeat/heartbeat.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "strconv" "strings" "time" @@ -26,13 +27,13 @@ const ( type Payload struct { // Status is "ok" when all non-paused systems are up, "warn" when alerts // are triggered but no systems are down, and "error" when any system is down. - Status string `json:"status"` - Timestamp string `json:"timestamp"` - Msg string `json:"msg"` - Systems SystemsSummary `json:"systems"` - Down []SystemInfo `json:"down_systems,omitempty"` - Alerts []AlertInfo `json:"triggered_alerts,omitempty"` - Version string `json:"beszel_version"` + Status string `json:"status"` + Timestamp string `json:"timestamp"` + Msg string `json:"msg"` + Systems SystemsSummary `json:"systems"` + Down []SystemInfo `json:"down_systems,omitempty"` + Alerts []AlertInfo `json:"triggered_alerts,omitempty"` + Version string `json:"beszel_version"` } // SystemsSummary contains counts of systems by status. @@ -76,8 +77,9 @@ type Heartbeat struct { // New creates a Heartbeat if configuration is present. // Returns nil if HEARTBEAT_URL is not set (feature disabled). func New(app core.App, getEnv func(string) (string, bool)) *Heartbeat { - url, ok := getEnv("HEARTBEAT_URL") - if !ok || url == "" { + url, _ := getEnv("HEARTBEAT_URL") + url = strings.TrimSpace(url) + if app == nil || url == "" { return nil } @@ -88,10 +90,10 @@ func New(app core.App, getEnv func(string) (string, bool)) *Heartbeat { } } - method := "POST" + method := http.MethodPost if v, ok := getEnv("HEARTBEAT_METHOD"); ok { v = strings.ToUpper(strings.TrimSpace(v)) - if v == "GET" || v == "HEAD" { + if v == http.MethodGet || v == http.MethodHead { method = v } } @@ -110,8 +112,9 @@ func New(app core.App, getEnv func(string) (string, bool)) *Heartbeat { // Start begins the heartbeat loop. It blocks and should be called in a goroutine. // The loop runs until the provided stop channel is closed. func (hb *Heartbeat) Start(stop <-chan struct{}) { + sanitizedURL := sanitizeHeartbeatURL(hb.config.URL) hb.app.Logger().Info("Heartbeat enabled", - "url", hb.config.URL, + "url", sanitizedURL, "interval", fmt.Sprintf("%ds", hb.config.Interval), "method", hb.config.Method, ) @@ -143,23 +146,25 @@ func (hb *Heartbeat) GetConfig() Config { } func (hb *Heartbeat) send() error { - payload, err := hb.buildPayload() - if err != nil { - hb.app.Logger().Error("Heartbeat: failed to build payload", "err", err) - return err - } - var req *http.Request + var err error + method := normalizeMethod(hb.config.Method) - if hb.config.Method == "GET" || hb.config.Method == "HEAD" { - req, err = http.NewRequest(hb.config.Method, hb.config.URL, nil) + if method == http.MethodGet || method == http.MethodHead { + req, err = http.NewRequest(method, hb.config.URL, nil) } else { + payload, payloadErr := hb.buildPayload() + if payloadErr != nil { + hb.app.Logger().Error("Heartbeat: failed to build payload", "err", payloadErr) + return payloadErr + } + body, jsonErr := json.Marshal(payload) if jsonErr != nil { hb.app.Logger().Error("Heartbeat: failed to marshal payload", "err", jsonErr) return jsonErr } - req, err = http.NewRequest("POST", hb.config.URL, bytes.NewReader(body)) + req, err = http.NewRequest(http.MethodPost, hb.config.URL, bytes.NewReader(body)) if err == nil { req.Header.Set("Content-Type", "application/json") } @@ -174,14 +179,14 @@ func (hb *Heartbeat) send() error { resp, err := hb.client.Do(req) if err != nil { - hb.app.Logger().Error("Heartbeat: request failed", "url", hb.config.URL, "err", err) + hb.app.Logger().Error("Heartbeat: request failed", "url", sanitizeHeartbeatURL(hb.config.URL), "err", err) return err } defer resp.Body.Close() if resp.StatusCode >= 400 { hb.app.Logger().Warn("Heartbeat: non-success response", - "url", hb.config.URL, + "url", sanitizeHeartbeatURL(hb.config.URL), "status", resp.StatusCode, ) return fmt.Errorf("heartbeat endpoint returned status %d", resp.StatusCode) @@ -220,9 +225,11 @@ func (hb *Heartbeat) buildPayload() (*Payload, error) { // Get names of down systems. var downSystems []SystemInfo - err = db.NewQuery("SELECT id, name, host FROM systems WHERE status = 'down'").All(&downSystems) - if err != nil { - return nil, fmt.Errorf("query down systems: %w", err) + if summary.Down > 0 { + err = db.NewQuery("SELECT id, name, host FROM systems WHERE status = 'down'").All(&downSystems) + if err != nil { + return nil, fmt.Errorf("query down systems: %w", err) + } } // Get triggered alerts with system names. @@ -278,3 +285,19 @@ func (hb *Heartbeat) buildPayload() (*Payload, error) { Version: beszel.Version, }, nil } + +func normalizeMethod(method string) string { + upper := strings.ToUpper(strings.TrimSpace(method)) + if upper == http.MethodGet || upper == http.MethodHead || upper == http.MethodPost { + return upper + } + return http.MethodPost +} + +func sanitizeHeartbeatURL(rawURL string) string { + parsed, err := url.Parse(strings.TrimSpace(rawURL)) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + return "" + } + return parsed.Scheme + "://" + parsed.Host +} diff --git a/internal/hub/heartbeat/heartbeat_test.go b/internal/hub/heartbeat/heartbeat_test.go new file mode 100644 index 00000000..d76bcfc5 --- /dev/null +++ b/internal/hub/heartbeat/heartbeat_test.go @@ -0,0 +1,258 @@ +//go:build testing +// +build testing + +package heartbeat_test + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/henrygd/beszel/internal/hub/heartbeat" + beszeltests "github.com/henrygd/beszel/internal/tests" + "github.com/pocketbase/pocketbase/core" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNew(t *testing.T) { + t.Run("returns nil when app is missing", func(t *testing.T) { + hb := heartbeat.New(nil, envGetter(map[string]string{ + "HEARTBEAT_URL": "https://heartbeat.example.com/ping", + })) + assert.Nil(t, hb) + }) + + t.Run("returns nil when URL is missing", func(t *testing.T) { + app := newTestHub(t) + hb := heartbeat.New(app.App, func(string) (string, bool) { + return "", false + }) + assert.Nil(t, hb) + }) + + t.Run("parses and normalizes config values", func(t *testing.T) { + app := newTestHub(t) + env := map[string]string{ + "HEARTBEAT_URL": " https://heartbeat.example.com/ping ", + "HEARTBEAT_INTERVAL": "90", + "HEARTBEAT_METHOD": "head", + } + getEnv := func(key string) (string, bool) { + v, ok := env[key] + return v, ok + } + + hb := heartbeat.New(app.App, getEnv) + require.NotNil(t, hb) + cfg := hb.GetConfig() + assert.Equal(t, "https://heartbeat.example.com/ping", cfg.URL) + assert.Equal(t, 90, cfg.Interval) + assert.Equal(t, http.MethodHead, cfg.Method) + }) +} + +func TestSendGETDoesNotRequireAppOrDB(t *testing.T) { + app := newTestHub(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "Beszel-Heartbeat", r.Header.Get("User-Agent")) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + hb := heartbeat.New(app.App, envGetter(map[string]string{ + "HEARTBEAT_URL": server.URL, + "HEARTBEAT_METHOD": "GET", + })) + require.NotNil(t, hb) + + require.NoError(t, hb.Send()) +} + +func TestSendReturnsErrorOnHTTPFailureStatus(t *testing.T) { + app := newTestHub(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + hb := heartbeat.New(app.App, envGetter(map[string]string{ + "HEARTBEAT_URL": server.URL, + "HEARTBEAT_METHOD": "GET", + })) + require.NotNil(t, hb) + + err := hb.Send() + require.Error(t, err) + assert.ErrorContains(t, err, "heartbeat endpoint returned status 500") +} + +func TestSendPOSTBuildsExpectedStatuses(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, app *beszeltests.TestHub, user *core.Record) + expectStatus string + expectMsgPart string + expectDown int + expectAlerts int + expectTotal int + expectUp int + expectPaused int + expectPending int + expectDownSumm int + }{ + { + name: "error when at least one system is down", + setup: func(t *testing.T, app *beszeltests.TestHub, user *core.Record) { + downSystem := createTestSystem(t, app, user.Id, "db-1", "10.0.0.1", "down") + _ = createTestSystem(t, app, user.Id, "web-1", "10.0.0.2", "up") + createTriggeredAlert(t, app, user.Id, downSystem.Id, "CPU", 95) + }, + expectStatus: "error", + expectMsgPart: "1 system(s) down", + expectDown: 1, + expectAlerts: 1, + expectTotal: 2, + expectUp: 1, + expectDownSumm: 1, + }, + { + name: "warn when only alerts are triggered", + setup: func(t *testing.T, app *beszeltests.TestHub, user *core.Record) { + system := createTestSystem(t, app, user.Id, "api-1", "10.1.0.1", "up") + createTriggeredAlert(t, app, user.Id, system.Id, "CPU", 90) + }, + expectStatus: "warn", + expectMsgPart: "1 alert(s) triggered", + expectDown: 0, + expectAlerts: 1, + expectTotal: 1, + expectUp: 1, + expectDownSumm: 0, + }, + { + name: "ok when no down systems and no alerts", + setup: func(t *testing.T, app *beszeltests.TestHub, user *core.Record) { + _ = createTestSystem(t, app, user.Id, "node-1", "10.2.0.1", "up") + _ = createTestSystem(t, app, user.Id, "node-2", "10.2.0.2", "paused") + _ = createTestSystem(t, app, user.Id, "node-3", "10.2.0.3", "pending") + }, + expectStatus: "ok", + expectMsgPart: "All systems operational", + expectDown: 0, + expectAlerts: 0, + expectTotal: 3, + expectUp: 1, + expectPaused: 1, + expectPending: 1, + expectDownSumm: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + app := newTestHub(t) + user := createTestUser(t, app) + tt.setup(t, app, user) + + type requestCapture struct { + method string + userAgent string + contentType string + payload heartbeat.Payload + } + + captured := make(chan requestCapture, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + + var payload heartbeat.Payload + require.NoError(t, json.Unmarshal(body, &payload)) + captured <- requestCapture{ + method: r.Method, + userAgent: r.Header.Get("User-Agent"), + contentType: r.Header.Get("Content-Type"), + payload: payload, + } + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + hb := heartbeat.New(app.App, envGetter(map[string]string{ + "HEARTBEAT_URL": server.URL, + "HEARTBEAT_METHOD": "POST", + })) + require.NotNil(t, hb) + require.NoError(t, hb.Send()) + + req := <-captured + assert.Equal(t, http.MethodPost, req.method) + assert.Equal(t, "Beszel-Heartbeat", req.userAgent) + assert.Equal(t, "application/json", req.contentType) + + assert.Equal(t, tt.expectStatus, req.payload.Status) + assert.Contains(t, req.payload.Msg, tt.expectMsgPart) + assert.Equal(t, tt.expectDown, len(req.payload.Down)) + assert.Equal(t, tt.expectAlerts, len(req.payload.Alerts)) + assert.Equal(t, tt.expectTotal, req.payload.Systems.Total) + assert.Equal(t, tt.expectUp, req.payload.Systems.Up) + assert.Equal(t, tt.expectDownSumm, req.payload.Systems.Down) + assert.Equal(t, tt.expectPaused, req.payload.Systems.Paused) + assert.Equal(t, tt.expectPending, req.payload.Systems.Pending) + }) + } +} + +func newTestHub(t *testing.T) *beszeltests.TestHub { + t.Helper() + app, err := beszeltests.NewTestHub(t.TempDir()) + require.NoError(t, err) + t.Cleanup(app.Cleanup) + return app +} + +func createTestUser(t *testing.T, app *beszeltests.TestHub) *core.Record { + t.Helper() + user, err := beszeltests.CreateUser(app.App, "admin@example.com", "password123") + require.NoError(t, err) + return user +} + +func createTestSystem(t *testing.T, app *beszeltests.TestHub, userID, name, host, status string) *core.Record { + t.Helper() + system, err := beszeltests.CreateRecord(app.App, "systems", map[string]any{ + "name": name, + "host": host, + "port": "45876", + "users": []string{userID}, + "status": status, + }) + require.NoError(t, err) + return system +} + +func createTriggeredAlert(t *testing.T, app *beszeltests.TestHub, userID, systemID, name string, threshold float64) *core.Record { + t.Helper() + alert, err := beszeltests.CreateRecord(app.App, "alerts", map[string]any{ + "name": name, + "system": systemID, + "user": userID, + "value": threshold, + "min": 0, + "triggered": true, + }) + require.NoError(t, err) + return alert +} + +func envGetter(values map[string]string) func(string) (string, bool) { + return func(key string) (string, bool) { + v, ok := values[key] + return v, ok + } +} diff --git a/internal/hub/hub.go b/internal/hub/hub.go index 70d96d93..bc58c76f 100644 --- a/internal/hub/hub.go +++ b/internal/hub/hub.go @@ -31,14 +31,14 @@ import ( type Hub struct { core.App *alerts.AlertManager - um *users.UserManager - rm *records.RecordManager - sm *systems.SystemManager - hb *heartbeat.Heartbeat - hbStop chan struct{} - pubKey string - signer ssh.Signer - appURL string + um *users.UserManager + rm *records.RecordManager + sm *systems.SystemManager + hb *heartbeat.Heartbeat + hbStop chan struct{} + pubKey string + signer ssh.Signer + appURL string } // NewHub creates a new Hub instance with default configuration @@ -419,10 +419,13 @@ func (h *Hub) getUniversalToken(e *core.RequestEvent) error { // getHeartbeatStatus returns current heartbeat configuration and whether it's enabled func (h *Hub) getHeartbeatStatus(e *core.RequestEvent) error { + if e.Auth.GetString("role") != "admin" { + return e.ForbiddenError("Requires admin role", nil) + } if h.hb == nil { return e.JSON(http.StatusOK, map[string]any{ "enabled": false, - "msg": "Set BESZEL_HUB_HEARTBEAT_URL to enable outbound heartbeat monitoring", + "msg": "Set HEARTBEAT_URL to enable outbound heartbeat monitoring", }) } cfg := h.hb.GetConfig() @@ -436,9 +439,12 @@ func (h *Hub) getHeartbeatStatus(e *core.RequestEvent) error { // testHeartbeat triggers a single heartbeat ping and returns the result func (h *Hub) testHeartbeat(e *core.RequestEvent) error { + if e.Auth.GetString("role") != "admin" { + return e.ForbiddenError("Requires admin role", nil) + } if h.hb == nil { return e.JSON(http.StatusOK, map[string]any{ - "err": "Heartbeat not configured. Set BESZEL_HUB_HEARTBEAT_URL environment variable.", + "err": "Heartbeat not configured. Set HEARTBEAT_URL environment variable.", }) } if err := h.hb.Send(); err != nil { diff --git a/internal/hub/hub_test.go b/internal/hub/hub_test.go index e4654dad..dafc1548 100644 --- a/internal/hub/hub_test.go +++ b/internal/hub/hub_test.go @@ -362,6 +362,58 @@ func TestApiRoutesAuthentication(t *testing.T) { ExpectedContent: []string{"test-system"}, TestAppFactory: testAppFactory, }, + { + Name: "GET /heartbeat-status - no auth should fail", + Method: http.MethodGet, + URL: "/api/beszel/heartbeat-status", + ExpectedStatus: 401, + ExpectedContent: []string{"requires valid"}, + TestAppFactory: testAppFactory, + }, + { + Name: "GET /heartbeat-status - with user auth should fail", + Method: http.MethodGet, + URL: "/api/beszel/heartbeat-status", + Headers: map[string]string{ + "Authorization": userToken, + }, + ExpectedStatus: 403, + ExpectedContent: []string{"Requires admin role"}, + TestAppFactory: testAppFactory, + }, + { + Name: "GET /heartbeat-status - with admin auth should succeed", + Method: http.MethodGet, + URL: "/api/beszel/heartbeat-status", + Headers: map[string]string{ + "Authorization": adminUserToken, + }, + ExpectedStatus: 200, + ExpectedContent: []string{`"enabled":false`}, + TestAppFactory: testAppFactory, + }, + { + Name: "POST /test-heartbeat - with user auth should fail", + Method: http.MethodPost, + URL: "/api/beszel/test-heartbeat", + Headers: map[string]string{ + "Authorization": userToken, + }, + ExpectedStatus: 403, + ExpectedContent: []string{"Requires admin role"}, + TestAppFactory: testAppFactory, + }, + { + Name: "POST /test-heartbeat - with admin auth should report disabled state", + Method: http.MethodPost, + URL: "/api/beszel/test-heartbeat", + Headers: map[string]string{ + "Authorization": adminUserToken, + }, + ExpectedStatus: 200, + ExpectedContent: []string{"Heartbeat not configured"}, + TestAppFactory: testAppFactory, + }, { Name: "GET /universal-token - no auth should fail", Method: http.MethodGet, diff --git a/internal/site/src/components/routes/settings/heartbeat.tsx b/internal/site/src/components/routes/settings/heartbeat.tsx index 00fc5af1..ade39369 100644 --- a/internal/site/src/components/routes/settings/heartbeat.tsx +++ b/internal/site/src/components/routes/settings/heartbeat.tsx @@ -136,7 +136,7 @@ export default function HeartbeatSettings() {
-

+

Payload format

@@ -155,30 +155,20 @@ export default function HeartbeatSettings() {

) : ( -
-

- Heartbeat monitoring is not configured. -

+
-

- Configuration -

Set the following environment variables on your Beszel hub to enable heartbeat monitoring:

-
+
+ - @@ -204,10 +194,10 @@ function ConfigItem({ label, value, mono }: { label: string; value: string; mono function EnvVarItem({ name, description, example }: { name: string; description: string; example: string }) { return ( -
- {name} -

{description}

-

+

+ {name} +

{description}

+

Example: {example}