From d097f9c2ff273ec9fd11b15a9e456cd9e31326f2 Mon Sep 17 00:00:00 2001 From: Saikat Chakrabortty Date: Thu, 6 Mar 2025 09:24:32 +0100 Subject: [PATCH 1/4] fix(registry): handle empty targets in TXT records logging an error --- registry/txt.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/registry/txt.go b/registry/txt.go index 1ed7cb0b9..b42993ecc 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -146,6 +146,11 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error continue } // We simply assume that TXT records for the registry will always have only one target. + // If there are no targets (e.g for routing policy based records in google), direct targets will be empty + if len(record.Targets) == 0 { + log.Errorf("TXT record has no targets", record.DNSName) + continue + } labels, err := endpoint.NewLabelsFromString(record.Targets[0], im.txtEncryptAESKey) if errors.Is(err, endpoint.ErrInvalidHeritage) { // if no heritage is found or it is invalid From 4ef055b1be25910f3ab398a0776ecdf202e9d8dc Mon Sep 17 00:00:00 2001 From: Saikat Chakrabortty Date: Thu, 6 Mar 2025 11:48:45 +0100 Subject: [PATCH 2/4] fix(registry): improve logging for TXT records with empty targets and add unit test --- registry/txt.go | 2 +- registry/txt_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/registry/txt.go b/registry/txt.go index b42993ecc..06a8314a7 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -148,7 +148,7 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error // We simply assume that TXT records for the registry will always have only one target. // If there are no targets (e.g for routing policy based records in google), direct targets will be empty if len(record.Targets) == 0 { - log.Errorf("TXT record has no targets", record.DNSName) + log.Errorf("TXT record has no targets %s", record.DNSName) continue } labels, err := endpoint.NewLabelsFromString(record.Targets[0], im.txtEncryptAESKey) diff --git a/registry/txt_test.go b/registry/txt_test.go index cdd190b12..3f8af218d 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -1798,3 +1798,38 @@ func TestApplyChangesWithNewFormatOnly(t *testing.T) { "TXT record should have 'a-' prefix when using new format only") } } + +func TestTXTRegistryRecordsWithEmptyTargets(t *testing.T) { + ctx := context.Background() + p := inmemory.NewInMemoryProvider() + p.CreateZone(testZone) + p.ApplyChanges(ctx, &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "empty-targets.test-zone.example.org", + RecordType: endpoint.RecordTypeTXT, + Targets: endpoint.Targets{}, + }, + { + DNSName: "valid-targets.test-zone.example.org", + RecordType: endpoint.RecordTypeTXT, + Targets: endpoint.Targets{"target1"}, + }, + }, + }) + + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false) + records, err := r.Records(ctx) + require.NoError(t, err) + + expectedRecords := []*endpoint.Endpoint{ + { + DNSName: "valid-targets.test-zone.example.org", + Targets: endpoint.Targets{"target1"}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{}, + }, + } + + assert.True(t, testutils.SameEndpoints(records, expectedRecords)) +} From b0198c883cc2239006e12213b5bda1ea1e8f7279 Mon Sep 17 00:00:00 2001 From: Saikat Chakrabortty Date: Thu, 6 Mar 2025 13:54:33 +0100 Subject: [PATCH 3/4] test(registry): enhance logging for empty targets in TXT records test --- registry/txt_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/registry/txt_test.go b/registry/txt_test.go index 3f8af218d..6cb598b25 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -26,6 +26,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + log "github.com/sirupsen/logrus" + "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/internal/testutils" "sigs.k8s.io/external-dns/plan" @@ -1819,6 +1821,7 @@ func TestTXTRegistryRecordsWithEmptyTargets(t *testing.T) { }) r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false) + b := testutils.LogsToBuffer(log.ErrorLevel, t) records, err := r.Records(ctx) require.NoError(t, err) @@ -1832,4 +1835,5 @@ func TestTXTRegistryRecordsWithEmptyTargets(t *testing.T) { } assert.True(t, testutils.SameEndpoints(records, expectedRecords)) + assert.Contains(t, b.String(), "TXT record has no targets empty-targets.test-zone.example.org") } From 2e0c62ad4545212e3ae3bbc98990b2615c8bfce4 Mon Sep 17 00:00:00 2001 From: Saikat Chakrabortty Date: Thu, 6 Mar 2025 14:07:26 +0100 Subject: [PATCH 4/4] Test author (#1) * fix(aws-sd): service instances registration and deregistration (#5135) * Only de-register removed targets * Use maps for current targets lookup. * Use camelCase, not _ * fix(registry): handle empty targets in TXT records logging an error * fix(registry): improve logging for TXT records with empty targets and add unit test * test(registry): enhance logging for empty targets in TXT records test