From 81b49e3d0ad97614b4730b48ac309540090b4c59 Mon Sep 17 00:00:00 2001 From: Christian Rohmann Date: Mon, 16 Jun 2025 23:56:49 +0200 Subject: [PATCH] fix: append dot to the target of SRV records as required by RFC 2782 According to RFC2782 [1], SRV records are to be fully-qualified, ending with a final dot. Most providers are liberal, but e.g. OpenStack Designate expects targets to end with a dot ([2]) [1] https://datatracker.ietf.org/doc/html/rfc2782 [2] https://github.com/openstack/designate/blob/a891d81974a4c3aa56a143252b26df9dcbd5fba5/designate/objects/fields.py#L235-L236 Signed-off-by: Christian Rohmann --- endpoint/endpoint.go | 13 +++++++++---- endpoint/endpoint_test.go | 17 +++++++++++++---- source/service.go | 3 ++- source/service_fqdn_test.go | 20 ++++++++++---------- source/service_test.go | 20 ++++++++++---------- 5 files changed, 44 insertions(+), 29 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index b1a0543607..68c8f630b1 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -260,8 +260,9 @@ func NewEndpointWithTTL(dnsName, recordType string, ttl TTL, targets ...string) for idx, target := range targets { // Only trim trailing dots for domain name record types, not for TXT or NAPTR records // TXT records can contain arbitrary text including multiple dots + // SRV can contain dots in their target part (RFC2782) switch recordType { - case RecordTypeTXT, RecordTypeNAPTR: + case RecordTypeTXT, RecordTypeNAPTR, RecordTypeSRV: cleanTargets[idx] = target default: cleanTargets[idx] = strings.TrimSuffix(target, ".") @@ -484,11 +485,15 @@ func (t Targets) ValidateMXRecord() bool { func (t Targets) ValidateSRVRecord() bool { for _, target := range t { - // SRV records must have a priority, weight, and port value, e.g. "10 5 5060 example.com" - // as per https://www.rfc-editor.org/rfc/rfc2782.txt + // SRV records must have a priority, weight, a port value and a target e.g. "10 5 5060 example.com." + // as per https://www.rfc-editor.org/rfc/rfc2782.txt the target host has to end with a dot. targetParts := strings.Fields(strings.TrimSpace(target)) if len(targetParts) != 4 { - log.Debugf("Invalid SRV record target: %s. SRV records must have a priority, weight, and port value, e.g. '10 5 5060 example.com'", target) + log.Debugf("Invalid SRV record target: %s. SRV records must have a priority, weight, a port value and a target host, e.g. '10 5 5060 example.com.'", target) + return false + } + if !strings.HasSuffix(targetParts[3], ".") { + log.Debugf("Invalid SRV record target: %s. Target host does not end with a dot.'", target) return false } diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index ef5fbc7821..beea285ba8 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -787,7 +787,7 @@ func TestPDNScheckEndpoint(t *testing.T) { endpoint: Endpoint{ DNSName: "_service._tls.example.com", RecordType: RecordTypeSRV, - Targets: Targets{"10 20 5060 service.example.com"}, + Targets: Targets{"10 20 5060 service.example.com."}, }, expected: true, }, @@ -805,7 +805,16 @@ func TestPDNScheckEndpoint(t *testing.T) { endpoint: Endpoint{ DNSName: "_service._tls.example.com", RecordType: RecordTypeSRV, - Targets: Targets{"10 20 abc service.example.com"}, + Targets: Targets{"10 20 abc service.example.com."}, + }, + expected: false, + }, + { + description: "Invalid SRV record with missing dot for target host", + endpoint: Endpoint{ + DNSName: "_service._tls.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 20 5060 service.example.com"}, }, expected: false, }, @@ -895,7 +904,7 @@ func TestCheckEndpoint(t *testing.T) { endpoint: Endpoint{ DNSName: "_service._tcp.example.com", RecordType: RecordTypeSRV, - Targets: Targets{"10 5 5060 example.com"}, + Targets: Targets{"10 5 5060 example.com."}, }, expected: true, }, @@ -904,7 +913,7 @@ func TestCheckEndpoint(t *testing.T) { endpoint: Endpoint{ DNSName: "_service._tcp.example.com", RecordType: RecordTypeSRV, - Targets: Targets{"10 5 example.com"}, + Targets: Targets{"10 5 example.com."}, }, expected: false, }, diff --git a/source/service.go b/source/service.go index afc7289e70..636ac1d21a 100644 --- a/source/service.go +++ b/source/service.go @@ -38,6 +38,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" + "sigs.k8s.io/external-dns/provider" "sigs.k8s.io/external-dns/source/informers" "sigs.k8s.io/external-dns/source/annotations" @@ -855,7 +856,7 @@ func (sc *serviceSource) extractNodePortEndpoints(svc *v1.Service, hostname stri // see https://en.wikipedia.org/wiki/SRV_record // build a target with a priority of 0, weight of 50, and pointing the given port on the given host - target := fmt.Sprintf("0 50 %d %s", port.NodePort, hostname) + target := fmt.Sprintf("0 50 %d %s", port.NodePort, provider.EnsureTrailingDot(hostname)) // take the service name from the K8s Service object // it is safe to use since it is DNS compatible diff --git a/source/service_fqdn_test.go b/source/service_fqdn_test.go index 3132be4d27..459cd42bbc 100644 --- a/source/service_fqdn_test.go +++ b/source/service_fqdn_test.go @@ -412,7 +412,7 @@ func TestServiceSourceFqdnTemplatingExamples(t *testing.T) { }, }, }, - fqdnTemplate: `{{ $name := .Name }}{{ range .Spec.Ports -}}{{ $name }}{{ if eq .Name "http2" }}.http2{{ else if eq .Name "debug" }}.debug{{ end }}.example.tld{{printf "," }}{{ end }}`, + fqdnTemplate: `{{ $name := .Name }}{{ range .Spec.Ports -}}{{ $name }}{{ if eq .Name "http2" }}.http2{{ else if eq .Name "debug" }}.debug{{ end }}.example.tld.{{printf "," }}{{ end }}`, expected: []*endpoint.Endpoint{ // TODO: This test shows that there is a bug that needs to be fixed in the external-dns logic. {DNSName: "", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"192.51.100.22", "192.51.100.5"}}, @@ -736,15 +736,15 @@ func TestServiceSourceFqdnTemplatingExamples(t *testing.T) { expected: []*endpoint.Endpoint{ // TODO: This test shows that there is a bug that needs to be fixed in the external-dns logic. Not a critical issue, as will be filtered out by the source. {DNSName: "", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"10.96.41.132", "203.0.113.10"}}, - {DNSName: "_service-one._tcp", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30080 ", "0 50 30082 "}}, // TODO: wrong SRV target format - {DNSName: "_service-one._tcp.debug.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30080 debug.host.tld", "0 50 30082 debug.host.tld"}}, - {DNSName: "_service-one._tcp.http.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30080 http.host.tld", "0 50 30082 http.host.tld"}}, - {DNSName: "_service-three._tcp", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 25565 "}}, // TODO: wrong SRV target format - {DNSName: "_service-three._tcp.debug.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 25565 debug.host.tld"}}, - {DNSName: "_service-three._tcp.minecraft.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 25565 minecraft.host.tld"}}, - {DNSName: "_service-three._udp", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30083 "}}, // TODO: wrong SRV target format - {DNSName: "_service-three._udp.debug.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30083 debug.host.tld"}}, - {DNSName: "_service-three._udp.minecraft.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30083 minecraft.host.tld"}}, + {DNSName: "_service-one._tcp", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30080 .", "0 50 30082 ."}}, // TODO: wrong SRV target format + {DNSName: "_service-one._tcp.debug.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30080 debug.host.tld.", "0 50 30082 debug.host.tld."}}, + {DNSName: "_service-one._tcp.http.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30080 http.host.tld.", "0 50 30082 http.host.tld."}}, + {DNSName: "_service-three._tcp", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 25565 ."}}, // TODO: wrong SRV target format + {DNSName: "_service-three._tcp.debug.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 25565 debug.host.tld."}}, + {DNSName: "_service-three._tcp.minecraft.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 25565 minecraft.host.tld."}}, + {DNSName: "_service-three._udp", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30083 ."}}, // TODO: wrong SRV target format + {DNSName: "_service-three._udp.debug.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30083 debug.host.tld."}}, + {DNSName: "_service-three._udp.minecraft.host.tld", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"0 50 30083 minecraft.host.tld."}}, {DNSName: "debug.host.tld", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"203.0.113.10"}}, {DNSName: "http.host.tld", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"203.0.113.10"}}, {DNSName: "minecraft.host.tld", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"203.0.113.10"}}, diff --git a/source/service_test.go b/source/service_test.go index 6d9e3a8d8f..5fe1e8d5da 100644 --- a/source/service_test.go +++ b/source/service_test.go @@ -1741,7 +1741,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { annotations.HostnameKey: "foo.example.org.", }, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"54.10.11.1", "54.10.11.2"}, RecordType: endpoint.RecordTypeA}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::3"}, RecordType: endpoint.RecordTypeAAAA}, }, @@ -1816,7 +1816,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { svcTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, fqdnTemplate: "{{.Name}}.bar.example.com", expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.bar.example.com", Targets: endpoint.Targets{"0 50 30192 foo.bar.example.com"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.bar.example.com", Targets: endpoint.Targets{"0 50 30192 foo.bar.example.com."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.bar.example.com", Targets: endpoint.Targets{"54.10.11.1", "54.10.11.2"}, RecordType: endpoint.RecordTypeA}, {DNSName: "foo.bar.example.com", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::3"}, RecordType: endpoint.RecordTypeAAAA}, }, @@ -1856,7 +1856,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { annotations.HostnameKey: "foo.example.org.", }, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"10.0.1.1", "10.0.1.2"}, RecordType: endpoint.RecordTypeA}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::2"}, RecordType: endpoint.RecordTypeAAAA}, }, @@ -1892,7 +1892,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { annotations.HostnameKey: "foo.example.org.", }, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"54.10.11.2"}, RecordType: endpoint.RecordTypeA}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"2001:DB8::3"}, RecordType: endpoint.RecordTypeAAAA}, }, @@ -1938,7 +1938,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { annotations.HostnameKey: "foo.example.org.", }, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"54.10.11.2"}, RecordType: endpoint.RecordTypeA}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"2001:DB8::3"}, RecordType: endpoint.RecordTypeAAAA}, }, @@ -1987,7 +1987,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { annotations.HostnameKey: "foo.example.org.", }, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"54.10.11.1"}, RecordType: endpoint.RecordTypeA}, }, nodes: []*v1.Node{{ @@ -2031,7 +2031,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { annotations.HostnameKey: "foo.example.org.", }, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"54.10.11.1"}, RecordType: endpoint.RecordTypeA}, }, nodes: []*v1.Node{{ @@ -2087,7 +2087,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { annotations.AccessKey: "private", }, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"10.0.1.1", "10.0.1.2"}, RecordType: endpoint.RecordTypeA}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::2"}, RecordType: endpoint.RecordTypeAAAA}, }, @@ -2127,7 +2127,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { annotations.AccessKey: "public", }, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"54.10.11.1", "54.10.11.2"}, RecordType: endpoint.RecordTypeA}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::3"}, RecordType: endpoint.RecordTypeAAAA}, }, @@ -2170,7 +2170,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { }, exposeInternalIPv6: true, expected: []*endpoint.Endpoint{ - {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org"}, RecordType: endpoint.RecordTypeSRV}, + {DNSName: "_foo._tcp.foo.example.org", Targets: endpoint.Targets{"0 50 30192 foo.example.org."}, RecordType: endpoint.RecordTypeSRV}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"54.10.11.1", "54.10.11.2"}, RecordType: endpoint.RecordTypeA}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::2", "2001:DB8::3", "2001:DB8::4"}, RecordType: endpoint.RecordTypeAAAA}, },