From 075b54751628888dff30215571ce54a4f3aa2bed Mon Sep 17 00:00:00 2001 From: "Patrick W. Healy" Date: Thu, 17 Sep 2026 01:10:07 +0000 Subject: [PATCH] net: preserve node information and freshness in cluster summaries Carry only NodeInfo metadata and lastPushTime alongside aggregate counts. Compare metadata by value to avoid spurious deltas and whitelist browser metadata without retaining diagnostic arrays. Preserve legacy projection and metadata-only updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d2243398-6c36-4c3d-969e-7ed7bfb5b459 --- .../node_overview_test.go | 3 +- cmd/unbounded-net-controller/status_types.go | 38 +++++++----- .../summary_metadata_test.go | 62 +++++++++++++++++++ frontend/src/state/clusterSummary.ts | 24 ++++++- frontend/src/types.ts | 2 + frontend/tests/clusterSummary.test.ts | 28 +++++++++ 6 files changed, 139 insertions(+), 18 deletions(-) create mode 100644 cmd/unbounded-net-controller/summary_metadata_test.go diff --git a/cmd/unbounded-net-controller/node_overview_test.go b/cmd/unbounded-net-controller/node_overview_test.go index b4edb51a0..065f81ac7 100644 --- a/cmd/unbounded-net-controller/node_overview_test.go +++ b/cmd/unbounded-net-controller/node_overview_test.go @@ -6,6 +6,7 @@ package main import ( "bytes" "encoding/json" + "reflect" "strings" "sync" "testing" @@ -134,7 +135,7 @@ func TestClusterOverviewPreservesCountsAndEnrichment(t *testing.T) { overview.NodeErrors = []NodeError{{Type: "cni", Message: "blocked"}} c.PatchOverview("node", overview) - if buildClusterSummary(snapshot).NodeSummaries[0] != row { + if !reflect.DeepEqual(buildClusterSummary(snapshot).NodeSummaries[0], row) { t.Fatal("patching changed a previously returned snapshot") } diff --git a/cmd/unbounded-net-controller/status_types.go b/cmd/unbounded-net-controller/status_types.go index c54113178..588338efe 100644 --- a/cmd/unbounded-net-controller/status_types.go +++ b/cmd/unbounded-net-controller/status_types.go @@ -6,6 +6,7 @@ package main import ( "bytes" "encoding/json" + "reflect" "sort" "time" @@ -163,21 +164,23 @@ type ClusterSummary struct { // NodeSummary is a compact per-node summary for use in ClusterSummary. type NodeSummary struct { - Name string `json:"name"` - SiteName string `json:"siteName,omitempty"` - IsGateway bool `json:"isGateway,omitempty"` - K8sReady string `json:"k8sReady,omitempty"` - StatusSource string `json:"statusSource,omitempty"` - CniStatus string `json:"cniStatus,omitempty"` - CniTone string `json:"cniTone,omitempty"` - ErrorCount int `json:"errorCount,omitempty"` - FirstError string `json:"firstError,omitempty"` - PeerCount int `json:"peerCount,omitempty"` - HealthyPeers int `json:"healthyPeers,omitempty"` - RouteCount int `json:"routeCount,omitempty"` - RouteMismatch bool `json:"routeMismatch,omitempty"` - FetchError string `json:"fetchError,omitempty"` - WireGuardOnline bool `json:"wireGuardOnline"` + NodeInfo *NodeInfo `json:"nodeInfo,omitempty"` + LastPushTime *time.Time `json:"lastPushTime,omitempty"` + Name string `json:"name"` + SiteName string `json:"siteName,omitempty"` + IsGateway bool `json:"isGateway,omitempty"` + K8sReady string `json:"k8sReady,omitempty"` + StatusSource string `json:"statusSource,omitempty"` + CniStatus string `json:"cniStatus,omitempty"` + CniTone string `json:"cniTone,omitempty"` + ErrorCount int `json:"errorCount,omitempty"` + FirstError string `json:"firstError,omitempty"` + PeerCount int `json:"peerCount,omitempty"` + HealthyPeers int `json:"healthyPeers,omitempty"` + RouteCount int `json:"routeCount,omitempty"` + RouteMismatch bool `json:"routeMismatch,omitempty"` + FetchError string `json:"fetchError,omitempty"` + WireGuardOnline bool `json:"wireGuardOnline"` } // buildClusterSummary extracts a ClusterSummary from a full ClusterStatusResponse. @@ -195,7 +198,10 @@ func buildClusterSummary(status *ClusterStatusResponse) *ClusterSummary { overview = &projected } + info := node.NodeInfo ns := NodeSummary{ + NodeInfo: &info, + LastPushTime: node.LastPushTime, Name: node.NodeInfo.Name, SiteName: node.NodeInfo.SiteName, IsGateway: node.NodeInfo.IsGateway, @@ -410,7 +416,7 @@ func computeClusterSummaryDelta(prev, curr *ClusterSummary) *ClusterSummaryDelta for _, ns := range curr.NodeSummaries { old, existed := prevByName[ns.Name] - if !existed || ns != old { + if !existed || !reflect.DeepEqual(ns, old) { delta.NodeSummaries = append(delta.NodeSummaries, ns) changed = true } diff --git a/cmd/unbounded-net-controller/summary_metadata_test.go b/cmd/unbounded-net-controller/summary_metadata_test.go new file mode 100644 index 000000000..a37b55165 --- /dev/null +++ b/cmd/unbounded-net-controller/summary_metadata_test.go @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/json" + "reflect" + "strings" + "testing" + "time" + + statusv1alpha1 "github.com/Azure/unbounded/internal/net/status/v1alpha1" +) + +func TestSummaryPreservesNodeInfoAndFreshnessWithoutDetails(t *testing.T) { + received := time.Now() + node := retentionFixture(10000) + node.LastPushTime = &received + node.NodeInfo.ProviderID = "azure://vm" + node.NodeInfo.InternalIPs = []string{"192.0.2.1"} + node.NodeInfo.K8sLabels = map[string]string{"node.kubernetes.io/instance-type": "test"} + node.NodeInfo.K8sUpdatedAt = &received + node.NodeInfo.BuildInfo = &statusv1alpha1.BuildInfo{Version: "test"} + cluster := &ClusterStatusResponse{Nodes: []*NodeStatusResponse{&node}} + summary := buildClusterSummary(cluster) + + row := summary.NodeSummaries[0] + if row.NodeInfo == &node.NodeInfo || !reflect.DeepEqual(row.NodeInfo, &node.NodeInfo) || + row.LastPushTime == nil || !row.LastPushTime.Equal(received) { + t.Fatal("summary lost metadata/freshness or retained the full node allocation") + } + + data, err := json.Marshal(summary) + if err != nil { + t.Fatal(err) + } + + for _, forbidden := range []string{`"peers"`, `"routingTable"`, `"bpfEntries"`, "private-detail-marker"} { + if strings.Contains(string(data), forbidden) { + t.Fatalf("summary retained diagnostic data: %s", forbidden) + } + } + + if delta := computeClusterSummaryDelta(summary, buildClusterSummary(cluster)); delta != nil { + t.Fatalf("equal metadata copies emitted a spurious update: %+v", delta) + } + + node.NodeInfo = NodeInfo{Name: "node", ProviderID: "azure://replacement"} + + next := buildClusterSummary(cluster) + if delta := computeClusterSummaryDelta(summary, next); delta == nil || len(delta.NodeSummaries) != 1 { + t.Fatal("metadata-only update was omitted") + } + + later := received.Add(time.Second) + node.LastPushTime = &later + + if delta := computeClusterSummaryDelta(next, buildClusterSummary(cluster)); delta == nil || len(delta.NodeSummaries) != 1 { + t.Fatal("freshness-only update was omitted") + } +} diff --git a/frontend/src/state/clusterSummary.ts b/frontend/src/state/clusterSummary.ts index fa2e65dbc..a96de5dcd 100644 --- a/frontend/src/state/clusterSummary.ts +++ b/frontend/src/state/clusterSummary.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 -import type { ClusterStatus, ClusterStatusDelta, ClusterSummary, ClusterSummaryDelta, NodeStatus, NodeSummary } from '../types'; +import type { ClusterStatus, ClusterStatusDelta, ClusterSummary, ClusterSummaryDelta, NodeInfo, NodeStatus, NodeSummary } from '../types'; import type { StatusEvent } from '../api'; export function summarySubscriptionMessage() { @@ -45,6 +45,8 @@ export function summarizeNode(node: NodeStatus, now = Date.now()): NodeSummary { const errorCount = node.nodeErrors?.length || 0; const source = node.statusSource; return { + nodeInfo: summaryNodeInfo(node.nodeInfo), + lastPushTime: node.lastPushTime, name: node.nodeInfo?.name, siteName: node.nodeInfo?.siteName, isGateway: node.nodeInfo?.isGateway, @@ -61,10 +63,28 @@ export function summarizeNode(node: NodeStatus, now = Date.now()): NodeSummary { }; } +function summaryNodeInfo(info?: NodeInfo): NodeInfo | undefined { + if (!info) return undefined; + return { + name: info.name, siteName: info.siteName, isGateway: info.isGateway, + k8sReady: info.k8sReady, k8sUpdatedAt: info.k8sUpdatedAt, + podCIDRs: info.podCIDRs, internalIPs: info.internalIPs, externalIPs: info.externalIPs, + providerId: info.providerId, osImage: info.osImage, kernel: info.kernel, + kubelet: info.kubelet, arch: info.arch, nodeOs: info.nodeOs, k8sLabels: info.k8sLabels, + buildInfo: info.buildInfo && { + version: info.buildInfo.version, commit: info.buildInfo.commit, buildTime: info.buildInfo.buildTime, + }, + wireGuard: info.wireGuard && { + interface: info.wireGuard.interface, publicKey: info.wireGuard.publicKey, peerCount: info.wireGuard.peerCount, + }, + }; +} + // Whitelist wire fields, including for already-summary input. Never spread full // cluster/node objects into persistent state or the global JSON export. function summaryNode(node: NodeSummary): NodeSummary { return { + nodeInfo: summaryNodeInfo(node.nodeInfo), lastPushTime: node.lastPushTime, name: node.name, siteName: node.siteName, isGateway: node.isGateway, k8sReady: node.k8sReady, statusSource: node.statusSource, cniStatus: node.cniStatus, cniTone: node.cniTone, @@ -123,6 +143,8 @@ export function mergeLegacySummary(current: ClusterSummary | null, delta: Cluste if (!old) return next; const merged = { ...old, ...next, + nodeInfo: node.nodeInfo ? next.nodeInfo : old.nodeInfo, + lastPushTime: node.lastPushTime ?? old.lastPushTime, siteName: node.nodeInfo ? next.siteName : old.siteName, isGateway: node.nodeInfo ? next.isGateway : old.isGateway, k8sReady: node.nodeInfo ? next.k8sReady : old.k8sReady, diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 46e796524..154ea26fe 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -252,6 +252,8 @@ export type ClusterSummaryDelta = { }; export type NodeSummary = { + nodeInfo?: NodeInfo; + lastPushTime?: string; name?: string; siteName?: string; isGateway?: boolean; diff --git a/frontend/tests/clusterSummary.test.ts b/frontend/tests/clusterSummary.test.ts index dadb8280f..26adc720f 100644 --- a/frontend/tests/clusterSummary.test.ts +++ b/frontend/tests/clusterSummary.test.ts @@ -44,6 +44,34 @@ test('summary input wins over legacy full fields and is itself whitelisted', () assert.equal(JSON.stringify(summary).includes('hidden'), false); }); +test('summary metadata and freshness survive snapshots and deltas without diagnostic arrays', () => { + const nodeInfo = { + name: 'node', internalIPs: ['192.0.2.1'], providerId: 'azure://vm', + k8sReady: 'Ready', k8sUpdatedAt: '2026-09-17T00:00:00Z', + buildInfo: { version: 'test', peers: ['hidden'] }, + wireGuard: { interface: 'wg0', publicKey: 'public-key', peers: ['hidden'] }, + peers: ['hidden'], bpfEntries: ['hidden'], + }; + const projected = toClusterSummary({ nodes: [{ + nodeInfo, lastPushTime: '2026-09-17T00:01:00Z', peers: [{ name: 'hidden' }], + }] }); + const initial = toClusterSummary({ seq: 1, nodeSummaries: projected.nodeSummaries }); + assert.equal(initial.nodeSummaries[0].nodeInfo.providerId, 'azure://vm'); + assert.equal(initial.nodeSummaries[0].nodeInfo.wireGuard.publicKey, 'public-key'); + assert.equal(initial.nodeSummaries[0].lastPushTime, '2026-09-17T00:01:00Z'); + assert.equal(JSON.stringify(initial).includes('hidden'), false); + const next = mergeSummary(initial, { + seq: 2, nodeSummaries: [{ ...initial.nodeSummaries[0], lastPushTime: '2026-09-17T00:02:00Z' }], + }); + assert.equal(next.nodeSummaries[0].lastPushTime, '2026-09-17T00:02:00Z'); + const legacy = mergeLegacySummary(next, { + seq: 3, updatedNodes: [{ nodeInfo: { ...nodeInfo, kernel: 'new-kernel' } }], + }); + assert.equal(legacy.nodeSummaries[0].nodeInfo.kernel, 'new-kernel'); + assert.equal(legacy.nodeSummaries[0].lastPushTime, '2026-09-17T00:02:00Z'); + assert.equal(JSON.stringify(legacy).includes('hidden'), false); +}); + test('CNI priority preserves no-data, errors, unknown and health', () => { assert.equal(summarizeNode({ statusSource: 'no-data', fetchError: 'error' }).cniStatus, 'No data'); assert.equal(summarizeNode({ fetchError: 'error' }).cniStatus, 'Fetch error');