feat(component-failover-adoption): route apiserver's candidate parsing through the real C parser

Replaces apiserver's hand-written Go parsePeer() with a CGO wrapper
around the shared C parser+validator pair (xce_parse_peers then
xce_cluster_config_validate, both src/lib/cluster.c) that worker and
scheduler already use from their own main.c's. Gives apiserver the same
[name=]host:port[,...] syntax (including the name= prefix the old Go
parser could not express) and identical malformed-candidate rejection
(empty host, port <= 0, non-numeric port) -- round-1 review caught the
first pass skipping the validate() half, silently accepting inputs
worker/scheduler both reject.

A malformed entry now rejects the whole candidate list rather than
being skipped with a warning (matches xce_parse_peers()'s own
all-or-nothing construction and worker/scheduler's precedent) --
surfaced loudly via RegisterToBackend's error, not silently.

Parsing only -- RegisterToBackend()'s connect loop and tryConnect()
are unchanged; collapsing them into a single connect call is task
4.2's job.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNB4zGPCsHyDUCDJyMbiQN

src/frontend/apiserver/backend/candidates.go

@@ -0,0 +1,97 @@
+//go:build cgo
+
+package backend
+
+/*
+#include <stdlib.h>
+#include "xce/cluster.h"
+*/
+import "C"
+
+import (
+	"fmt"
+	"unsafe"
+)
+
+// Candidate is one parsed controller candidate: an optional logical name
+// plus the host:port it resolves to. Mirrors xce_candidate_t (xce/cluster.h).
+type Candidate struct {
+	Name string
+	Host string
+	Port int
+}
+
+// parseCandidates parses a comma-joined "[name=]host:port,..." candidate
+// list through the same shared C parsing+validation pair (xce_parse_peers
+// then xce_cluster_config_validate, both src/lib/cluster.c) that worker and
+// scheduler already call directly, in that same order, from their C
+// main.c's -- retiring apiserver's own hand-written parsePeer() in favor of
+// it (Req 6.1: apiserver accepts the identical candidate-list syntax
+// scheduler and worker already accept).
+//
+// xce_parse_peers alone is SYNTAX-only: it splits on ','/'='/':' but never
+// rejects a zero/non-numeric port (atoi() silently defaults malformed input
+// to 0) or an empty hostname. xce_cluster_config_validate is what actually
+// rejects those (src/lib/cluster.c:85-100) -- worker/scheduler always call
+// it immediately after a successful xce_parse_peers
+// (src/scheduler/src/main.c:300-309, src/worker/src/main.c:414-422), so
+// this wrapper must too, or apiserver would silently accept candidate
+// strings worker/scheduler reject (Req 6.2).
+// xce_cluster_config_validate also requires cfg->identity to be non-empty;
+// parseCandidates has no real identity to give it at this layer (clib.go's
+// RegisterToBackend assembles that separately, after parsing, from
+// operator-supplied From/Key config) -- a placeholder is stamped here
+// purely to satisfy that one precondition so the per-candidate
+// hostname/port checks actually run. It is never read or reused beyond
+// this call.
+//
+// On any malformed entry -- whether xce_parse_peers' own syntax check or
+// xce_cluster_config_validate's semantic check fails -- the WHOLE string is
+// rejected; neither function identifies which token was the bad one, and
+// neither does this wrapper (Req 6.2): that matches worker's and
+// scheduler's own established behavior, which likewise echoes the full
+// attempted --peers string back on any parse/validate failure (main.c in
+// both) rather than pinpointing one entry.
+//
+// If candidatesCount exceeds MAX_HA_CANDIDATES, xce_parse_peers silently
+// stops reading further tokens (no error, no warning at this layer) --
+// this wrapper does not add stricter behavior than the C functions have.
+func parseCandidates(peersStr string) ([]Candidate, error) {
+	cPeers := C.CString(peersStr)
+	defer C.free(unsafe.Pointer(cPeers))
+
+	malformedErr := func() error {
+		return fmt.Errorf(
+			"apiserver: --peers parse error: expected \"[name=]host:port,...\" got %q",
+			peersStr)
+	}
+
+	var cCfg C.xce_cluster_config_t
+	if rc := C.xce_parse_peers(cPeers, &cCfg); rc != 0 {
+		return nil, malformedErr()
+	}
+
+	// Placeholder identity: satisfies xce_cluster_config_validate's
+	// non-empty-identity precondition only -- see doc comment above. cCfg is
+	// a fresh zero-valued struct, so the array is already NUL-padded past
+	// what we write here.
+	const placeholderIdentity = "apiserver-candidates-validate"
+	for i := 0; i < len(placeholderIdentity); i++ {
+		cCfg.identity[i] = C.char(placeholderIdentity[i])
+	}
+	if rc := C.xce_cluster_config_validate(&cCfg); rc != 0 {
+		return nil, malformedErr()
+	}
+
+	n := int(cCfg.candidates_count)
+	out := make([]Candidate, n)
+	for i := 0; i < n; i++ {
+		cand := &cCfg.candidates[i]
+		out[i] = Candidate{
+			Name: C.GoString(&cand.name[0]),
+			Host: C.GoString(&cand.hostname[0]),
+			Port: int(cand.port),
+		}
+	}
+	return out, nil
+}

src/frontend/apiserver/backend/candidates_test.go

@@ -0,0 +1,140 @@
+//go:build cgo
+
+package backend
+
+import (
+	"strings"
+	"testing"
+)
+
+// candidates_test.go exercises parseCandidates (candidates.go), the CGO
+// wrapper around the shared C parser xce_parse_peers() that replaces
+// apiserver's retired hand-written parsePeer() (Req 6.1, 6.2). Kept
+// //go:build cgo since parseCandidates only exists in the CGO build --
+// this file itself does not `import "C"` (a _test.go file can never do
+// that directly, golang/go#13938): it only calls the plain-Go-signature
+// parseCandidates defined in candidates.go, same split already used by
+// wire_dispatch_test.go/wire_dispatch_support.go in this package.
+
+func TestParseCandidates_ValidHostPort(t *testing.T) {
+	got, err := parseCandidates("ctrl01:9990")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	want := []Candidate{{Name: "", Host: "ctrl01", Port: 9990}}
+	if len(got) != len(want) || got[0] != want[0] {
+		t.Errorf("got %+v, want %+v", got, want)
+	}
+}
+
+func TestParseCandidates_NamePrefixSyntax(t *testing.T) {
+	// The old hand-written parsePeer() could not parse this syntax at all
+	// -- this is the format-parity case that proves real parity with
+	// worker/scheduler's --peers (Req 6.1).
+	got, err := parseCandidates("ctrl2=host2:9991")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	want := []Candidate{{Name: "ctrl2", Host: "host2", Port: 9991}}
+	if len(got) != len(want) || got[0] != want[0] {
+		t.Errorf("got %+v, want %+v", got, want)
+	}
+}
+
+func TestParseCandidates_MultipleCommaJoined(t *testing.T) {
+	got, err := parseCandidates("ctrl01:9990,ctrl2=host2:9991,ctrl03:9992")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	want := []Candidate{
+		{Name: "", Host: "ctrl01", Port: 9990},
+		{Name: "ctrl2", Host: "host2", Port: 9991},
+		{Name: "", Host: "ctrl03", Port: 9992},
+	}
+	if len(got) != len(want) {
+		t.Fatalf("got %d candidates, want %d: %+v", len(got), len(want), got)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Errorf("candidate[%d]: got %+v, want %+v", i, got[i], want[i])
+		}
+	}
+}
+
+func TestParseCandidates_MalformedEntry_MissingColon(t *testing.T) {
+	_, err := parseCandidates("ctrl01")
+	if err == nil {
+		t.Fatal("expected error for missing colon, got nil")
+	}
+	// Req 6.2: reject identifying the malformed entry, consistent with how
+	// worker/scheduler report it -- since xce_parse_peers() itself cannot
+	// identify which token was malformed (same limit worker/scheduler's own
+	// --peers parsing has), the whole attempted string is echoed back.
+	if !strings.Contains(err.Error(), "ctrl01") {
+		t.Errorf("error %q does not identify the malformed input", err.Error())
+	}
+}
+
+func TestParseCandidates_EmptyString(t *testing.T) {
+	_, err := parseCandidates("")
+	if err == nil {
+		t.Fatal("expected error for empty peers string, got nil")
+	}
+}
+
+func TestParseCandidates_MalformedEntry_MidList(t *testing.T) {
+	// A malformed second entry rejects the WHOLE list -- xce_parse_peers()
+	// does not skip just the bad token and keep the rest (Req 6.2, matching
+	// worker/scheduler's own all-or-nothing --peers parsing).
+	_, err := parseCandidates("ctrl01:9990,badentry,ctrl03:9992")
+	if err == nil {
+		t.Fatal("expected error for malformed mid-list entry, got nil")
+	}
+}
+
+func TestParseCandidates_MalformedEntry_ZeroPort(t *testing.T) {
+	// xce_parse_peers() alone would happily atoi() "0" into port=0 -- it is
+	// xce_cluster_config_validate()'s port<=0 check (src/lib/cluster.c:94)
+	// that rejects it, same as worker/scheduler (Req 6.2).
+	_, err := parseCandidates("ctrl01:0")
+	if err == nil {
+		t.Fatal("expected error for port 0, got nil")
+	}
+}
+
+func TestParseCandidates_MalformedEntry_NonNumericPort(t *testing.T) {
+	// atoi("abc") silently returns 0 -- caught by the same port<=0 check as
+	// the explicit-zero case above, not by xce_parse_peers() itself.
+	_, err := parseCandidates("ctrl01:abc")
+	if err == nil {
+		t.Fatal("expected error for non-numeric port, got nil")
+	}
+}
+
+func TestParseCandidates_MalformedEntry_EmptyHost(t *testing.T) {
+	// xce_cluster_config_validate()'s hostname[0]=='\0' check
+	// (src/lib/cluster.c:92) rejects an empty host, same as worker/scheduler.
+	_, err := parseCandidates(":9990")
+	if err == nil {
+		t.Fatal("expected error for empty host, got nil")
+	}
+}
+
+func TestParseCandidates_OverCapSilentlyTruncates(t *testing.T) {
+	// MAX_HA_CANDIDATES (src/include/config.h) is 4. xce_parse_peers()
+	// silently stops reading further tokens once the cap is reached --
+	// no error, no warning at this layer (the WARN-on-truncation behavior
+	// belongs to xce_cluster_config_to_client_config(), a different
+	// function, not exercised by this task). Don't invent stricter
+	// behavior than the C function actually has.
+	got, err := parseCandidates("c1:1,c2:2,c3:3,c4:4,c5:5")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(got) != 4 {
+		t.Fatalf("got %d candidates, want 4 (MAX_HA_CANDIDATES cap)", len(got))
+	}
+	if got[3] != (Candidate{Name: "", Host: "c4", Port: 4}) {
+		t.Errorf("candidate[3] = %+v, want the 4th entry (c5 silently dropped)", got[3])
+	}
+}

src/frontend/apiserver/backend/clib.go

@@ -59,6 +59,7 @@ import (
 	logger "apiserver/utils"
 	"context"
 	"fmt"
+	"strings"
 	"sync"
 	"unsafe"
 
@@ -114,22 +115,27 @@ func RegisterToBackend(config *BackendConfig) error {
 	}
 
 	if len(config.Peers) > 0 {
+		// Parsing is routed through the same C parser (xce_parse_peers) worker
+		// and scheduler already call directly -- Req 6.1/6.2. A malformed
+		// entry rejects the whole candidate list up front (matching
+		// worker's/scheduler's own all-or-nothing --peers parsing) rather
+		// than being skipped with a warning as the old hand-written
+		// parsePeer()-per-entry loop did.
+		candidates, err := parseCandidates(strings.Join(config.Peers, ","))
+		if err != nil {
+			return err
+		}
+
 		var lastErr error
-		for _, peer := range config.Peers {
-			host, port, err := parsePeer(peer)
-			if err != nil {
-				logger.Warn("Skipping invalid peer", zap.String("peer", peer), zap.Error(err))
-				lastErr = err
-				continue
-			}
-			if err := tryConnect(host, port, from, key); err != nil {
+		for _, cand := range candidates {
+			if err := tryConnect(cand.Host, cand.Port, from, key); err != nil {
 				logger.Warn("Candidate unreachable, trying next",
-					zap.String("host", host), zap.Int("port", port), zap.Error(err))
+					zap.String("host", cand.Host), zap.Int("port", cand.Port), zap.Error(err))
 				lastErr = err
 				continue
 			}
 			logger.Info("Backend connection established (HA)",
-				zap.String("host", host), zap.Int("port", port))
+				zap.String("host", cand.Host), zap.Int("port", cand.Port))
 			return nil
 		}
 		return fmt.Errorf("all HA candidates failed: %w", lastErr)

src/frontend/apiserver/backend/config.go

@@ -8,10 +8,13 @@ type BackendConfig struct {
 	From    string // identity sent to controller (default: "xce_apiserver")
 	Key     string // authentication key
 
-	// Peers lists additional controller candidates for HA mode in "host:port"
-	// format.  When non-empty, RegisterToBackend tries each candidate in order
-	// and connects to whichever accepts the join as primary.  Address+Port
-	// serve as a single-host fallback when Peers is empty.
+	// Peers lists additional controller candidates for HA mode, each entry in
+	// "[name=]host:port" format -- the same syntax scheduler's and worker's
+	// --peers flag already accept (Req 6.1), parsed via the shared C parser
+	// xce_parse_peers (candidates.go).  When non-empty, RegisterToBackend
+	// tries each candidate in order and connects to whichever accepts the
+	// join as primary.  Address+Port serve as a single-host fallback when
+	// Peers is empty.
 	Peers []string
 }
 

src/frontend/apiserver/backend/peers.go

@@ -1,24 +0,0 @@
-package backend
-
-import (
-	"fmt"
-	"net"
-	"strconv"
-)
-
-// parsePeer splits a "host:port" string. Uses net.SplitHostPort so IPv6
-// bracket notation (e.g. "[::1]:9990") is handled correctly.
-func parsePeer(peer string) (host string, port int, err error) {
-	h, portStr, err := net.SplitHostPort(peer)
-	if err != nil {
-		return "", 0, fmt.Errorf("invalid peer %q: %w", peer, err)
-	}
-	if h == "" {
-		return "", 0, fmt.Errorf("invalid peer %q: empty host", peer)
-	}
-	port, err = strconv.Atoi(portStr)
-	if err != nil || port <= 0 || port > 65535 {
-		return "", 0, fmt.Errorf("invalid port in peer %q", peer)
-	}
-	return h, port, nil
-}

src/frontend/apiserver/backend/peers_test.go

@@ -1,95 +0,0 @@
-package backend
-
-import "testing"
-
-func TestParsePeer(t *testing.T) {
-	tests := []struct {
-		name      string
-		peer      string
-		wantHost  string
-		wantPort  int
-		wantError bool
-	}{
-		{
-			name:     "valid simple host:port",
-			peer:     "ctrl01:9990",
-			wantHost: "ctrl01",
-			wantPort: 9990,
-		},
-		{
-			name:     "valid IPv4",
-			peer:     "192.168.1.10:9990",
-			wantHost: "192.168.1.10",
-			wantPort: 9990,
-		},
-		{
-			name:     "valid FQDN",
-			peer:     "ctrl01.cluster.local:9990",
-			wantHost: "ctrl01.cluster.local",
-			wantPort: 9990,
-		},
-		{
-			name:     "min valid port",
-			peer:     "host:1",
-			wantHost: "host",
-			wantPort: 1,
-		},
-		{
-			name:     "max valid port",
-			peer:     "host:65535",
-			wantHost: "host",
-			wantPort: 65535,
-		},
-		{
-			name:      "missing colon",
-			peer:      "ctrl019990",
-			wantError: true,
-		},
-		{
-			name:      "port zero",
-			peer:      "ctrl01:0",
-			wantError: true,
-		},
-		{
-			name:      "port out of range",
-			peer:      "ctrl01:65536",
-			wantError: true,
-		},
-		{
-			name:      "non-numeric port",
-			peer:      "ctrl01:abc",
-			wantError: true,
-		},
-		{
-			name:      "colon at start (empty host)",
-			peer:      ":9990",
-			wantError: true,
-		},
-		{
-			name:      "empty string",
-			peer:      "",
-			wantError: true,
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			host, port, err := parsePeer(tc.peer)
-			if tc.wantError {
-				if err == nil {
-					t.Errorf("parsePeer(%q): expected error, got host=%q port=%d", tc.peer, host, port)
-				}
-				return
-			}
-			if err != nil {
-				t.Fatalf("parsePeer(%q): unexpected error: %v", tc.peer, err)
-			}
-			if host != tc.wantHost {
-				t.Errorf("host: got %q, want %q", host, tc.wantHost)
-			}
-			if port != tc.wantPort {
-				t.Errorf("port: got %d, want %d", port, tc.wantPort)
-			}
-		})
-	}
-}