Skip to content

Commit cbb4482

Browse files
committed
fix(awssd): use namespace-aware parsing for dotted service names
parseHostname splits at the first dot, which breaks when Cloud Map service names contain dots (e.g. consumer-gen-mcp.elb in namespace dev.local). The naive split produces namespace=elb.dev.local instead of namespace=dev.local, causing the record to be silently skipped. Change parseHostname to accept known namespaces and use longest-suffix matching, falling back to the original first-dot split when no namespace matches for backward compatibility. Fixes #6364 Ref #5714 Signed-off-by: Andrew Moes <andrew.moes@rewardstyle.com> Made-with: Cursor
1 parent dc28c3e commit cbb4482

2 files changed

Lines changed: 154 additions & 5 deletions

File tree

provider/awssd/aws_sd.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ func (p *AWSSDProvider) submitCreates(ctx context.Context, namespaces []*sdtypes
310310
}
311311

312312
for _, ch := range changeList {
313-
_, srvName := p.parseHostname(ch.DNSName)
313+
_, srvName := p.parseHostname(ch.DNSName, namespaces)
314314

315315
srv := services[srvName]
316316
if srv == nil {
@@ -350,7 +350,7 @@ func (p *AWSSDProvider) submitDeletes(ctx context.Context, namespaces []*sdtypes
350350

351351
for _, ch := range changeList {
352352
hostname := ch.DNSName
353-
_, srvName := p.parseHostname(hostname)
353+
_, srvName := p.parseHostname(hostname, namespaces)
354354

355355
srv := services[srvName]
356356
if srv == nil {
@@ -605,7 +605,7 @@ func (p *AWSSDProvider) changesByNamespaceID(namespaces []*sdtypes.NamespaceSumm
605605
for _, c := range changes {
606606
// trim the trailing dot from hostname if any
607607
hostname := strings.TrimSuffix(c.DNSName, ".")
608-
nsName, _ := p.parseHostname(hostname)
608+
nsName, _ := p.parseHostname(hostname, namespaces)
609609

610610
matchingNamespaces := matchingNamespaces(nsName, namespaces)
611611
if len(matchingNamespaces) == 0 {
@@ -640,8 +640,26 @@ func matchingNamespaces(hostname string, namespaces []*sdtypes.NamespaceSummary)
640640
return matchingNamespaces
641641
}
642642

643-
// parseHostname parse hostname to namespace (domain) and service
644-
func (p *AWSSDProvider) parseHostname(hostname string) (string, string) {
643+
// parseHostname parses hostname into namespace (domain) and service name.
644+
// When known namespaces are provided, it uses longest-suffix matching to
645+
// correctly handle service names that contain dots (e.g. "foo.bar.dev.local"
646+
// with namespace "dev.local" yields service "foo.bar"). Falls back to the
647+
// original first-dot split when no namespace suffix matches.
648+
func (p *AWSSDProvider) parseHostname(hostname string, namespaces []*sdtypes.NamespaceSummary) (string, string) {
649+
var bestNS, bestSrv string
650+
for _, ns := range namespaces {
651+
suffix := "." + aws.ToString(ns.Name)
652+
if strings.HasSuffix(hostname, suffix) {
653+
candidate := strings.TrimSuffix(hostname, suffix)
654+
if bestNS == "" || len(aws.ToString(ns.Name)) > len(bestNS) {
655+
bestNS = aws.ToString(ns.Name)
656+
bestSrv = candidate
657+
}
658+
}
659+
}
660+
if bestNS != "" {
661+
return bestNS, bestSrv
662+
}
645663
parts := strings.Split(hostname, ".")
646664
return strings.Join(parts[1:], "."), parts[0]
647665
}

provider/awssd/aws_sd_test.go

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,56 @@ func TestAWSSDProvider_ApplyChanges_Update(t *testing.T) {
280280
assert.Equal(t, "1.2.3.5", api.deregistered[0], "wrong target de-registered")
281281
}
282282

283+
func TestAWSSDProvider_ApplyChanges_DottedServiceName(t *testing.T) {
284+
namespaces := map[string]*sdtypes.Namespace{
285+
"dev-local": {
286+
Id: aws.String("dev-local"),
287+
Name: aws.String("dev.local"),
288+
Type: sdtypes.NamespaceTypeDnsPrivate,
289+
},
290+
}
291+
292+
api := &AWSSDClientStub{
293+
namespaces: namespaces,
294+
services: make(map[string]map[string]*sdtypes.Service),
295+
instances: make(map[string]map[string]*sdtypes.Instance),
296+
}
297+
298+
createEndpoints := []*endpoint.Endpoint{
299+
{DNSName: "consumer-gen-mcp.elb.dev.local", Targets: endpoint.Targets{"1.2.3.4"}, RecordType: endpoint.RecordTypeA, RecordTTL: 60},
300+
}
301+
302+
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{"dev.local"}), "", "")
303+
304+
ctx := t.Context()
305+
306+
err := provider.ApplyChanges(ctx, &plan.Changes{
307+
Create: createEndpoints,
308+
})
309+
require.NoError(t, err)
310+
311+
// service must be created with the dotted name "consumer-gen-mcp.elb"
312+
assert.Len(t, api.services["dev-local"], 1)
313+
existingServices, err := provider.ListServicesByNamespaceID(ctx, namespaces["dev-local"].Id)
314+
require.NoError(t, err)
315+
assert.NotNil(t, existingServices["consumer-gen-mcp.elb"], "service should be named 'consumer-gen-mcp.elb'")
316+
317+
// verify the record round-trips through Records()
318+
endpoints, err := provider.Records(ctx)
319+
require.NoError(t, err)
320+
assert.True(t, testutils.SameEndpoints(createEndpoints, endpoints),
321+
"expected and actual endpoints don't match, expected=%v, actual=%v", createEndpoints, endpoints)
322+
323+
// apply deletes
324+
err = provider.ApplyChanges(ctx, &plan.Changes{
325+
Delete: createEndpoints,
326+
})
327+
require.NoError(t, err)
328+
329+
endpoints, _ = provider.Records(ctx)
330+
assert.Empty(t, endpoints)
331+
}
332+
283333
func TestAWSSDProvider_ListNamespaces(t *testing.T) {
284334
namespaces := map[string]*sdtypes.Namespace{
285335
"private": {
@@ -1042,3 +1092,84 @@ func TestAWSSDProvider_awsTags(t *testing.T) {
10421092
require.ElementsMatch(t, test.Expectation, awsTags(test.Input))
10431093
}
10441094
}
1095+
1096+
func TestAWSSDProvider_parseHostname(t *testing.T) {
1097+
tests := []struct {
1098+
name string
1099+
hostname string
1100+
namespaces []*sdtypes.NamespaceSummary
1101+
wantNS string
1102+
wantSrv string
1103+
}{
1104+
{
1105+
name: "simple service name",
1106+
hostname: "foo.dev.local",
1107+
namespaces: []*sdtypes.NamespaceSummary{
1108+
{Name: aws.String("dev.local")},
1109+
},
1110+
wantNS: "dev.local",
1111+
wantSrv: "foo",
1112+
},
1113+
{
1114+
name: "dotted service name",
1115+
hostname: "foo.bar.dev.local",
1116+
namespaces: []*sdtypes.NamespaceSummary{
1117+
{Name: aws.String("dev.local")},
1118+
},
1119+
wantNS: "dev.local",
1120+
wantSrv: "foo.bar",
1121+
},
1122+
{
1123+
name: "SRV-style hostname",
1124+
hostname: "_tcp.backend.mynet.internal",
1125+
namespaces: []*sdtypes.NamespaceSummary{
1126+
{Name: aws.String("mynet.internal")},
1127+
},
1128+
wantNS: "mynet.internal",
1129+
wantSrv: "_tcp.backend",
1130+
},
1131+
{
1132+
name: "longest namespace match wins",
1133+
hostname: "foo.a.b.c",
1134+
namespaces: []*sdtypes.NamespaceSummary{
1135+
{Name: aws.String("b.c")},
1136+
{Name: aws.String("a.b.c")},
1137+
},
1138+
wantNS: "a.b.c",
1139+
wantSrv: "foo",
1140+
},
1141+
{
1142+
name: "no matching namespace falls back to first-dot split",
1143+
hostname: "foo.unknown.tld",
1144+
namespaces: []*sdtypes.NamespaceSummary{
1145+
{Name: aws.String("dev.local")},
1146+
},
1147+
wantNS: "unknown.tld",
1148+
wantSrv: "foo",
1149+
},
1150+
{
1151+
name: "empty namespaces falls back to first-dot split",
1152+
hostname: "foo.bar.baz",
1153+
namespaces: []*sdtypes.NamespaceSummary{},
1154+
wantNS: "bar.baz",
1155+
wantSrv: "foo",
1156+
},
1157+
{
1158+
name: "nil namespaces falls back to first-dot split",
1159+
hostname: "foo.bar.baz",
1160+
namespaces: nil,
1161+
wantNS: "bar.baz",
1162+
wantSrv: "foo",
1163+
},
1164+
}
1165+
1166+
provider := &AWSSDProvider{}
1167+
1168+
for _, tc := range tests {
1169+
t.Run(tc.name, func(t *testing.T) {
1170+
gotNS, gotSrv := provider.parseHostname(tc.hostname, tc.namespaces)
1171+
assert.Equal(t, tc.wantNS, gotNS)
1172+
assert.Equal(t, tc.wantSrv, gotSrv)
1173+
})
1174+
}
1175+
}

0 commit comments

Comments
 (0)