From 80cc379444f03b927d68bdf4de3fba13af3450a2 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Fri, 19 Dec 2025 11:58:09 +0000 Subject: [PATCH 1/3] chore(wrapper): centralized endpoint target validation Signed-off-by: ivan katliarchuk --- endpoint/endpoint.go | 1 + source/wrappers/dedupsource.go | 6 ++ source/wrappers/dedupsource_test.go | 127 +++++++++++++++++++++++++--- 3 files changed, 123 insertions(+), 11 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 68c8f630b1..42d7f1d1e4 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -423,6 +423,7 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { return result } +// TODO: rename to Validate // CheckEndpoint Check if endpoint is properly formatted according to RFC standards func (e *Endpoint) CheckEndpoint() bool { switch recordType := e.RecordType; recordType { diff --git a/source/wrappers/dedupsource.go b/source/wrappers/dedupsource.go index 1fde740988..75de8d3cda 100644 --- a/source/wrappers/dedupsource.go +++ b/source/wrappers/dedupsource.go @@ -52,6 +52,12 @@ func (ms *dedupSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, err continue } + // validate endpoint before normalization + if ok := ep.CheckEndpoint(); !ok { + log.Warnf("Skipping endpoint [%s:%s] due to invalid targets [%s:%s]", ep.SetIdentifier, ep.DNSName, ep.RecordType, strings.Join(ep.Targets, ",")) + continue + } + if len(ep.Targets) > 1 { ep.Targets = endpoint.NewTargets(ep.Targets...) } diff --git a/source/wrappers/dedupsource_test.go b/source/wrappers/dedupsource_test.go index 5af481398a..64a4d8d7cb 100644 --- a/source/wrappers/dedupsource_test.go +++ b/source/wrappers/dedupsource_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + "github.com/stretchr/testify/require" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/internal/testutils" "sigs.k8s.io/external-dns/source" @@ -94,32 +95,32 @@ func testDedupEndpoints(t *testing.T) { { "two endpoints with same dnsname, same type, and same target return one endpoint", []*endpoint.Endpoint{ - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4"}}, - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, }, []*endpoint.Endpoint{ - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, }, }, { "two endpoints with same dnsname, different record type, and same target return two endpoints", []*endpoint.Endpoint{ - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4"}}, - {DNSName: "foo.example.org", RecordType: "AAAA", Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"1.2.3.4"}}, }, []*endpoint.Endpoint{ - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4"}}, - {DNSName: "foo.example.org", RecordType: "AAAA", Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"1.2.3.4"}}, }, }, { "two endpoints with same dnsname, one with record type, one without, and same target return two endpoints", []*endpoint.Endpoint{ - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"1.2.3.4"}}, }, []*endpoint.Endpoint{ - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, {DNSName: "foo.example.org", Targets: endpoint.Targets{"1.2.3.4"}}, }, }, @@ -131,10 +132,10 @@ func testDedupEndpoints(t *testing.T) { { "one endpoint with multiple targets returns one endpoint and targets without duplicates", []*endpoint.Endpoint{ - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4", "34.66.66.77", "34.66.66.77"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4", "34.66.66.77", "34.66.66.77"}}, }, []*endpoint.Endpoint{ - {DNSName: "foo.example.org", RecordType: "A", Targets: endpoint.Targets{"1.2.3.4", "34.66.66.77"}}, + {DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4", "34.66.66.77"}}, }, }, } { @@ -182,3 +183,107 @@ func TestDedupSource_AddEventHandler(t *testing.T) { }) } } + +func TestDedupEndpointsValidation(t *testing.T) { + tests := []struct { + name string + endpoints []*endpoint.Endpoint + expected []*endpoint.Endpoint + }{ + { + name: "mix of SRV records", + endpoints: []*endpoint.Endpoint{ + {DNSName: "_service._tcp.example.org", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"10 5 443 target.example.org."}}, // valid + {DNSName: "_service._tcp.example.org", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"11 5 target.example.org"}}, // invalid + }, + expected: []*endpoint.Endpoint{ + {DNSName: "_service._tcp.example.org", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"10 5 443 target.example.org."}}, + }, + }, + { + name: "invalid SRV record - missing priority", + endpoints: []*endpoint.Endpoint{ + {DNSName: "_service._tcp.example.org", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"5 443 target.example.org"}}, + }, + expected: []*endpoint.Endpoint{}, + }, + { + name: "valid MX record", + endpoints: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeMX, Targets: endpoint.Targets{"10 mail.example.org"}}, + }, + expected: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeMX, Targets: endpoint.Targets{"10 mail.example.org"}}, + }, + }, + { + name: "invalid MX record - missing priority", + endpoints: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeMX, Targets: endpoint.Targets{"mail.example.org"}}, + }, + expected: []*endpoint.Endpoint{}, + }, + { + name: "valid NAPTR record", + endpoints: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeNAPTR, Targets: endpoint.Targets{"100 10 \"u\" \"E2U+sip\" \"!^.*$!sip:info@example.org!\" ."}}, + }, + expected: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeNAPTR, Targets: endpoint.Targets{"100 10 \"u\" \"E2U+sip\" \"!^.*$!sip:info@example.org!\" ."}}, + }, + }, + { + name: "invalid NAPTR record - incomplete format", + endpoints: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeNAPTR, Targets: endpoint.Targets{"100 10 \"u\""}}, // invalid + }, + expected: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeNAPTR, Targets: endpoint.Targets{"100 10 \"u\""}}, + }, + }, + { + name: "mixed valid and invalid records", + endpoints: []*endpoint.Endpoint{ + {DNSName: "_service._tcp.example.org", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"10 5 443"}}, // invalid + {DNSName: "example.org", RecordType: endpoint.RecordTypeMX, Targets: endpoint.Targets{"mail.example.org"}}, // invalid + {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, + }, + expected: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, + }, + }, + { + name: "mixed valid and invalid TXT, A, AAAA records", + endpoints: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeTXT, Targets: endpoint.Targets{"v=spf1 include:example.com ~all"}}, // valid + {DNSName: "example.org", RecordType: endpoint.RecordTypeTXT, Targets: endpoint.Targets{""}}, // invalid + {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"192.168.1.1"}}, // valid + {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"not-an-ip"}}, // invalid + {DNSName: "example.org", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:db8::1"}}, // valid + {DNSName: "example.org", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"invalid-ipv6"}}, // invalid + }, + expected: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeTXT, Targets: endpoint.Targets{"v=spf1 include:example.com ~all"}}, + {DNSName: "example.org", RecordType: endpoint.RecordTypeTXT, Targets: endpoint.Targets{""}}, + {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"192.168.1.1"}}, + {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"not-an-ip"}}, + {DNSName: "example.org", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:db8::1"}}, + {DNSName: "example.org", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"invalid-ipv6"}}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockSource := new(testutils.MockSource) + mockSource.On("Endpoints").Return(tt.endpoints, nil) + + sr := NewDedupSource(mockSource) + endpoints, err := sr.Endpoints(context.Background()) + require.NoError(t, err) + + validateEndpoints(t, endpoints, tt.expected) + mockSource.AssertExpectations(t) + }) + } +} From e90448dc18ead7d7473f5b2b3e3c494227079d75 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Fri, 19 Dec 2025 12:44:24 +0000 Subject: [PATCH 2/3] chore(wrapper): centralized endpoint target validation Signed-off-by: ivan katliarchuk --- source/wrappers/dedupsource_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/source/wrappers/dedupsource_test.go b/source/wrappers/dedupsource_test.go index 64a4d8d7cb..7d84fa28a8 100644 --- a/source/wrappers/dedupsource_test.go +++ b/source/wrappers/dedupsource_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/internal/testutils" @@ -287,3 +288,23 @@ func TestDedupEndpointsValidation(t *testing.T) { }) } } + +func TestDedupSource_WarnsOnInvalidEndpoint(t *testing.T) { + hook := testutils.LogsUnderTestWithLogLevel(log.WarnLevel, t) + + invalidEndpoint := &endpoint.Endpoint{ + DNSName: "example.org", + RecordType: endpoint.RecordTypeSRV, + SetIdentifier: "default/svc/my-service", + Targets: endpoint.Targets{"10 mail.example.org"}, + } + + mockSource := new(testutils.MockSource) + mockSource.On("Endpoints").Return([]*endpoint.Endpoint{invalidEndpoint}, nil) + + src := NewDedupSource(mockSource) + _, err := src.Endpoints(context.Background()) + require.NoError(t, err) + + testutils.TestHelperLogContains("Skipping endpoint [default/svc/my-service:example.org] due to invalid targets [SRV:10 mail.example.org]", hook, t) +} From 604d5126c133424c9ca6e37616359cbab75e5431 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Fri, 19 Dec 2025 12:47:02 +0000 Subject: [PATCH 3/3] chore(wrapper): centralized endpoint target validation Signed-off-by: ivan katliarchuk --- source/wrappers/dedupsource.go | 2 +- source/wrappers/dedupsource_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/wrappers/dedupsource.go b/source/wrappers/dedupsource.go index 75de8d3cda..11c4295049 100644 --- a/source/wrappers/dedupsource.go +++ b/source/wrappers/dedupsource.go @@ -54,7 +54,7 @@ func (ms *dedupSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, err // validate endpoint before normalization if ok := ep.CheckEndpoint(); !ok { - log.Warnf("Skipping endpoint [%s:%s] due to invalid targets [%s:%s]", ep.SetIdentifier, ep.DNSName, ep.RecordType, strings.Join(ep.Targets, ",")) + log.Warnf("Skipping endpoint [%s:%s] due to invalid configuration [%s:%s]", ep.SetIdentifier, ep.DNSName, ep.RecordType, strings.Join(ep.Targets, ",")) continue } diff --git a/source/wrappers/dedupsource_test.go b/source/wrappers/dedupsource_test.go index 7d84fa28a8..9ebaa2f8a7 100644 --- a/source/wrappers/dedupsource_test.go +++ b/source/wrappers/dedupsource_test.go @@ -306,5 +306,5 @@ func TestDedupSource_WarnsOnInvalidEndpoint(t *testing.T) { _, err := src.Endpoints(context.Background()) require.NoError(t, err) - testutils.TestHelperLogContains("Skipping endpoint [default/svc/my-service:example.org] due to invalid targets [SRV:10 mail.example.org]", hook, t) + testutils.TestHelperLogContains("Skipping endpoint [default/svc/my-service:example.org] due to invalid configuration [SRV:10 mail.example.org]", hook, t) }