diff --git a/cmd/spinifex/cmd/templates/nats.conf b/cmd/spinifex/cmd/templates/nats.conf index 7baec86ae..267d4f4de 100644 --- a/cmd/spinifex/cmd/templates/nats.conf +++ b/cmd/spinifex/cmd/templates/nats.conf @@ -1,4 +1,4 @@ -# spinifex-config-version: 1 +# spinifex-config-version: 2 # NATS Server Configuration # Generated by Spinifex admin init @@ -49,9 +49,5 @@ max_payload: 1MB max_pending: 64MB # Monitoring -{{- if .BindIP }} -http: {{.BindIP}}:8222 -{{- else }} -http_port: 8222 -{{- end }} +http: 127.0.0.1:8222 diff --git a/spinifex/daemon/daemon_handlers.go b/spinifex/daemon/daemon_handlers.go index a0bfafcb9..558db5633 100644 --- a/spinifex/daemon/daemon_handlers.go +++ b/spinifex/daemon/daemon_handlers.go @@ -207,7 +207,7 @@ func (d *Daemon) queryNATSRole() string { if !d.config.HasService("nats") { return "" } - url := "http://" + net.JoinHostPort(d.daemonIP(), strconv.Itoa(natsMonitorPort)) + "/varz" + url := "http://" + net.JoinHostPort("127.0.0.1", strconv.Itoa(natsMonitorPort)) + "/varz" return fetchNATSRole(url, roleHTTPClient) } diff --git a/spinifex/migrate/003_nats_monitor_localhost.go b/spinifex/migrate/003_nats_monitor_localhost.go new file mode 100644 index 000000000..a2c545653 --- /dev/null +++ b/spinifex/migrate/003_nats_monitor_localhost.go @@ -0,0 +1,51 @@ +package migrate + +import ( + "fmt" + "os" + "path/filepath" + "regexp" +) + +func init() { + DefaultRegistry.RegisterConfig("nats.conf", ConfigMigration{ + FromVersion: 1, + ToVersion: 2, + Description: "Bind NATS monitoring to localhost only", + Run: func(ctx ConfigContext) error { + path := filepath.Join(ctx.ConfigDir, "nats", "nats.conf") + content, err := os.ReadFile(path) + if err != nil { + return err + } + + // Match "http: :8222" or "http_port: 8222" + httpRe := regexp.MustCompile(`(?m)^http(?:_port)?:\s*.*8222\s*$`) + if !httpRe.Match(content) { + ctx.Logger.Info("nats.conf: no monitoring line found, skipping") + return nil + } + + updated := httpRe.ReplaceAll(content, []byte("http: 127.0.0.1:8222")) + + info, err := os.Stat(path) + if err != nil { + return err + } + if err := os.WriteFile(path, updated, info.Mode()); err != nil { + return err + } + + // Post-migration validation. + result, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("post-migration validation read failed: %w", err) + } + localhostRe := regexp.MustCompile(`(?m)^http:\s*127\.0\.0\.1:8222\s*$`) + if !localhostRe.Match(result) { + return fmt.Errorf("monitoring line not rewritten to localhost after migration") + } + return nil + }, + }) +} diff --git a/spinifex/migrate/migrate_test.go b/spinifex/migrate/migrate_test.go index cd470bad5..c9833a1a5 100644 --- a/spinifex/migrate/migrate_test.go +++ b/spinifex/migrate/migrate_test.go @@ -712,6 +712,67 @@ listen: 0.0.0.0:4222 assert.Equal(t, 2, v) } +// --- NATS monitoring localhost migration tests --- + +func TestNATSMonitorLocalhost(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + "BindIP 0.0.0.0", + "# spinifex-config-version: 1\n# NATS\nhttp: 0.0.0.0:8222\n", + "http: 127.0.0.1:8222", + }, + { + "BindIP specific IP", + "# spinifex-config-version: 1\n# NATS\nhttp: 10.0.0.1:8222\n", + "http: 127.0.0.1:8222", + }, + { + "no BindIP (http_port)", + "# spinifex-config-version: 1\n# NATS\nhttp_port: 8222\n", + "http: 127.0.0.1:8222", + }, + { + "already migrated (idempotent)", + "# spinifex-config-version: 1\n# NATS\nhttp: 127.0.0.1:8222\n", + "http: 127.0.0.1:8222", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + natsDir := filepath.Join(dir, "nats") + require.NoError(t, os.MkdirAll(natsDir, 0o755)) + confPath := filepath.Join(natsDir, "nats.conf") + require.NoError(t, os.WriteFile(confPath, []byte(tt.input), 0o640)) + + r := NewRegistry() + r.RegisterConfigTarget("nats.conf", "nats/nats.conf", &NATSConfVersionReader{}) + r.RegisterConfig("nats.conf", ConfigMigration{ + FromVersion: 1, + ToVersion: 2, + Description: "Bind NATS monitoring to localhost only", + Run: DefaultRegistry.configMigrations["nats.conf"][1].Run, + }) + + err := r.RunConfig("nats.conf", dir, dir) + require.NoError(t, err) + + data, err := os.ReadFile(confPath) + require.NoError(t, err) + assert.Contains(t, string(data), tt.expected) + // Verify no leftover non-localhost monitoring lines + assert.NotContains(t, string(data), "0.0.0.0:8222") + assert.NotContains(t, string(data), "10.0.0.1:8222") + assert.NotContains(t, string(data), "http_port:") + }) + } +} + func TestRunConfig_RejectsChainGap(t *testing.T) { dir := t.TempDir() natsDir := filepath.Join(dir, "nats") diff --git a/spinifex/services/nats/nats_test.go b/spinifex/services/nats/nats_test.go index c84e20969..23d0a0765 100644 --- a/spinifex/services/nats/nats_test.go +++ b/spinifex/services/nats/nats_test.go @@ -98,6 +98,6 @@ func TestRenderedConfig_HasMigrationVersionMarker(t *testing.T) { require.NoError(t, err) firstLine := strings.SplitN(string(raw), "\n", 2)[0] - assert.Equal(t, "# spinifex-config-version: 1", firstLine, + assert.Equal(t, "# spinifex-config-version: 2", firstLine, "nats.conf template must start with the current migration version marker") } diff --git a/tests/e2e/lib/multinode-helpers.sh b/tests/e2e/lib/multinode-helpers.sh index 4613fbc6b..7b5a32271 100755 --- a/tests/e2e/lib/multinode-helpers.sh +++ b/tests/e2e/lib/multinode-helpers.sh @@ -120,7 +120,7 @@ verify_nats_cluster() { # Check cluster info via monitoring endpoint on node1 local cluster_info - cluster_info=$(curl -s "http://${NODE1_IP}:${NATS_MONITOR_PORT}/routez" 2>/dev/null) || { + cluster_info=$(curl -s "http://127.0.0.1:${NATS_MONITOR_PORT}/routez" 2>/dev/null) || { echo " ERROR: Cannot reach NATS monitoring endpoint" return 1 } @@ -645,8 +645,8 @@ verify_all_services_down() { all_down=false fi - # Check NATS - if curl -s --connect-timeout 2 "http://${node_ip}:${NATS_MONITOR_PORT}" > /dev/null 2>&1; then + # Check NATS (client port — monitoring is localhost-only) + if nc -z -w 2 "${node_ip}" ${NATS_CLIENT_PORT} 2>/dev/null; then echo " Node$i: NATS still responding" all_down=false fi diff --git a/tests/e2e/run-multinode-e2e.sh b/tests/e2e/run-multinode-e2e.sh index 17cb98bea..992567e59 100755 --- a/tests/e2e/run-multinode-e2e.sh +++ b/tests/e2e/run-multinode-e2e.sh @@ -333,7 +333,7 @@ echo "========================================" # NATS cluster: verify 2 unique peers from node1 echo "Checking NATS cluster..." -NATS_INFO=$(curl -s "http://${LOCAL_IP}:${NATS_MONITOR_PORT}/routez" 2>/dev/null) || { +NATS_INFO=$(curl -s "http://127.0.0.1:${NATS_MONITOR_PORT}/routez" 2>/dev/null) || { echo " ERROR: Cannot reach NATS monitoring endpoint" exit 1 } @@ -809,7 +809,7 @@ else fi # Check NATS degraded state (should have 1 route instead of 2) -NATS_DEGRADED=$(curl -s "http://${LOCAL_IP}:${NATS_MONITOR_PORT}/routez" 2>/dev/null) +NATS_DEGRADED=$(curl -s "http://127.0.0.1:${NATS_MONITOR_PORT}/routez" 2>/dev/null) DEGRADED_PEERS=$(echo "$NATS_DEGRADED" | jq -r '[.routes[].remote_name] | unique | length' 2>/dev/null || echo "0") echo " NATS peers during failure: $DEGRADED_PEERS (expected: 1)" if [ "$DEGRADED_PEERS" -eq 1 ]; then @@ -849,7 +849,7 @@ echo " Waiting for NATS cluster to reform..." ATTEMPT=0 REFORMED=false while [ $ATTEMPT -lt 60 ]; do - NATS_RECOVER=$(curl -s "http://${LOCAL_IP}:${NATS_MONITOR_PORT}/routez" 2>/dev/null) + NATS_RECOVER=$(curl -s "http://127.0.0.1:${NATS_MONITOR_PORT}/routez" 2>/dev/null) RECOVER_PEERS=$(echo "$NATS_RECOVER" | jq -r '[.routes[].remote_name] | unique | length' 2>/dev/null || echo "0") if [ "$RECOVER_PEERS" -ge 2 ]; then echo " NATS cluster reformed ($RECOVER_PEERS peers)" diff --git a/tests/e2e/run-pseudo-multinode-e2e.sh b/tests/e2e/run-pseudo-multinode-e2e.sh index b8a22dd2c..1fad30130 100755 --- a/tests/e2e/run-pseudo-multinode-e2e.sh +++ b/tests/e2e/run-pseudo-multinode-e2e.sh @@ -134,6 +134,12 @@ wait $JOIN3_PID || { echo "ERROR: Node 3 join failed"; exit 1; } wait $LEADER_INIT_PID || { echo "ERROR: Leader init failed"; exit 1; } echo "Cluster formation complete — all configs generated" +# Disable monitoring on non-primary nodes to avoid port conflict on 127.0.0.1:8222 +for i in 2 3; do + sed -i '/^http:\s*127\.0\.0\.1:8222/d' "$HOME/node${i}/config/nats/nats.conf" + echo " Stripped NATS monitoring from node$i (avoids 127.0.0.1:8222 conflict)" +done + # Now start services (configs exist for all nodes) echo "" echo "Starting node services..."