From 013f8cf9e93057a90b68ad3e917e76f754b1f929 Mon Sep 17 00:00:00 2001 From: Seweryn Chlewicki Date: Fri, 23 Jun 2023 11:52:46 +0100 Subject: [PATCH] Fix `TXT` registry handling of `NewTXT` domains --- registry/txt.go | 39 +++++++++++------- registry/txt_test.go | 97 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 113 insertions(+), 23 deletions(-) diff --git a/registry/txt.go b/registry/txt.go index e9411185b..4ea92d316 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -321,7 +321,9 @@ func newaffixNameMapper(prefix, suffix, wildcardReplacement string) affixNameMap return affixNameMapper{prefix: strings.ToLower(prefix), suffix: strings.ToLower(suffix), wildcardReplacement: strings.ToLower(wildcardReplacement)} } -func extractRecordType(name string) (baseName, recordType string) { +// extractRecordTypeDefaultPosition extracts record type from the default position +// when not using '%{record_type}' in the prefix/suffix +func extractRecordTypeDefaultPosition(name string) (baseName, recordType string) { nameS := strings.Split(name, "-") for _, t := range getSupportedTypes() { if nameS[0] == strings.ToLower(t) { @@ -331,32 +333,34 @@ func extractRecordType(name string) (baseName, recordType string) { return name, "" } -// dropAffix strips TXT record to find an endpoint name it manages -// It takes into consideration a fact that it could contain record type -// So it gets stripped first -func (pr affixNameMapper) dropAffix(name string) string { +// dropAffixExtractType strips TXT record to find an endpoint name it manages +// it also returns the record type +func (pr affixNameMapper) dropAffixExtractType(name string) (string, string) { if pr.recordTypeInAffix() { for _, t := range getSupportedTypes() { t = strings.ToLower(t) iPrefix := strings.ReplaceAll(pr.prefix, recordTemplate, t) iSuffix := strings.ReplaceAll(pr.suffix, recordTemplate, t) + if pr.isPrefix() && strings.HasPrefix(name, iPrefix) { - return strings.TrimPrefix(name, iPrefix) + return strings.TrimPrefix(name, iPrefix), t } if pr.isSuffix() && strings.HasSuffix(name, iSuffix) { - return strings.TrimSuffix(name, iSuffix) + return strings.TrimSuffix(name, iSuffix), t } } } + if strings.HasPrefix(name, pr.prefix) && pr.isPrefix() { - return strings.TrimPrefix(name, pr.prefix) + return extractRecordTypeDefaultPosition(strings.TrimPrefix(name, pr.prefix)) } if strings.HasSuffix(name, pr.suffix) && pr.isSuffix() { - return strings.TrimSuffix(name, pr.suffix) + return extractRecordTypeDefaultPosition(strings.TrimSuffix(name, pr.suffix)) } - return "" + + return "", "" } func (pr affixNameMapper) dropAffixTemplate(name string) string { @@ -372,17 +376,22 @@ func (pr affixNameMapper) isSuffix() bool { } func (pr affixNameMapper) toEndpointName(txtDNSName string) (endpointName string, isAAAA bool) { - lowerDNSName, recordType := extractRecordType(strings.ToLower(txtDNSName)) + lowerDNSName := strings.ToLower(txtDNSName) // drop prefix - if strings.HasPrefix(lowerDNSName, pr.prefix) && pr.isPrefix() { - return pr.dropAffix(lowerDNSName), recordType == endpoint.RecordTypeAAAA + if pr.isPrefix() { + r, rType := pr.dropAffixExtractType(lowerDNSName) + return r, rType == endpoint.RecordTypeAAAA } // drop suffix if pr.isSuffix() { - DNSName := strings.SplitN(lowerDNSName, ".", 2) - return pr.dropAffix(DNSName[0]) + "." + DNSName[1], recordType == endpoint.RecordTypeAAAA + dc := strings.Count(pr.suffix, ".") + DNSName := strings.SplitN(lowerDNSName, ".", 2+dc) + domainWithSuffix := strings.Join(DNSName[:1+dc], ".") + + r, rType := pr.dropAffixExtractType(domainWithSuffix) + return r + "." + DNSName[1+dc], rType == endpoint.RecordTypeAAAA } return "", false } diff --git a/registry/txt_test.go b/registry/txt_test.go index 2493fbb11..ce6d21fb9 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -114,7 +114,7 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { newEndpointWithOwner("dualstack.test-zone.example.org", "1.1.1.1", endpoint.RecordTypeA, ""), newEndpointWithOwner("txt.dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("dualstack.test-zone.example.org", "2001:DB8::1", endpoint.RecordTypeAAAA, ""), - newEndpointWithOwner("aaaa-txt.dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("txt.aaaa-dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -216,13 +216,13 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "wc", []string{}, false, nil) records, _ := r.Records(ctx) - assert.True(t, testutils.SameEndpoints(records, expectedRecords)) + assert.ElementsMatch(t, expectedRecords, records) // Ensure prefix is case-insensitive r, _ = NewTXTRegistry(p, "TxT.", "", "owner", time.Hour, "", []string{}, false, nil) records, _ = r.Records(ctx) - assert.True(t, testutils.SameEndpointLabels(records, expectedRecords)) + assert.ElementsMatch(t, expectedRecords, records) } func testTXTRegistryRecordsSuffixed(t *testing.T) { @@ -1116,8 +1116,8 @@ func TestDropPrefix(t *testing.T) { aRecord := "foo-a-test.example.com" expectedCnameRecord := "test.example.com" expectedARecord := "test.example.com" - actualCnameRecord := mapper.dropAffix(cnameRecord) - actualARecord := mapper.dropAffix(aRecord) + actualCnameRecord, _ := mapper.dropAffixExtractType(cnameRecord) + actualARecord, _ := mapper.dropAffixExtractType(aRecord) assert.Equal(t, expectedCnameRecord, actualCnameRecord) assert.Equal(t, expectedARecord, actualARecord) } @@ -1127,11 +1127,12 @@ func TestDropSuffix(t *testing.T) { aRecord := "test-a-foo.example.com" expectedARecord := "test.example.com" r := strings.SplitN(aRecord, ".", 2) - actualARecord := mapper.dropAffix(r[0]) + "." + r[1] + rClean, _ := mapper.dropAffixExtractType(r[0]) + actualARecord := rClean + "." + r[1] assert.Equal(t, expectedARecord, actualARecord) } -func TestExtractRecordType(t *testing.T) { +func TestExtractRecordTypeDefaultPosition(t *testing.T) { tests := []struct { input string expectedName string @@ -1160,13 +1161,93 @@ func TestExtractRecordType(t *testing.T) { } for _, tc := range tests { t.Run(tc.input, func(t *testing.T) { - actualName, actualType := extractRecordType(tc.input) + actualName, actualType := extractRecordTypeDefaultPosition(tc.input) assert.Equal(t, tc.expectedName, actualName) assert.Equal(t, tc.expectedType, actualType) }) } } +func TestToEndpointNameNewTXT(t *testing.T) { + type testCase struct { + name string + mapper affixNameMapper + domain string + recordType string + } + + testCases := []testCase{ + { + name: "prefix", + mapper: newaffixNameMapper("foo", "", ""), + domain: "example.com", + recordType: "A", + }, + { + name: "suffix", + mapper: newaffixNameMapper("", "foo", ""), + domain: "example.com", + recordType: "AAAA", + }, + { + name: "prefix with dash", + mapper: newaffixNameMapper("foo-", "", ""), + domain: "example.com", + recordType: "A", + }, + { + name: "suffix with dash", + mapper: newaffixNameMapper("", "-foo", ""), + domain: "example.com", + recordType: "CNAME", + }, + { + name: "prefix with dot", + mapper: newaffixNameMapper("foo.", "", ""), + domain: "example.com", + recordType: "CNAME", + }, + { + name: "suffix with dot", + mapper: newaffixNameMapper("", ".foo", ""), + domain: "example.com", + recordType: "CNAME", + }, + { + name: "templated prefix", + mapper: newaffixNameMapper("%{record_type}-foo", "", ""), + domain: "example.com", + recordType: "A", + }, + { + name: "templated suffix", + mapper: newaffixNameMapper("", "foo-%{record_type}", ""), + domain: "example.com", + recordType: "A", + }, + { + name: "templated prefix with dot", + mapper: newaffixNameMapper("%{record_type}foo.", "", ""), + domain: "example.com", + recordType: "CNAME", + }, + { + name: "templated suffix with dot", + mapper: newaffixNameMapper("", ".foo%{record_type}", ""), + domain: "example.com", + recordType: "A", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + txtName := tc.mapper.toNewTXTName(tc.domain, tc.recordType) + res, _ := tc.mapper.toEndpointName(txtName) + assert.Equal(t, tc.domain, res) + }) + } +} + func TestNewTXTScheme(t *testing.T) { p := inmemory.NewInMemoryProvider() p.CreateZone(testZone)