From 8e2e7a7d594efbcaad89ed4e406afc5e7bfd4e77 Mon Sep 17 00:00:00 2001 From: Christo Date: Tue, 16 Jun 2026 14:03:30 +0800 Subject: [PATCH] =?UTF-8?q?feat:=20NodeHostField=20=E2=80=94=20dial=20Node?= =?UTF-8?q?sKey=20peers=20by=20name,=20not=20raw=20IP?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an opt-in Config.NodeHostField: when set, parseNodeAddr uses that JSON field of a node entry as the dial host (combined with the entry's port) instead of "ip", falling back to ip per-entry when the field is absent or blank. Empty (default) preserves the historical ip:port behavior byte-for-byte. This lets a leader dial nodes by a DNS name so a name-only TLS leaf (no IP SAN) validates under standard hostname verification — the leader→node health check and trigger-sync no longer fail with "tls: bad certificate" when SSL is on. Threaded through nodesCache and makeGetNodes; new TestParseNodeAddr covers override, per-entry fallback, and defaults. Co-Authored-By: Claude Opus 4.8 --- getnodes_bench_test.go | 4 +-- nodeaddr_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++ pivot.go | 49 ++++++++++++++++++-------- 3 files changed, 115 insertions(+), 17 deletions(-) create mode 100644 nodeaddr_test.go diff --git a/getnodes_bench_test.go b/getnodes_bench_test.go index 8499288..378f503 100644 --- a/getnodes_bench_test.go +++ b/getnodes_bench_test.go @@ -66,7 +66,7 @@ func seedNodes(t testing.TB, s *ooo.Server, n int, extraSettingsKB int) { // on the GetNodes call and does not spin up health-check goroutines etc. func newBenchInstance(s *ooo.Server, nodesKey string) *Instance { inst := &Instance{} - inst.nodesCache = newNodesCache(s, nodesKey, inst.IsShutdown) + inst.nodesCache = newNodesCache(s, nodesKey, "", inst.IsShutdown) return inst } @@ -81,7 +81,7 @@ func BenchmarkGetNodesFresh(b *testing.B) { defer s.Storage.Close() seedNodes(b, s, n, kb) inst := newBenchInstance(s, "nodes/*") - get := makeGetNodes(s, "nodes/*", inst) + get := makeGetNodes(s, "nodes/*", "", inst) b.ReportAllocs() b.ResetTimer() diff --git a/nodeaddr_test.go b/nodeaddr_test.go new file mode 100644 index 0000000..fe4153f --- /dev/null +++ b/nodeaddr_test.go @@ -0,0 +1,79 @@ +package pivot + +import "testing" + +// TestParseNodeAddr covers the host-field override that lets the leader dial +// nodes by a DNS name (so a name-only TLS leaf validates) while preserving the +// historical ip:port behavior by default. +func TestParseNodeAddr(t *testing.T) { + cases := []struct { + name string + data string + hostField string + want string + }{ + { + name: "default uses ip lowercase", + data: `{"ip":"10.0.2.101","port":3099}`, + want: "10.0.2.101:3099", + }, + { + name: "default uses IP uppercase", + data: `{"IP":"10.0.2.101","Port":3099}`, + want: "10.0.2.101:3099", + }, + { + name: "default ignores host field when not configured", + data: `{"ip":"10.0.2.101","host":"table-001.internal","port":3099}`, + want: "10.0.2.101:3099", + }, + { + name: "host field overrides ip when configured and present", + data: `{"ip":"10.0.2.101","host":"table-001.internal","port":3099}`, + hostField: "host", + want: "table-001.internal:3099", + }, + { + name: "host field falls back to ip when blank", + data: `{"ip":"10.0.2.101","host":"","port":3099}`, + hostField: "host", + want: "10.0.2.101:3099", + }, + { + name: "host field falls back to ip when absent", + data: `{"ip":"10.0.2.101","port":3099}`, + hostField: "host", + want: "10.0.2.101:3099", + }, + { + name: "port as quoted string is accepted with host field", + data: `{"host":"table-001.internal","port":"3099"}`, + hostField: "host", + want: "table-001.internal:3099", + }, + { + name: "no host and no ip yields empty", + data: `{"port":3099}`, + hostField: "host", + want: "", + }, + { + name: "missing port yields empty", + data: `{"ip":"10.0.2.101"}`, + want: "", + }, + { + name: "invalid json yields empty", + data: `not json`, + want: "", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := parseNodeAddr([]byte(tc.data), tc.hostField) + if got != tc.want { + t.Fatalf("parseNodeAddr(%s, %q) = %q, want %q", tc.data, tc.hostField, got, tc.want) + } + }) + } +} diff --git a/pivot.go b/pivot.go index 5f6145c..ac20cc6 100644 --- a/pivot.go +++ b/pivot.go @@ -51,6 +51,14 @@ type Config struct { SSL bool // If true, use HTTPS instead of HTTP for all requests. Default false. HealthCheckInterval time.Duration // Interval for health checks. Default 3s. SyncRetryInterval time.Duration // Initial backoff for sync retries. Default 1s. + // NodeHostField, when non-empty, is the JSON field name in a NodesKey entry + // whose value is used as the dial host (combined with the entry's port) + // instead of the "ip" field. Empty (default) preserves the historical + // ip:port behavior exactly. Use it to dial nodes by a DNS name so a + // name-only TLS leaf (no IP SAN) validates under standard hostname + // verification. Resolution is per-entry: an entry missing/blank in this + // field falls back to its ip, so a mixed fleet stays reachable. + NodeHostField string } // Scheme returns "https" if SSL is enabled, "http" otherwise. @@ -86,19 +94,28 @@ type getNodes func() []string // GetNodes is the exported type for node discovery functions (backward compatibility) type GetNodes func() []string -// parseNodeAddr extracts "ip:port" from a node entry's JSON data. +// parseNodeAddr extracts "host:port" from a node entry's JSON data. // Returns "" if either field is missing or invalid. Accepts lower/upper case -// keys and int/float64/string port encodings. -func parseNodeAddr(data []byte) string { +// keys and int/float64/string port encodings. When hostField is non-empty and +// the entry carries a non-blank value under it, that value is used as the host +// (e.g. a DNS name for name-based TLS); otherwise it falls back to the ip field. +func parseNodeAddr(data []byte, hostField string) string { var raw map[string]any if err := json.Unmarshal(data, &raw); err != nil { return "" } var ip string - if v, ok := raw["ip"].(string); ok { - ip = v - } else if v, ok := raw["IP"].(string); ok { - ip = v + if hostField != "" { + if v, ok := raw[hostField].(string); ok { + ip = v + } + } + if ip == "" { + if v, ok := raw["ip"].(string); ok { + ip = v + } else if v, ok := raw["IP"].(string); ok { + ip = v + } } var port int for _, k := range [2]string{"port", "Port"} { @@ -128,18 +145,20 @@ func parseNodeAddr(data []byte) string { type nodesCache struct { server *ooo.Server nodesKey string + hostField string isShutdown func() bool mu sync.RWMutex loaded bool - entries map[string]string // obj.Index -> "ip:port" + entries map[string]string // obj.Index -> "host:port" slice []string // immutable after rebuild; callers must not mutate } -func newNodesCache(server *ooo.Server, nodesKey string, isShutdown func() bool) *nodesCache { +func newNodesCache(server *ooo.Server, nodesKey, hostField string, isShutdown func() bool) *nodesCache { return &nodesCache{ server: server, nodesKey: nodesKey, + hostField: hostField, isShutdown: isShutdown, } } @@ -180,7 +199,7 @@ func (c *nodesCache) loadLocked() { return } for _, obj := range objs { - if addr := parseNodeAddr(obj.Data); addr != "" { + if addr := parseNodeAddr(obj.Data, c.hostField); addr != "" { c.entries[obj.Index] = addr } } @@ -210,7 +229,7 @@ func (c *nodesCache) update(event storage.Event) { delete(c.entries, idx) c.rebuildSliceLocked() case "set": - newAddr := parseNodeAddr(event.Object.Data) + newAddr := parseNodeAddr(event.Object.Data, c.hostField) prev, had := c.entries[idx] if had && newAddr == prev { // Settings-only change — ip:port unchanged, nothing to do. @@ -274,7 +293,7 @@ func buildKeys(server *ooo.Server, config Config) []Key { // storage on every call. This preserves synchronous read-after-write semantics // for external consumers (e.g., the UI, GetPivotInfo) who expect a node that // was just registered to appear immediately. -func makeGetNodes(server *ooo.Server, nodesKey string, instance *Instance) getNodes { +func makeGetNodes(server *ooo.Server, nodesKey, hostField string, instance *Instance) getNodes { return func() []string { if instance.IsShutdown() { return nil @@ -296,7 +315,7 @@ func makeGetNodes(server *ooo.Server, nodesKey string, instance *Instance) getNo result := make([]string, 0, len(extras)+len(objs)) result = append(result, extras...) for _, obj := range objs { - if addr := parseNodeAddr(obj.Data); addr != "" { + if addr := parseNodeAddr(obj.Data, hostField); addr != "" { result = append(result, addr) } } @@ -530,9 +549,9 @@ func SetupWithError(server *ooo.Server, config Config) (*ooo.Server, error) { ctx: instanceCtx, cancel: instanceCancel, } - instance.nodesCache = newNodesCache(server, config.NodesKey, instance.IsShutdown) + instance.nodesCache = newNodesCache(server, config.NodesKey, config.NodeHostField, instance.IsShutdown) - getNodes := makeGetNodes(server, config.NodesKey, instance) + getNodes := makeGetNodes(server, config.NodesKey, config.NodeHostField, instance) getNodesCached := makeGetNodesCached(instance) // Construct VVManager first so the syncer pool can read local VVs