Health check config collides between two targets sharing the same backend host:port on reconnect/reload

[gravis]

Describe the Bug

When two targets point at the same backend host:port but have different Expected Response Codes, a websocket reconnect / config reload cross-contaminates their health-check configuration: one target inherits the other's expected status code and is then reported unhealthy forever even though it responds correctly.

This is distinct from #240 (which reverts custom codes to the 2xx default). Here the code does not revert to 2xx — it is overwritten with the other target's custom code.

Setup that triggers it:

• Target A — expects 200 (e.g. GET /manifest.json) → backend 10.0.0.10:8123
• Target B — expects 401 (e.g. GET /api/mcp) → same backend 10.0.0.10:8123

On a clean start both load correctly and are healthy. After a websocket drop + reconnect, targets are re-registered (Replacing existing target with ID ...) and Target A ends up expecting 401 (Target B's code). Target A then logs unexpected status code: 200 (expected: 401) every health-check interval and shows Unhealthy, even though 200 is its correct/expected response.

A clean newt restart fixes it (both targets healthy again) — but the next reconnect can re-trigger it.

Environment

• OS Type & Version: NixOS (newt in a systemd-nspawn container)
• Pangolin Version: 1.20.0 (also observed under 1.19.4)
• Newt Version: 1.14.0
• Gerbil / Traefik / Olm: n/a

To Reproduce

1. Create two resources whose health-check targets share the same backend host:port, with different Expected Response Codes (e.g. Target A → 200, Target B → 401).
2. Start newt cleanly. Both log Target X initial status: healthy.
3. Cause a websocket reconnect (server restart, DNS blip, or brief network drop — enough for newt to log not connected then Websocket connected).
4. On reconnect newt logs Replacing existing target with ID A / ID B, then flips one target to initial status: unhealthy.
5. That target now logs Target A: unexpected status code: 200 (expected: 401) on every interval and stays Unhealthy until newt is restarted.

Actual log (reconnect race, IDs/IPs genericized)

14:42:01 ERROR Failed to send registration message: not connected
   ... (~35s of "not connected") ...
14:42:34 ERROR Failed to send message: not connected
14:42:37 INFO  Websocket connected
14:42:37 INFO  Target B initial status: healthy
14:42:37 INFO  Target A initial status: healthy      <- both healthy right after connect
14:42:37 INFO  Replacing existing target with ID A
14:42:37 INFO  Replacing existing target with ID B
14:42:37 INFO  Target B initial status: healthy
14:42:37 INFO  Target A initial status: unhealthy     <- flips after the "Replacing" step
14:42:37 WARN  Target A: unexpected status code: 200 (expected: 401)   <- inherited B's 401
14:43:07 WARN  Target A: unexpected status code: 200 (expected: 401)
   ... repeats every 30s until newt is restarted ...

The initial status: healthy → Replacing existing target → initial status: unhealthy sequence for Target A within the same second strongly suggests the re-registration keys health-check config by backend address, so the second target (B) sharing 10.0.0.10:8123 overwrites Target A's expected code.

Expected Behavior

On reconnect/reload, each target should retain its own configured Expected Response Code regardless of whether another target shares the same backend host:port.

Workaround

Restart newt for a clean reload (both targets load correct codes), or avoid two targets sharing an identical backend host:port for health checks (disable the health check on one, or differentiate backends). Both are temporary — the collision returns on the next reconnect.

[gravis]

Update after tracing this end-to-end in the Newt source: this is not a Newt bug — moving it to the server. Filed as fosrl/pangolin#3444.

Newt stores each health-check target's expected code by target ID (healthcheck.Config.Status) and applies whatever the server sends; it never derives, mutates, or keys the expected code by backend address. I confirmed the reconnect "Replacing existing target" path preserves each target's own expected code (reproduced with a unit test).

The real trigger is a server payload disagreement on reconnect: the newt/sync message carries the target's correct expected code (200), but the newt/wg/connect registration message that arrives right after carries the other co-located resource's code (401), and it overwrites the good value. Details + logs in pangolin#3444.

Separately, while investigating I did find a genuine (unrelated) data race in healthcheck.go: monitorTarget/performHealthCheck mutate a target's status fields with no lock while getAllTargetsUnsafe snapshots the whole struct — confirmed with -race. Happy to open a small PR for that if useful. Closing this issue in favor of pangolin#3444.

[gravis]

Reopening with the correct root cause — I was wrong twice above (it is neither a server-side payload bug nor a host:port keying bug). It is a client bug in handleConnect, and I've reproduced it with a unit test.

handleConnect decodes the newt/wg/connect payload into the persistent n.wgData:

json.Unmarshal(jsonData, &n.wgData)

Go's encoding/json merges a JSON array into an existing slice by position, and a JSON null is a no-op for a non-pointer int (healthcheck.Config.Status). On reconnect the health-check rows come back in a different order (the server's config query has no ORDER BY), so a target with hcStatus: null (no custom expected code) lands on the slice slot previously held by another target and inherits its stale expected code — e.g. a /manifest.json target that should accept any 2xx inherits a sibling's 401 and is then reported Unhealthy forever until restart.

handleSync is immune because it decodes into a fresh local SyncData; clean boot is immune because wgData starts empty. Fix + regression test in #413 (decode into a fresh WgData, assign on success).

[AstralDestiny]

Are the repro steps different or just the final root cause different?

[gravis]

@AstralDestiny The repro steps remain the same, the root cause is different. Sorry for the confusion.

[gravis]

And we started seeing this only after switching to Newt 1.14.0 by the way.