🛡️ Add SafeGo() panic recovery wrapper for background goroutines#13302
Conversation
Add pkg/safego package with Go() and GoWith() helpers that wrap goroutines with defer/recover to prevent panics from crashing the entire process. Migrate all unprotected go func() calls across pkg/ and cmd/ to use these helpers. Changes: - New: pkg/safego/safego.go — Go() and GoWith() helpers - New: pkg/safego/safego_test.go — unit tests for all helpers - Migrated: 42 files with 102 unprotected goroutines Impact: - Before: A panic in any background goroutine crashes the entire console process - After: Panics are logged with full stack traces and isolated — process continues serving Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <copilot@github.com>
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole canceled.
|
kubestellar-prow
Bot
added
the
size/XL
|
👋 Hey @kubestellar-hive[bot] — thanks for opening this PR!
This is an automated message. |
![kubestellar-hive[bot]](https://avatars.githubusercontent.com/u/134407106?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Security Review — PR #13302
Verdict PASS — No security issues:
pkg/safego/safego.go — Clean & correct
Go()andGoWith()correctly usedefer recover()+debug.Stack()+slog.Error- No panics can escape; no silent swallowing (stack is always logged)
- Tests cover both normal execution and panic recovery paths
Variable capture — Safe
Go 1.26.3 (go.mod) has per-iteration loop variable scoping (Go 1.22+), so all the cl := cluster / idx := i captures are technically redundant but harmless — no correctness issue.
One missed bare goroutine
cmd/watcher/watcher.go:255 still has a bare go handleConn(...) inside the accept loop. If handleConn panics, it crashes the process. Low risk (handleConn is simple TLS routing), but should be wrapped for consistency. Non-blocking — can be a follow-up.
Signal handler wrapping — Acceptable
Signal handlers in cmd/console/main.go and cmd/kc-agent/main.go are now wrapped. If they panic, the process continues without shutdown capability. Acceptable because signal handlers are trivially simple and unlikely to panic.
CI
All checks green (build, go test, CodeQL, startup smoke, full-stack E2E).
No security concerns. Clean mechanical migration.
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
Complete the safego migration started in #13302 — the original PR missed 18 go func() sites across 10 handler files. These goroutines had no panic recovery, meaning a single panic could crash the entire console process. Affected files: - benchmarks.go (5 goroutines) - websocket.go (3 goroutines) - nightly_e2e.go (2 goroutines) - console_persistence.go (2 goroutines) - gitops_operators.go, gitops.go, mcp_workloads.go, mcp_resources.go, timeline.go, mcp_query.go (1 each) Signed-off-by: Copilot <copilot@github.com> Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Migrate remaining bare go func()/go method() calls to use safego.GoWith() for consistent panic recovery. Continues safego migration from PRs #13302, #13306, #13343. Affected packages: agent, api, mcp, watcher. Signed-off-by: Copilot <copilot@github.com> Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes #13301
Summary
Adds a
safegopackage (pkg/safego/safego.go) with panic recovery wrappers and migrates all 102 unprotected background goroutines acrosspkg/andcmd/to use them.New helpers
safego.Go(fn)safego.GoWith(label, fn)Migration scope
cmd/console,cmd/kc-agent,cmd/watcher,pkg/agent,pkg/api,pkg/k8s,pkg/mcprecover()were left untoucheddefer wg.Done()orderingImpact
Testing
pkg/safego/safego_test.go(normal execution + panic recovery for both helpers)go build ./...passesgo test ./pkg/safego/...passes