Skip to content

Commit e83ab2b

Browse files
scotwellsclaude
andcommitted
fix(connector): carry full connectionDetails in liveness annotation
The connector liveness annotation previously carried a reduced {ready, nodeID} snapshot, hard-coding the assumption that the tunnel node ID is the PublicKey id. Carry the upstream Connector's full status.connectionDetails (type discriminator + type-specific block) alongside ready instead, so additional connection types can be supported without changing the annotation's JSON schema. The extension server now derives the tunnel node ID via a type-aware ConnectorConnectionDetails.TunnelNodeID() that switches on the connection type, with an empty-string default for unknown types, and keeps the existing status fallback when the annotation is absent or unparseable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1d645ee commit e83ab2b

6 files changed

Lines changed: 100 additions & 37 deletions

File tree

api/v1alpha1/connector_types.go

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -200,20 +200,40 @@ const ConnectorLivenessAnnotation = "networking.datumapis.com/connector-liveness
200200

201201
// ConnectorLiveness is the JSON payload stored in ConnectorLivenessAnnotation.
202202
//
203-
// It carries only the status-derived fields the extension server needs to
204-
// classify a connector online/offline and to build the data-plane tunnel
205-
// cluster. It deliberately does NOT include the tunnel TargetHost/TargetPort:
206-
// those are derived from the referencing HTTPProxy backend endpoint URL, not
207-
// from Connector status.
203+
// It carries the upstream Connector's Ready classification plus its full
204+
// ConnectionDetails. Embedding the complete ConnectionDetails — including the
205+
// type discriminator and the type-specific block — lets the extension server
206+
// derive the tunnel node ID for any connection type the API supports, today and
207+
// in future, without changing this annotation's JSON schema. It deliberately
208+
// does NOT include the tunnel TargetHost/TargetPort: those are derived from the
209+
// referencing HTTPProxy backend endpoint URL, not from Connector status.
208210
type ConnectorLiveness struct {
209211
// Ready mirrors the upstream Connector's Ready condition being True.
210212
Ready bool `json:"ready"`
211213

212-
// NodeID is the connector's public-key id, taken from
213-
// Status.ConnectionDetails.PublicKey.Id when the connector is ready and
214-
// advertises PublicKey connection details. Empty otherwise. Used as the
215-
// tunnel endpoint_id in the data-plane connector cluster.
216-
NodeID string `json:"nodeID,omitempty"`
214+
// ConnectionDetails is the upstream Connector's full
215+
// Status.ConnectionDetails, copied verbatim so the extension server can
216+
// extract the tunnel node ID in a type-aware way (see TunnelNodeID). Nil
217+
// when the upstream connector has not yet published connection details.
218+
ConnectionDetails *ConnectorConnectionDetails `json:"connectionDetails,omitempty"`
219+
}
220+
221+
// TunnelNodeID returns the data-plane tunnel endpoint_id for these connection
222+
// details, dispatching on the connection Type so new connection types can be
223+
// added by extending the switch rather than by assuming PublicKey. It returns
224+
// an empty string for nil details or any type that does not (yet) advertise a
225+
// node ID.
226+
func (d *ConnectorConnectionDetails) TunnelNodeID() string {
227+
if d == nil {
228+
return ""
229+
}
230+
switch d.Type {
231+
case PublicKeyConnectorConnectionType:
232+
if d.PublicKey != nil {
233+
return d.PublicKey.Id
234+
}
235+
}
236+
return ""
217237
}
218238

219239
// +kubebuilder:object:root=true

api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/controller/gateway_resource_replicator_controller.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -483,10 +483,12 @@ func setConnectorLivenessAnnotation(downstreamObj, upstreamObj *unstructured.Uns
483483
return nil
484484
}
485485

486-
// connectorLivenessFromUpstream extracts the Ready condition and PublicKey node
487-
// ID from an upstream Connector's (unstructured) status. A connector with no
488-
// status, a non-True Ready condition, or no PublicKey connection details yields
489-
// a not-ready liveness with an empty NodeID.
486+
// connectorLivenessFromUpstream extracts the Ready condition and the full
487+
// ConnectionDetails from an upstream Connector's (unstructured) status. A
488+
// connector with no status yields a not-ready liveness with nil
489+
// ConnectionDetails. The complete ConnectionDetails is carried so the extension
490+
// server can derive the tunnel node ID for any connection type without an
491+
// annotation-schema change.
490492
func connectorLivenessFromUpstream(upstreamObj *unstructured.Unstructured) (networkingv1alpha1.ConnectorLiveness, error) {
491493
var liveness networkingv1alpha1.ConnectorLiveness
492494

@@ -505,13 +507,7 @@ func connectorLivenessFromUpstream(upstreamObj *unstructured.Unstructured) (netw
505507
}
506508

507509
liveness.Ready = apimeta.IsStatusConditionTrue(status.Conditions, networkingv1alpha1.ConnectorConditionReady)
508-
if liveness.Ready {
509-
if details := status.ConnectionDetails; details != nil &&
510-
details.Type == networkingv1alpha1.PublicKeyConnectorConnectionType &&
511-
details.PublicKey != nil {
512-
liveness.NodeID = details.PublicKey.Id
513-
}
514-
}
510+
liveness.ConnectionDetails = status.ConnectionDetails
515511
return liveness, nil
516512
}
517513

internal/controller/gateway_resource_replicator_controller_test.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -441,9 +441,9 @@ func TestReplicatorMirrorsNSOPolicyTypesSkipsUpstreamStatusSync(t *testing.T) {
441441
// TestReplicatorMirrorsConnectorSpecAndLivenessAnnotation verifies that when the
442442
// replicator handles a Connector it:
443443
// - copies spec into the downstream ns-<uid> namespace, AND
444-
// - stamps the ConnectorLivenessAnnotation (ready + nodeID) onto the
445-
// downstream object's metadata so the edge extension server can read tunnel
446-
// liveness locally.
444+
// - stamps the ConnectorLivenessAnnotation (ready + full connectionDetails)
445+
// onto the downstream object's metadata so the edge extension server can read
446+
// tunnel liveness locally.
447447
//
448448
// The annotation — not the status subresource — carries liveness because Karmada
449449
// propagates a resource template's spec + metadata to member clusters but NOT
@@ -526,12 +526,21 @@ func TestReplicatorMirrorsConnectorSpecAndLivenessAnnotation(t *testing.T) {
526526
"downstream spec must mirror upstream spec")
527527

528528
// Key assertion: the liveness annotation reflects the upstream Ready
529-
// condition and the PublicKey node ID.
530-
expected, err := json.Marshal(networkingv1alpha1.ConnectorLiveness{Ready: true, NodeID: "node-abc"})
529+
// condition and carries the full upstream connectionDetails (type
530+
// discriminator + type-specific block), not a reduced node-ID-only view.
531+
expected, err := json.Marshal(networkingv1alpha1.ConnectorLiveness{
532+
Ready: true,
533+
ConnectionDetails: &networkingv1alpha1.ConnectorConnectionDetails{
534+
Type: networkingv1alpha1.PublicKeyConnectorConnectionType,
535+
PublicKey: &networkingv1alpha1.ConnectorConnectionDetailsPublicKey{
536+
Id: "node-abc",
537+
},
538+
},
539+
})
531540
assert.NoError(t, err)
532541
assert.Equal(t, string(expected),
533542
downstream.GetAnnotations()[networkingv1alpha1.ConnectorLivenessAnnotation],
534-
"downstream liveness annotation must carry ready + nodeID derived from upstream status")
543+
"downstream liveness annotation must carry ready + the full upstream connectionDetails")
535544

536545
// The replicator must NOT mirror the status subresource downstream (Karmada
537546
// would not propagate it to members anyway).

internal/extensionserver/cache/index.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,10 @@ func connectorLiveness(connector *networkingv1alpha1.Connector) (online bool, no
181181
if raw, ok := connector.Annotations[networkingv1alpha1.ConnectorLivenessAnnotation]; ok {
182182
var liveness networkingv1alpha1.ConnectorLiveness
183183
if err := json.Unmarshal([]byte(raw), &liveness); err == nil {
184-
return liveness.Ready, liveness.NodeID
184+
// TunnelNodeID dispatches on the connection type, so a new
185+
// connection type is supported by extending that switch rather than
186+
// by changing the annotation schema here.
187+
return liveness.Ready, liveness.ConnectionDetails.TunnelNodeID()
185188
}
186189
// Unparseable annotation: fall through to status-based classification.
187190
}
@@ -191,11 +194,7 @@ func connectorLiveness(connector *networkingv1alpha1.Connector) (online bool, no
191194
networkingv1alpha1.ConnectorConditionReady,
192195
)
193196
if online {
194-
if details := connector.Status.ConnectionDetails; details != nil &&
195-
details.Type == networkingv1alpha1.PublicKeyConnectorConnectionType &&
196-
details.PublicKey != nil {
197-
nodeID = details.PublicKey.Id
198-
}
197+
nodeID = connector.Status.ConnectionDetails.TunnelNodeID()
199198
}
200199
return online, nodeID
201200
}

internal/extensionserver/cache/index_test.go

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,17 @@ func TestBuildPolicyIndexFromClient_ConnectorResolution_NonConnectorBackend_Skip
910910
// status-fallback classification.
911911
// =============================================================================
912912

913+
// publicKeyDetails builds ConnectionDetails for the PublicKey connection type
914+
// advertising the given node id — the only connection type defined today.
915+
func publicKeyDetails(nodeID string) *networkingv1alpha1.ConnectorConnectionDetails {
916+
return &networkingv1alpha1.ConnectorConnectionDetails{
917+
Type: networkingv1alpha1.PublicKeyConnectorConnectionType,
918+
PublicKey: &networkingv1alpha1.ConnectorConnectionDetailsPublicKey{
919+
Id: nodeID,
920+
},
921+
}
922+
}
923+
913924
// newConnectorWithLiveness builds a Connector carrying only the liveness
914925
// annotation (no status) — simulating a member-cluster replica object.
915926
func newConnectorWithLiveness(ns, name string, liveness networkingv1alpha1.ConnectorLiveness) *networkingv1alpha1.Connector {
@@ -927,10 +938,33 @@ func newConnectorWithLiveness(ns, name string, liveness networkingv1alpha1.Conne
927938

928939
func TestConnectorLiveness_AnnotationOnlineWithNodeID(t *testing.T) {
929940
c := newConnectorWithLiveness("ns", "conn",
930-
networkingv1alpha1.ConnectorLiveness{Ready: true, NodeID: "node-anno"})
941+
networkingv1alpha1.ConnectorLiveness{Ready: true, ConnectionDetails: publicKeyDetails("node-anno")})
931942
online, nodeID := connectorLiveness(c)
932943
assert.True(t, online, "ready annotation must classify online")
933-
assert.Equal(t, "node-anno", nodeID, "nodeID must come from the annotation")
944+
assert.Equal(t, "node-anno", nodeID, "nodeID must come from the annotation's connectionDetails")
945+
}
946+
947+
func TestConnectorLiveness_AnnotationOnlineUnknownTypeEmptyNodeID(t *testing.T) {
948+
// A connection type the extension server does not (yet) understand still
949+
// classifies online from ready, but yields no node ID rather than panicking
950+
// or assuming PublicKey.
951+
c := newConnectorWithLiveness("ns", "conn",
952+
networkingv1alpha1.ConnectorLiveness{
953+
Ready: true,
954+
ConnectionDetails: &networkingv1alpha1.ConnectorConnectionDetails{Type: "FutureType"},
955+
})
956+
online, nodeID := connectorLiveness(c)
957+
assert.True(t, online, "ready annotation must classify online regardless of connection type")
958+
assert.Empty(t, nodeID, "unknown connection type must yield an empty node ID")
959+
}
960+
961+
func TestConnectorLiveness_AnnotationOnlineNilDetailsEmptyNodeID(t *testing.T) {
962+
// Ready but no connection details published yet: online, empty node ID.
963+
c := newConnectorWithLiveness("ns", "conn",
964+
networkingv1alpha1.ConnectorLiveness{Ready: true})
965+
online, nodeID := connectorLiveness(c)
966+
assert.True(t, online, "ready annotation with nil connectionDetails must classify online")
967+
assert.Empty(t, nodeID, "nil connectionDetails must yield an empty node ID")
934968
}
935969

936970
func TestConnectorLiveness_AnnotationOffline(t *testing.T) {
@@ -945,7 +979,7 @@ func TestConnectorLiveness_AnnotationTakesPrecedenceOverStatus(t *testing.T) {
945979
// Annotation says online; status says offline. Annotation wins (it is the
946980
// authoritative liveness on member clusters where status never propagates).
947981
c := newOfflineConnector("ns", "conn")
948-
raw, _ := json.Marshal(networkingv1alpha1.ConnectorLiveness{Ready: true, NodeID: "node-anno"})
982+
raw, _ := json.Marshal(networkingv1alpha1.ConnectorLiveness{Ready: true, ConnectionDetails: publicKeyDetails("node-anno")})
949983
c.Annotations = map[string]string{networkingv1alpha1.ConnectorLivenessAnnotation: string(raw)}
950984

951985
online, nodeID := connectorLiveness(c)
@@ -996,7 +1030,7 @@ func TestBuildPolicyIndexFromClient_ConnectorResolution_AnnotationDrivesOnline(t
9961030

9971031
proxy := newHTTPProxy(upstreamNS, "http://backend.example.com:9000", connectorName)
9981032
connector := newConnectorWithLiveness(upstreamNS, connectorName,
999-
networkingv1alpha1.ConnectorLiveness{Ready: true, NodeID: nodeID})
1033+
networkingv1alpha1.ConnectorLiveness{Ready: true, ConnectionDetails: publicKeyDetails(nodeID)})
10001034

10011035
cl := fake.NewClientBuilder().
10021036
WithScheme(scheme).

0 commit comments

Comments
 (0)