From b2f8def9ad601c3a815b6ab2a2e8bded6c067794 Mon Sep 17 00:00:00 2001 From: Mangglesh Dagar Date: Fri, 18 Sep 2026 13:31:09 +0530 Subject: [PATCH] fix(metrics): keep k8s workload names when DNS resolves a destination late WithResolvedDomain replaced both destination workload names with the FQDN, even when a side had already resolved to a k8s workload. That produced labels like name="rabbitmq.rabbit.svc.cluster.local", kind="StatefulSet", which the service map reports as a second copy of the StatefulSet. Apply the same isKubernetesResolved guard that NewDestinationKey got in #298. Co-Authored-By: Claude Opus 5 (1M context) --- common/net.go | 11 +++++++-- common/net_workload_identity_test.go | 37 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/common/net.go b/common/net.go index 8f406ac7..5e3dc5f8 100644 --- a/common/net.go +++ b/common/net.go @@ -352,10 +352,17 @@ func NormalizeFQDN(fqdn string, requestType string) string { return fqdn } +// WithResolvedDomain swaps an IP-named destination for its DNS name. As in +// NewDestinationKey, a side that already resolved to a k8s workload keeps its +// own name; the FQDN is still carried by the `destination` label. func (dk DestinationKey) WithResolvedDomain(fqdn string) DestinationKey { dk.destination = HostPortWithEmptyIP(fqdn, dk.destination.Port()) - dk.destinationWorkload.Name = fqdn - dk.actualDestinationWorkload.Name = fqdn + if !isKubernetesResolved(dk.destinationWorkload) { + dk.destinationWorkload.Name = fqdn + } + if !isKubernetesResolved(dk.actualDestinationWorkload) { + dk.actualDestinationWorkload.Name = fqdn + } return dk } diff --git a/common/net_workload_identity_test.go b/common/net_workload_identity_test.go index a65641e8..81c98084 100644 --- a/common/net_workload_identity_test.go +++ b/common/net_workload_identity_test.go @@ -85,3 +85,40 @@ func TestIsKubernetesResolved(t *testing.T) { assert.True(t, isKubernetesResolved(Workload{Kind: "pod"})) assert.True(t, isKubernetesResolved(Workload{Kind: "node"})) } + +// Regression: WithResolvedDomain is the late path that swaps an IP-named +// destination for its DNS name once the DNS cache fills. It must apply the +// same rule as NewDestinationKey. Here the destination (a ClusterIP) was not +// resolved yet, but the actual destination is a known StatefulSet pod. +// Renaming the pod side produced labels such as +// name="rabbitmq.rabbit.svc.cluster.local", kind="StatefulSet", which the +// service map reports as a second copy of the StatefulSet. +func TestWithResolvedDomain_ResolvedWorkloadKeepsItsName(t *testing.T) { + d := netaddr.IPPortFrom(netaddr.MustParseIP("34.118.0.10"), 5672) + ad := netaddr.IPPortFrom(netaddr.MustParseIP("100.128.0.7"), 5672) + + unresolved := Workload{Name: "34.118.0.10", Namespace: "external", Kind: "external"} + resolved := Workload{Name: "rabbitmq", Namespace: "rabbit", Kind: "StatefulSet"} + + key := NewDestinationKey(d, ad, nil, unresolved, resolved). + WithResolvedDomain("rabbitmq.rabbit.svc.cluster.local") + + assert.Equal(t, "rabbitmq.rabbit.svc.cluster.local", key.destinationWorkload.Name, + "the unresolved side still takes the FQDN") + assert.Equal(t, "rabbitmq", key.actualDestinationWorkload.Name, + "a k8s-resolved workload must keep its own name") + assert.Equal(t, "StatefulSet", key.actualDestinationWorkload.Kind) + assert.Equal(t, "rabbitmq.rabbit.svc.cluster.local:5672", key.Destination().String()) +} + +// A genuinely external destination on both sides still takes the FQDN. +func TestWithResolvedDomain_ExternalTakesFQDN(t *testing.T) { + d := netaddr.IPPortFrom(netaddr.MustParseIP("52.0.0.1"), 443) + external := Workload{Name: "52.0.0.1", Namespace: "external", Kind: "external"} + + key := NewDestinationKey(d, d, nil, external, external). + WithResolvedDomain("kms.us-east-1.amazonaws.com") + + assert.Equal(t, "kms.us-east-1.amazonaws.com", key.destinationWorkload.Name) + assert.Equal(t, "kms.us-east-1.amazonaws.com", key.actualDestinationWorkload.Name) +}