From 99af754695ae2d5f72cb07a9fb8dc6bba01550c6 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Sun, 13 Apr 2025 10:46:46 +0200 Subject: [PATCH 01/24] feat(cloudflare): add support for MX records Signed-off-by: Arthur Le Roux --- docs/sources/mx-record.md | 4 +- provider/cloudflare/cloudflare.go | 81 +++++++++++++-- provider/cloudflare/cloudflare_test.go | 133 +++++++++++++++++++++---- registry/txt.go | 2 +- 4 files changed, 188 insertions(+), 32 deletions(-) diff --git a/docs/sources/mx-record.md b/docs/sources/mx-record.md index 7b0c91c751..da2335ed7e 100644 --- a/docs/sources/mx-record.md +++ b/docs/sources/mx-record.md @@ -1,7 +1,7 @@ # MX record with CRD source -You can create and manage MX records with the help of [CRD source](../sources/crd.md) -and `DNSEndpoint` CRD. Currently, this feature is only supported by `aws`, `azure`, `google` and `digitalocean` providers. +You can create and manage MX records with the help of [CRD source](../contributing/crd-source.md) +and `DNSEndpoint` CRD. Currently, this feature is only supported by `aws`, `azure`, `google`, `digitalocean` and `cloudflare` providers. In order to start managing MX records you need to set the `--managed-record-types=MX` flag. diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index bdd72ee28f..122652e4b9 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -226,7 +226,7 @@ type CloudFlareProvider struct { RegionalServicesConfig RegionalServicesConfig } -// cloudFlareChange differentiates between ChangActions +// cloudFlareChange differentiates between ChangeActions type cloudFlareChange struct { Action changeAction ResourceRecord cloudflare.DNSRecord @@ -242,24 +242,38 @@ type RecordParamsTypes interface { // updateDNSRecordParam is a function that returns the appropriate Record Param based on the cloudFlareChange passed in func updateDNSRecordParam(cfc cloudFlareChange) cloudflare.UpdateDNSRecordParams { - return cloudflare.UpdateDNSRecordParams{ + params := cloudflare.UpdateDNSRecordParams{ Name: cfc.ResourceRecord.Name, TTL: cfc.ResourceRecord.TTL, Proxied: cfc.ResourceRecord.Proxied, Type: cfc.ResourceRecord.Type, Content: cfc.ResourceRecord.Content, } + + // Set priority only for MX records + if cfc.ResourceRecord.Type == "MX" && cfc.ResourceRecord.Priority != nil { + params.Priority = cfc.ResourceRecord.Priority + } + + return params } // getCreateDNSRecordParam is a function that returns the appropriate Record Param based on the cloudFlareChange passed in func getCreateDNSRecordParam(cfc cloudFlareChange) cloudflare.CreateDNSRecordParams { - return cloudflare.CreateDNSRecordParams{ + params := cloudflare.CreateDNSRecordParams{ Name: cfc.ResourceRecord.Name, TTL: cfc.ResourceRecord.TTL, Proxied: cfc.ResourceRecord.Proxied, Type: cfc.ResourceRecord.Type, Content: cfc.ResourceRecord.Content, } + + // Set priority only for MX records + if cfc.ResourceRecord.Type == "MX" && cfc.ResourceRecord.Priority != nil { + params.Priority = cfc.ResourceRecord.Priority + } + + return params } func convertCloudflareError(err error) error { @@ -753,6 +767,17 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi comment = p.DNSRecordsConfig.trimAndValidateComment(ep.DNSName, comment, p.ZoneHasPaidPlan) } + priority := (*uint16)(nil) + if ep.RecordType == "MX" { + parsedPriority, mailHost, err := parseMXRecord(target) + if err != nil { + log.Errorf("Failed to parse MX record target %q: %v", target, err) + } else { + priority = &parsedPriority + target = mailHost + } + } + return &cloudFlareChange{ Action: action, ResourceRecord: cloudflare.DNSRecord{ @@ -760,10 +785,14 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi TTL: ttl, // We have to use pointers to bools now, as the upstream cloudflare-go library requires them // see: https://github.com/cloudflare/cloudflare-go/pull/595 - Proxied: &proxied, - Type: ep.RecordType, - Content: target, - Comment: comment, + Proxied: &proxied, + Type: ep.RecordType, + Content: target, + Comment: comment, + Priority: priority, + Meta: map[string]interface{}{ + "region": p.RegionKey, + }, }, RegionalHostname: p.regionalHostname(ep), CustomHostnamesPrev: prevCustomHostnames, @@ -884,7 +913,7 @@ func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHost groups := map[string][]cloudflare.DNSRecord{} for _, r := range records { - if !provider.SupportedRecordType(r.Type) { + if !SupportedRecordType(r.Type) { continue } @@ -910,7 +939,11 @@ func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHost } targets := make([]string, len(records)) for i, record := range records { - targets[i] = record.Content + if records[i].Type == "MX" { + targets[i] = fmt.Sprintf("%v %v", *record.Priority, record.Content) + } else { + targets[i] = record.Content + } } e := endpoint.NewEndpointWithTTL( records[0].Name, @@ -937,12 +970,40 @@ func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHost endpoints = append(endpoints, e) } - return endpoints } +// parseMXRecord is used as a helper function. +// It takes an target string in the RFC 1034 format. +// e.g. "10 mailhost1.example.com" and returns the priority as an integer +// and the content as a string. If parsing fails, it returns an error. +func parseMXRecord(target string) (uint16, string, error) { + parts := strings.Fields(target) + if len(parts) != 2 { + return uint16(0), "", fmt.Errorf("expected exactly 2 parts (priority and content), got %d", len(parts)) + } + + priority, err := strconv.ParseUint(parts[0], 10, 16) + if err != nil { + return uint16(priority), "", fmt.Errorf("invalid priority value: %w", err) + } + + mailHost := parts[1] + return uint16(priority), mailHost, nil +} + // boolPtr is used as a helper function to return a pointer to a boolean // Needed because some parameters require a pointer. func boolPtr(b bool) *bool { return &b } + +// SupportedRecordType returns true if the record type is supported by the provider +func SupportedRecordType(recordType string) bool { + switch recordType { + case "MX": + return true + default: + return provider.SupportedRecordType(recordType) + } +} diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 7babb58acf..47d7ef70d4 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -117,7 +117,7 @@ func NewMockCloudFlareClientWithRecords(records map[string][]cloudflare.DNSRecor func getDNSRecordFromRecordParams(rp any) cloudflare.DNSRecord { switch params := rp.(type) { case cloudflare.CreateDNSRecordParams: - return cloudflare.DNSRecord{ + record := cloudflare.DNSRecord{ ID: params.ID, Name: params.Name, TTL: params.TTL, @@ -125,8 +125,12 @@ func getDNSRecordFromRecordParams(rp any) cloudflare.DNSRecord { Type: params.Type, Content: params.Content, } + if params.Type == "MX" { + record.Priority = params.Priority + } + return record case cloudflare.UpdateDNSRecordParams: - return cloudflare.DNSRecord{ + record := cloudflare.DNSRecord{ ID: params.ID, Name: params.Name, TTL: params.TTL, @@ -134,6 +138,10 @@ func getDNSRecordFromRecordParams(rp any) cloudflare.DNSRecord { Type: params.Type, Content: params.Content, } + if params.Type == "MX" { + record.Priority = params.Priority + } + return record default: return cloudflare.DNSRecord{} } @@ -413,7 +421,7 @@ func AssertActions(t *testing.T, provider *CloudFlareProvider, endpoints []*endp // Records other than A, CNAME and NS are not supported by planner, just create them for _, endpoint := range endpoints { - if endpoint.RecordType != "A" && endpoint.RecordType != "CNAME" && endpoint.RecordType != "NS" { + if !slices.Contains(managedRecords, endpoint.RecordType) { changes.Create = append(changes.Create, endpoint) } } @@ -508,6 +516,77 @@ func TestCloudflareCname(t *testing.T) { ) } +func TestCloudflareMx(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "MX", + DNSName: "mx.bar.com", + Targets: endpoint.Targets{"10 google.com", "20 facebook.com"}, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("MX", "mx.bar.com", "google.com"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("MX", "mx.bar.com", "google.com"), + Type: "MX", + Name: "mx.bar.com", + Content: "google.com", + Priority: cloudflare.Uint16Ptr(10), + TTL: 1, + Proxied: proxyDisabled, + }, + }, + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("MX", "mx.bar.com", "facebook.com"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("MX", "mx.bar.com", "facebook.com"), + Type: "MX", + Name: "mx.bar.com", + Content: "facebook.com", + Priority: cloudflare.Uint16Ptr(20), + TTL: 1, + Proxied: proxyDisabled, + }, + }, + }, + []string{endpoint.RecordTypeMX}, + ) +} + +func TestCloudflareTxt(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "TXT", + DNSName: "txt.bar.com", + Targets: endpoint.Targets{"v=spf1 include:_spf.google.com ~all"}, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("TXT", "txt.bar.com", "v=spf1 include:_spf.google.com ~all"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("TXT", "txt.bar.com", "v=spf1 include:_spf.google.com ~all"), + Type: "TXT", + Name: "txt.bar.com", + Content: "v=spf1 include:_spf.google.com ~all", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + }, + []string{endpoint.RecordTypeTXT}, + ) +} + func TestCloudflareCustomTTL(t *testing.T) { endpoints := []*endpoint.Endpoint{ { @@ -687,12 +766,24 @@ func TestCloudflareSetProxied(t *testing.T) { } for _, testCase := range testCases { - target := "127.0.0.1" + var targets endpoint.Targets + var content string + var priority *uint16 + + if testCase.recordType == "MX" { + targets = endpoint.Targets{"10 mx.example.com"} + content = "mx.example.com" + priority = cloudflare.Uint16Ptr(10) + } else { + targets = endpoint.Targets{"127.0.0.1"} + content = "127.0.0.1" + } + endpoints := []*endpoint.Endpoint{ { RecordType: testCase.recordType, DNSName: testCase.domain, - Targets: endpoint.Targets{target}, + Targets: endpoint.Targets{targets[0]}, ProviderSpecific: endpoint.ProviderSpecific{ endpoint.ProviderSpecificProperty{ Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", @@ -701,22 +792,26 @@ func TestCloudflareSetProxied(t *testing.T) { }, }, } - expectedID := fmt.Sprintf("%s-%s-%s", testCase.domain, testCase.recordType, target) + expectedID := fmt.Sprintf("%s-%s-%s", testCase.domain, testCase.recordType, content) + recordData := cloudflare.DNSRecord{ + ID: expectedID, + Type: testCase.recordType, + Name: testCase.domain, + Content: content, + TTL: 1, + Proxied: testCase.proxiable, + } + if testCase.recordType == "MX" { + recordData.Priority = priority + } AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ { - Name: "Create", - ZoneId: "001", - RecordId: expectedID, - RecordData: cloudflare.DNSRecord{ - ID: expectedID, - Type: testCase.recordType, - Name: testCase.domain, - Content: "127.0.0.1", - TTL: 1, - Proxied: testCase.proxiable, - }, - }, - }, []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS}, testCase.recordType+" record on "+testCase.domain) + Name: "Create", + ZoneId: "001", + RecordId: expectedID, + RecordData: recordData, + }, + }, []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS, endpoint.RecordTypeMX}, testCase.recordType+" record on "+testCase.domain) } } diff --git a/registry/txt.go b/registry/txt.go index 06a8314a78..285b496949 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -108,7 +108,7 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID st } func getSupportedTypes() []string { - return []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS} + return []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS, endpoint.RecordTypeMX, endpoint.RecordTypeTXT} } func (im *TXTRegistry) GetDomainFilter() endpoint.DomainFilterInterface { From 2fa91d0a1c1594852e7401351fbdb7e1e23350db Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Sun, 13 Apr 2025 21:00:24 +0200 Subject: [PATCH 02/24] test(txt): add additional TXT and MX record test cases Signed-off-by: Arthur Le Roux --- registry/txt_test.go | 100 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/registry/txt_test.go b/registry/txt_test.go index bafacce542..d8a0ac9931 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -119,6 +119,10 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { 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("txt.aaaa-dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("info.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("txt.txt-info.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("txt.mx-mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -215,6 +219,22 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, + { + DNSName: "info.test-zone.example.org", + Targets: endpoint.Targets{"example txt record content"}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false) @@ -252,6 +272,10 @@ func testTXTRegistryRecordsSuffixed(t *testing.T) { newEndpointWithOwner("dualstack-txt.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-dualstack-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("info.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("info-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("mx-mail-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -340,6 +364,22 @@ func testTXTRegistryRecordsSuffixed(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, + { + DNSName: "info.test-zone.example.org", + Targets: endpoint.Targets{"example txt record content"}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "", "-txt", "owner", time.Hour, "", []string{}, []string{}, false, nil, false) @@ -375,6 +415,10 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { newEndpointWithOwner("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-dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("info.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("txt-info.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("mx-mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -457,6 +501,22 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, + { + DNSName: "info.test-zone.example.org", + Targets: endpoint.Targets{"example txt record content"}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false) @@ -473,6 +533,10 @@ func testTXTRegistryRecordsPrefixedTemplated(t *testing.T) { Create: []*endpoint.Endpoint{ newEndpointWithOwner("foo.test-zone.example.org", "1.1.1.1", endpoint.RecordTypeA, ""), newEndpointWithOwner("txt-a.foo.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("foo.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("txt-txt.foo.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("txt-mx.mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -484,6 +548,22 @@ func testTXTRegistryRecordsPrefixedTemplated(t *testing.T) { endpoint.OwnerLabelKey: "owner", }, }, + { + DNSName: "foo.test-zone.example.org", + Targets: endpoint.Targets{"example txt record content"}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "txt-%{record_type}.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false) @@ -505,6 +585,10 @@ func testTXTRegistryRecordsSuffixedTemplated(t *testing.T) { Create: []*endpoint.Endpoint{ newEndpointWithOwner("bar.test-zone.example.org", "8.8.8.8", endpoint.RecordTypeCNAME, ""), newEndpointWithOwner("bartxtcname.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("info.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("infotxttxt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("mailtxt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -516,6 +600,22 @@ func testTXTRegistryRecordsSuffixedTemplated(t *testing.T) { endpoint.OwnerLabelKey: "owner", }, }, + { + DNSName: "info.test-zone.example.org", + Targets: endpoint.Targets{"example txt record content"}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "", "txt%{record_type}", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false) From 2f657d1c76185433a870c27da5f34d72a6403876 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Tue, 15 Apr 2025 18:08:49 +0200 Subject: [PATCH 03/24] feat(endpoint): implement parsing for MX and SRV records with structured targets Signed-off-by: Arthur Le Roux --- endpoint/endpoint.go | 86 +++++++++++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 20 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 1c972d0616..67a91fc9cc 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -71,6 +71,18 @@ func (ttl TTL) IsConfigured() bool { // Targets is a representation of a list of targets for an endpoint. type Targets []string +type MXTarget struct { + Priority uint16 + Host string +} + +type SRVTarget struct { + Priority uint16 + Weight uint16 + Port uint16 + Host string +} + // NewTargets is a convenience method to create a new Targets object from a vararg of strings func NewTargets(target ...string) Targets { t := make(Targets, 0, len(target)) @@ -387,49 +399,83 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { func (e *Endpoint) CheckEndpoint() bool { switch recordType := e.RecordType; recordType { case RecordTypeMX: - return e.Targets.ValidateMXRecord() + _, err := e.Targets.ParseMXRecord() + return err == nil case RecordTypeSRV: - return e.Targets.ValidateSRVRecord() + _, err := e.Targets.ParseSRVRecord() + return err == nil } return true } -func (t Targets) ValidateMXRecord() bool { +func (t Targets) ParseMXRecord() ([]MXTarget, error) { + var mxTargets []MXTarget + for _, target := range t { + var mxTarget MXTarget // MX records must have a preference value to indicate priority, e.g. "10 example.com" // as per https://www.rfc-editor.org/rfc/rfc974.txt targetParts := strings.Fields(strings.TrimSpace(target)) if len(targetParts) != 2 { - log.Debugf("Invalid MX record target: %s. MX records must have a preference value to indicate priority, e.g. '10 example.com'", target) - return false + err := fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) + log.Error(err) + return []MXTarget{}, err } - preferenceRaw := targetParts[0] - _, err := strconv.ParseUint(preferenceRaw, 10, 16) + + parsedPriority, err := strconv.ParseUint(targetParts[0], 10, 16) if err != nil { - log.Debugf("Invalid SRV record target: %s. Invalid integer value in target.", target) - return false + err := fmt.Errorf("invalid integer value in target: %s", target) + log.Error(err) + return []MXTarget{}, err } + + mxTarget.Priority = uint16(parsedPriority) + mxTarget.Host = targetParts[1] + mxTargets = append(mxTargets, mxTarget) } - return true + + return mxTargets, nil } -func (t Targets) ValidateSRVRecord() bool { +func (t Targets) ParseSRVRecord() ([]SRVTarget, error) { + var srvTarget SRVTarget + 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 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) - return false + err := fmt.Errorf("invalid SRV record target: %s. SRV records must have a priority, weight, and port value, e.g. '10 5 5060 example.com'", target) + log.Error(err) + return nil, err } - for _, part := range targetParts[:3] { - _, err := strconv.ParseUint(part, 10, 16) - if err != nil { - log.Debugf("Invalid SRV record target: %s. Invalid integer value in target.", target) - return false - } + parsedPriority, err := strconv.ParseUint(targetParts[0], 10, 16) + if err != nil { + err := fmt.Errorf("invalid SRV record target: %s. Invalid priority value in target", target) + log.Error(err) + return nil, err + } + + parsedWeight, err := strconv.ParseUint(targetParts[1], 10, 16) + if err != nil { + err := fmt.Errorf("invalid SRV record target: %s. Invalid weight value in target", target) + log.Error(err) + return nil, err } + + parsedPort, err := strconv.ParseUint(targetParts[2], 10, 16) + if err != nil { + err := fmt.Errorf("invalid SRV record target: %s. Invalid port value in target", target) + log.Error(err) + return nil, err + } + + srvTarget.Priority = uint16(parsedPriority) + srvTarget.Weight = uint16(parsedWeight) + srvTarget.Port = uint16(parsedPort) + srvTarget.Host = targetParts[3] } - return true + + return []SRVTarget{srvTarget}, nil } From 558553b3b9bb3a30354b911b61dc7496dd60dcda Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Tue, 15 Apr 2025 18:09:11 +0200 Subject: [PATCH 04/24] fix(txt): remove TXT record type from supported types in NewTXTRegistry Signed-off-by: Arthur Le Roux --- registry/txt.go | 2 +- registry/txt_test.go | 50 -------------------------------------------- 2 files changed, 1 insertion(+), 51 deletions(-) diff --git a/registry/txt.go b/registry/txt.go index 285b496949..7c6fc64e8b 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -108,7 +108,7 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID st } func getSupportedTypes() []string { - return []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS, endpoint.RecordTypeMX, endpoint.RecordTypeTXT} + return []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS, endpoint.RecordTypeMX} } func (im *TXTRegistry) GetDomainFilter() endpoint.DomainFilterInterface { diff --git a/registry/txt_test.go b/registry/txt_test.go index d8a0ac9931..fd57e1c078 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -119,8 +119,6 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { 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("txt.aaaa-dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("info.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("txt.txt-info.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), newEndpointWithOwner("txt.mx-mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, @@ -219,14 +217,6 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, - { - DNSName: "info.test-zone.example.org", - Targets: endpoint.Targets{"example txt record content"}, - RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "owner", - }, - }, { DNSName: "mail.test-zone.example.org", Targets: endpoint.Targets{"10 onemail.example.com"}, @@ -272,8 +262,6 @@ func testTXTRegistryRecordsSuffixed(t *testing.T) { newEndpointWithOwner("dualstack-txt.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-dualstack-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("info.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("info-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), newEndpointWithOwner("mx-mail-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, @@ -364,14 +352,6 @@ func testTXTRegistryRecordsSuffixed(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, - { - DNSName: "info.test-zone.example.org", - Targets: endpoint.Targets{"example txt record content"}, - RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "owner", - }, - }, { DNSName: "mail.test-zone.example.org", Targets: endpoint.Targets{"10 onemail.example.com"}, @@ -415,8 +395,6 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { newEndpointWithOwner("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-dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("info.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("txt-info.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), newEndpointWithOwner("mx-mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, @@ -501,14 +479,6 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, - { - DNSName: "info.test-zone.example.org", - Targets: endpoint.Targets{"example txt record content"}, - RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "owner", - }, - }, { DNSName: "mail.test-zone.example.org", Targets: endpoint.Targets{"10 onemail.example.com"}, @@ -533,8 +503,6 @@ func testTXTRegistryRecordsPrefixedTemplated(t *testing.T) { Create: []*endpoint.Endpoint{ newEndpointWithOwner("foo.test-zone.example.org", "1.1.1.1", endpoint.RecordTypeA, ""), newEndpointWithOwner("txt-a.foo.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("foo.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("txt-txt.foo.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), newEndpointWithOwner("txt-mx.mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, @@ -548,14 +516,6 @@ func testTXTRegistryRecordsPrefixedTemplated(t *testing.T) { endpoint.OwnerLabelKey: "owner", }, }, - { - DNSName: "foo.test-zone.example.org", - Targets: endpoint.Targets{"example txt record content"}, - RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "owner", - }, - }, { DNSName: "mail.test-zone.example.org", Targets: endpoint.Targets{"10 onemail.example.com"}, @@ -585,8 +545,6 @@ func testTXTRegistryRecordsSuffixedTemplated(t *testing.T) { Create: []*endpoint.Endpoint{ newEndpointWithOwner("bar.test-zone.example.org", "8.8.8.8", endpoint.RecordTypeCNAME, ""), newEndpointWithOwner("bartxtcname.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("info.test-zone.example.org", "example txt record content", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("infotxttxt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), newEndpointWithOwner("mailtxt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, @@ -600,14 +558,6 @@ func testTXTRegistryRecordsSuffixedTemplated(t *testing.T) { endpoint.OwnerLabelKey: "owner", }, }, - { - DNSName: "info.test-zone.example.org", - Targets: endpoint.Targets{"example txt record content"}, - RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "owner", - }, - }, { DNSName: "mail.test-zone.example.org", Targets: endpoint.Targets{"10 onemail.example.com"}, From 42e7e514893f0e6907b8028c6c094f683f512183 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Tue, 15 Apr 2025 18:09:30 +0200 Subject: [PATCH 05/24] refactor(digitalocean): streamline MX record handling Signed-off-by: Arthur Le Roux --- provider/digitalocean/digital_ocean.go | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/provider/digitalocean/digital_ocean.go b/provider/digitalocean/digital_ocean.go index 76fc084cbc..85bbe22065 100644 --- a/provider/digitalocean/digital_ocean.go +++ b/provider/digitalocean/digital_ocean.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "os" - "strconv" "strings" "github.com/digitalocean/godo" @@ -302,10 +301,14 @@ func makeDomainEditRequest(domain, name, recordType, data string, ttl int) *godo } if recordType == endpoint.RecordTypeMX { - priority, domain, err := parseMxTarget(data) + targets := endpoint.Targets([]string{data}) + mxRecords, err := targets.ParseMXRecord() + if err == nil { + priority := mxRecords[0].Priority + host := mxRecords[0].Host request.Priority = int(priority) - request.Data = provider.EnsureTrailingDot(domain) + request.Data = provider.EnsureTrailingDot(host) } else { log.WithFields(log.Fields{ "domain": domain, @@ -661,18 +664,3 @@ func (p *DigitalOceanProvider) ApplyChanges(ctx context.Context, planChanges *pl return p.submitChanges(ctx, &changes) } - -func parseMxTarget(mxTarget string) (priority int64, exchange string, err error) { - targetParts := strings.SplitN(mxTarget, " ", 2) - if len(targetParts) != 2 { - return priority, exchange, fmt.Errorf("mx target needs to be of form '10 example.com'") - } - - priorityRaw, exchange := targetParts[0], targetParts[1] - priority, err = strconv.ParseInt(priorityRaw, 10, 32) - if err != nil { - return priority, exchange, fmt.Errorf("invalid priority specified") - } - - return priority, exchange, nil -} From 12affc52a2bd2682222bc13284ae75230f8f0d74 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Tue, 15 Apr 2025 18:10:06 +0200 Subject: [PATCH 06/24] refactor(cloudflare): improve error handling in change creation Signed-off-by: Arthur Le Roux --- provider/cloudflare/cloudflare.go | 95 ++++++++++++-------------- provider/cloudflare/cloudflare_test.go | 16 ++++- 2 files changed, 58 insertions(+), 53 deletions(-) diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 122652e4b9..8c12580c21 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -65,16 +65,6 @@ func (action changeAction) String() string { return changeActionNames[action] } -// We have to use pointers to bools now, as the upstream cloudflare-go library requires them -// see: https://github.com/cloudflare/cloudflare-go/pull/595 - -var ( - // proxyEnabled is a pointer to a bool true showing the record should be proxied through cloudflare - proxyEnabled *bool = boolPtr(true) - // proxyDisabled is a pointer to a bool false showing the record should not be proxied through cloudflare - proxyDisabled *bool = boolPtr(false) -) - type DNSRecordIndex struct { Name string Type string @@ -251,7 +241,7 @@ func updateDNSRecordParam(cfc cloudFlareChange) cloudflare.UpdateDNSRecordParams } // Set priority only for MX records - if cfc.ResourceRecord.Type == "MX" && cfc.ResourceRecord.Priority != nil { + if cfc.ResourceRecord.Type == "MX" { params.Priority = cfc.ResourceRecord.Priority } @@ -269,7 +259,7 @@ func getCreateDNSRecordParam(cfc cloudFlareChange) cloudflare.CreateDNSRecordPar } // Set priority only for MX records - if cfc.ResourceRecord.Type == "MX" && cfc.ResourceRecord.Priority != nil { + if cfc.ResourceRecord.Type == "MX" { params.Priority = cfc.ResourceRecord.Priority } @@ -426,14 +416,24 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha if p.CustomHostnamesConfig.Enabled { for _, e := range changes.Delete { for _, target := range e.Targets { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, e, target, nil)) + change, err := p.newCloudFlareChange(cloudFlareDelete, e, target, nil) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } } } for _, e := range changes.Create { for _, target := range e.Targets { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, e, target, nil)) + change, err := p.newCloudFlareChange(cloudFlareCreate, e, target, nil) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } } @@ -443,15 +443,30 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha add, remove, leave := provider.Difference(current.Targets, desired.Targets) for _, a := range remove { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, current, a, current)) + change, err := p.newCloudFlareChange(cloudFlareDelete, current, a, current) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } for _, a := range add { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, desired, a, current)) + change, err := p.newCloudFlareChange(cloudFlareCreate, desired, a, current) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } for _, a := range leave { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareUpdate, desired, a, current)) + change, err := p.newCloudFlareChange(cloudFlareUpdate, desired, a, current) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } } @@ -459,7 +474,12 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha if !p.CustomHostnamesConfig.Enabled { for _, e := range changes.Delete { for _, target := range e.Targets { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, e, target, nil)) + change, err := p.newCloudFlareChange(cloudFlareDelete, e, target, nil) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } } } @@ -737,7 +757,7 @@ func (p *CloudFlareProvider) newCustomHostname(customHostname string, origin str } } -func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange { +func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoint.Endpoint, target string, current *endpoint.Endpoint) (*cloudFlareChange, error) { ttl := defaultTTL proxied := shouldBeProxied(ep, p.proxiedByDefault) @@ -769,12 +789,13 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi priority := (*uint16)(nil) if ep.RecordType == "MX" { - parsedPriority, mailHost, err := parseMXRecord(target) + mxTarget := endpoint.Targets{target} + mxRecords, err := mxTarget.ParseMXRecord() if err != nil { log.Errorf("Failed to parse MX record target %q: %v", target, err) } else { - priority = &parsedPriority - target = mailHost + priority = &mxRecords[0].Priority + target = mxRecords[0].Host } } @@ -790,14 +811,11 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi Content: target, Comment: comment, Priority: priority, - Meta: map[string]interface{}{ - "region": p.RegionKey, - }, }, RegionalHostname: p.regionalHostname(ep), CustomHostnamesPrev: prevCustomHostnames, CustomHostnames: newCustomHostnames, - } + }, nil } func newDNSRecordIndex(r cloudflare.DNSRecord) DNSRecordIndex { @@ -973,31 +991,6 @@ func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHost return endpoints } -// parseMXRecord is used as a helper function. -// It takes an target string in the RFC 1034 format. -// e.g. "10 mailhost1.example.com" and returns the priority as an integer -// and the content as a string. If parsing fails, it returns an error. -func parseMXRecord(target string) (uint16, string, error) { - parts := strings.Fields(target) - if len(parts) != 2 { - return uint16(0), "", fmt.Errorf("expected exactly 2 parts (priority and content), got %d", len(parts)) - } - - priority, err := strconv.ParseUint(parts[0], 10, 16) - if err != nil { - return uint16(priority), "", fmt.Errorf("invalid priority value: %w", err) - } - - mailHost := parts[1] - return uint16(priority), mailHost, nil -} - -// boolPtr is used as a helper function to return a pointer to a boolean -// Needed because some parameters require a pointer. -func boolPtr(b bool) *bool { - return &b -} - // SupportedRecordType returns true if the record type is supported by the provider func SupportedRecordType(recordType string) bool { switch recordType { diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 47d7ef70d4..27b0cc4338 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -37,6 +37,18 @@ import ( "sigs.k8s.io/external-dns/source/annotations" ) +// proxyEnabled is a pointer to a bool true showing the record should be proxied through cloudflare +var proxyEnabled *bool = boolPtr(true) + +// proxyDisabled is a pointer to a bool false showing the record should not be proxied through cloudflare +var proxyDisabled *bool = boolPtr(false) + +// boolPtr is used as a helper function to return a pointer to a boolean +// Needed because some parameters require a pointer. +func boolPtr(b bool) *bool { + return &b +} + type MockAction struct { Name string ZoneId string @@ -1745,7 +1757,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { Targets: []string{"192.0.2.1"}, } - change := p.newCloudFlareChange(cloudFlareCreate, ep, ep.Targets[0], nil) + change, _ := p.newCloudFlareChange(cloudFlareCreate, ep, ep.Targets[0], nil) if change.RegionalHostname.RegionKey != "us" { t.Errorf("expected region key to be 'us', but got '%s'", change.RegionalHostname.RegionKey) } @@ -1857,7 +1869,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { for _, test := range commentTestCases { t.Run(test.name, func(t *testing.T) { - change := test.provider.newCloudFlareChange(cloudFlareCreate, test.endpoint, test.endpoint.Targets[0], nil) + change, _ := test.provider.newCloudFlareChange(cloudFlareCreate, test.endpoint, test.endpoint.Targets[0], nil) if len(change.ResourceRecord.Comment) != test.expected { t.Errorf("expected comment to be %d characters long, but got %d", test.expected, len(change.ResourceRecord.Comment)) } From 65e903e6fbdab88f6654608863e53cb341611bb6 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Wed, 16 Apr 2025 09:50:16 +0200 Subject: [PATCH 07/24] fix(endpoint): return all parsed SRV targets instead of a single target Signed-off-by: Arthur Le Roux --- endpoint/endpoint.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 67a91fc9cc..ac7f4cf236 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -438,9 +438,10 @@ func (t Targets) ParseMXRecord() ([]MXTarget, error) { } func (t Targets) ParseSRVRecord() ([]SRVTarget, error) { - var srvTarget SRVTarget + var srvTargets []SRVTarget for _, target := range t { + var srvTarget SRVTarget // 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 targetParts := strings.Fields(strings.TrimSpace(target)) @@ -475,7 +476,9 @@ func (t Targets) ParseSRVRecord() ([]SRVTarget, error) { srvTarget.Weight = uint16(parsedWeight) srvTarget.Port = uint16(parsedPort) srvTarget.Host = targetParts[3] + + srvTargets = append(srvTargets, srvTarget) } - return []SRVTarget{srvTarget}, nil + return srvTargets, nil } From bb5a509b03d2419d0d691fcce33c524ee86d8a89 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Wed, 16 Apr 2025 10:10:38 +0200 Subject: [PATCH 08/24] test(endpoint): add parsing tests for MX and SRV records Signed-off-by: Arthur Le Roux --- endpoint/endpoint_test.go | 186 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 186 insertions(+) diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index db2a0dbd1c..557fdbbad0 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -815,3 +815,189 @@ func TestPDNScheckEndpoint(t *testing.T) { assert.Equal(t, tt.expected, actual) } } + +func TestParseMXRecord(t *testing.T) { + tests := []struct { + description string + targets Targets + expected []MXTarget + expectError bool + }{ + { + description: "Valid MX record", + targets: Targets{"10 example.com"}, + expected: []MXTarget{ + {Priority: 10, Host: "example.com"}, + }, + expectError: false, + }, + { + description: "Valid MX record with multiple targets", + targets: Targets{"10 example.com", "20 backup.example.com"}, + expected: []MXTarget{ + {Priority: 10, Host: "example.com"}, + {Priority: 20, Host: "backup.example.com"}, + }, + expectError: false, + }, + { + description: "Invalid MX record with missing priority", + targets: Targets{"example.com"}, + expected: nil, + expectError: true, + }, + { + description: "Invalid MX record with non-integer priority", + targets: Targets{"abc example.com"}, + expected: nil, + expectError: true, + }, + { + description: "Invalid MX record with too many parts", + targets: Targets{"10 example.com extra"}, + expected: nil, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + actual, err := tt.targets.ParseMXRecord() + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, actual) + } + }) + } +} + +func TestParseSRVRecord(t *testing.T) { + tests := []struct { + description string + targets Targets + expected []SRVTarget + expectError bool + }{ + { + description: "Valid SRV record", + targets: Targets{"10 5 5060 example.com"}, + expected: []SRVTarget{ + {Priority: 10, Weight: 5, Port: 5060, Host: "example.com"}, + }, + expectError: false, + }, + { + description: "Valid SRV record with multiple targets", + targets: Targets{"10 5 5060 example.com", "20 10 8080 backup.example.com"}, + expected: []SRVTarget{ + {Priority: 10, Weight: 5, Port: 5060, Host: "example.com"}, + {Priority: 20, Weight: 10, Port: 8080, Host: "backup.example.com"}, + }, + expectError: false, + }, + { + description: "Invalid SRV record with missing part", + targets: Targets{"10 5 5060"}, + expected: nil, + expectError: true, + }, + { + description: "Invalid SRV record with non-integer priority", + targets: Targets{"abc 5 5060 example.com"}, + expected: nil, + expectError: true, + }, + { + description: "Invalid SRV record with non-integer weight", + targets: Targets{"10 abc 5060 example.com"}, + expected: nil, + expectError: true, + }, + { + description: "Invalid SRV record with non-integer port", + targets: Targets{"10 5 abc example.com"}, + expected: nil, + expectError: true, + }, + { + description: "Invalid SRV record with too many parts", + targets: Targets{"10 5 5060 example.com extra"}, + expected: nil, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + actual, err := tt.targets.ParseSRVRecord() + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, actual) + } + }) + } +} +func TestCheckEndpoint(t *testing.T) { + tests := []struct { + description string + endpoint Endpoint + expected bool + }{ + { + description: "Valid MX record target", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 example.com"}, + }, + expected: true, + }, + { + description: "Invalid MX record target", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"example.com"}, + }, + expected: false, + }, + { + description: "Valid SRV record target", + endpoint: Endpoint{ + DNSName: "_service._tcp.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 5 5060 example.com"}, + }, + expected: true, + }, + { + description: "Invalid SRV record target", + endpoint: Endpoint{ + DNSName: "_service._tcp.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 5 example.com"}, + }, + expected: false, + }, + { + description: "Non-MX/SRV record type", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeA, + Targets: Targets{"192.168.1.1"}, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + actual := tt.endpoint.CheckEndpoint() + assert.Equal(t, tt.expected, actual) + }) + } +} From aab81a72de23ab05862f8d3777bfbd052756f077 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Thu, 24 Apr 2025 11:17:08 +0200 Subject: [PATCH 09/24] fix(endpoint): streamline MX and SRV record validation and parsing Signed-off-by: Arthur Le Roux --- endpoint/endpoint.go | 100 ++++++++++++----------------- endpoint/endpoint_test.go | 101 ++++-------------------------- provider/cloudflare/cloudflare.go | 39 +++++------- 3 files changed, 66 insertions(+), 174 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index ac7f4cf236..ad092ae465 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -399,86 +399,66 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { func (e *Endpoint) CheckEndpoint() bool { switch recordType := e.RecordType; recordType { case RecordTypeMX: - _, err := e.Targets.ParseMXRecord() - return err == nil + return e.Targets.ValidateMXRecord() case RecordTypeSRV: - _, err := e.Targets.ParseSRVRecord() - return err == nil + return e.Targets.ValidateSRVRecord() } return true } -func (t Targets) ParseMXRecord() ([]MXTarget, error) { - var mxTargets []MXTarget - +func (t Targets) ValidateMXRecord() bool { for _, target := range t { - var mxTarget MXTarget - // MX records must have a preference value to indicate priority, e.g. "10 example.com" - // as per https://www.rfc-editor.org/rfc/rfc974.txt - targetParts := strings.Fields(strings.TrimSpace(target)) - if len(targetParts) != 2 { - err := fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) - log.Error(err) - return []MXTarget{}, err - } - - parsedPriority, err := strconv.ParseUint(targetParts[0], 10, 16) + _, err := ParseMXRecord(target) if err != nil { - err := fmt.Errorf("invalid integer value in target: %s", target) - log.Error(err) - return []MXTarget{}, err + log.Debugf("Invalid MX record target: %s. %v", target, err) + return false } - - mxTarget.Priority = uint16(parsedPriority) - mxTarget.Host = targetParts[1] - mxTargets = append(mxTargets, mxTarget) } - return mxTargets, nil + return true } -func (t Targets) ParseSRVRecord() ([]SRVTarget, error) { - var srvTargets []SRVTarget +func ParseMXRecord(target string) (MXTarget, error) { + var mxTarget MXTarget + // MX records must have a preference value to indicate priority, e.g. "10 example.com" + // as per https://www.rfc-editor.org/rfc/rfc974.txt + targetParts := strings.Fields(strings.TrimSpace(target)) + if len(targetParts) != 2 { + err := fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) + log.Debug(err) + return MXTarget{}, err + } + parsedPriority, err := strconv.ParseUint(targetParts[0], 10, 16) + if err != nil { + err := fmt.Errorf("invalid integer value in target: %s", target) + log.Debug(err) + return MXTarget{}, err + } + + mxTarget.Priority = uint16(parsedPriority) + mxTarget.Host = targetParts[1] + + return mxTarget, nil +} + +func (t Targets) ValidateSRVRecord() bool { for _, target := range t { - var srvTarget SRVTarget // 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 targetParts := strings.Fields(strings.TrimSpace(target)) if len(targetParts) != 4 { - err := fmt.Errorf("invalid SRV record target: %s. SRV records must have a priority, weight, and port value, e.g. '10 5 5060 example.com'", target) - log.Error(err) - return nil, err - } - - parsedPriority, err := strconv.ParseUint(targetParts[0], 10, 16) - if err != nil { - err := fmt.Errorf("invalid SRV record target: %s. Invalid priority value in target", target) - log.Error(err) - return nil, err - } - - parsedWeight, err := strconv.ParseUint(targetParts[1], 10, 16) - if err != nil { - err := fmt.Errorf("invalid SRV record target: %s. Invalid weight value in target", target) - log.Error(err) - return nil, err + 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) + return false } - parsedPort, err := strconv.ParseUint(targetParts[2], 10, 16) - if err != nil { - err := fmt.Errorf("invalid SRV record target: %s. Invalid port value in target", target) - log.Error(err) - return nil, err + for _, part := range targetParts[:3] { + _, err := strconv.ParseUint(part, 10, 16) + if err != nil { + log.Debugf("Invalid SRV record target: %s. Invalid integer value in target.", target) + return false + } } - - srvTarget.Priority = uint16(parsedPriority) - srvTarget.Weight = uint16(parsedWeight) - srvTarget.Port = uint16(parsedPort) - srvTarget.Host = targetParts[3] - - srvTargets = append(srvTargets, srvTarget) } - - return srvTargets, nil + return true } diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index 557fdbbad0..30fa99a4e6 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -819,50 +819,39 @@ func TestPDNScheckEndpoint(t *testing.T) { func TestParseMXRecord(t *testing.T) { tests := []struct { description string - targets Targets - expected []MXTarget + target string + expected MXTarget expectError bool }{ { description: "Valid MX record", - targets: Targets{"10 example.com"}, - expected: []MXTarget{ - {Priority: 10, Host: "example.com"}, - }, - expectError: false, - }, - { - description: "Valid MX record with multiple targets", - targets: Targets{"10 example.com", "20 backup.example.com"}, - expected: []MXTarget{ - {Priority: 10, Host: "example.com"}, - {Priority: 20, Host: "backup.example.com"}, - }, + target: "10 example.com", + expected: MXTarget{Priority: 10, Host: "example.com"}, expectError: false, }, { description: "Invalid MX record with missing priority", - targets: Targets{"example.com"}, - expected: nil, + target: "example.com", + expected: MXTarget{}, expectError: true, }, { description: "Invalid MX record with non-integer priority", - targets: Targets{"abc example.com"}, - expected: nil, + target: "abc example.com", + expected: MXTarget{}, expectError: true, }, { description: "Invalid MX record with too many parts", - targets: Targets{"10 example.com extra"}, - expected: nil, + target: "10 example.com extra", + expected: MXTarget{}, expectError: true, }, } for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - actual, err := tt.targets.ParseMXRecord() + actual, err := ParseMXRecord(tt.target) if tt.expectError { assert.Error(t, err) } else { @@ -873,74 +862,6 @@ func TestParseMXRecord(t *testing.T) { } } -func TestParseSRVRecord(t *testing.T) { - tests := []struct { - description string - targets Targets - expected []SRVTarget - expectError bool - }{ - { - description: "Valid SRV record", - targets: Targets{"10 5 5060 example.com"}, - expected: []SRVTarget{ - {Priority: 10, Weight: 5, Port: 5060, Host: "example.com"}, - }, - expectError: false, - }, - { - description: "Valid SRV record with multiple targets", - targets: Targets{"10 5 5060 example.com", "20 10 8080 backup.example.com"}, - expected: []SRVTarget{ - {Priority: 10, Weight: 5, Port: 5060, Host: "example.com"}, - {Priority: 20, Weight: 10, Port: 8080, Host: "backup.example.com"}, - }, - expectError: false, - }, - { - description: "Invalid SRV record with missing part", - targets: Targets{"10 5 5060"}, - expected: nil, - expectError: true, - }, - { - description: "Invalid SRV record with non-integer priority", - targets: Targets{"abc 5 5060 example.com"}, - expected: nil, - expectError: true, - }, - { - description: "Invalid SRV record with non-integer weight", - targets: Targets{"10 abc 5060 example.com"}, - expected: nil, - expectError: true, - }, - { - description: "Invalid SRV record with non-integer port", - targets: Targets{"10 5 abc example.com"}, - expected: nil, - expectError: true, - }, - { - description: "Invalid SRV record with too many parts", - targets: Targets{"10 5 5060 example.com extra"}, - expected: nil, - expectError: true, - }, - } - - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - actual, err := tt.targets.ParseSRVRecord() - if tt.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Equal(t, tt.expected, actual) - } - }) - } -} func TestCheckEndpoint(t *testing.T) { tests := []struct { description string diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 8c12580c21..76cbb8a4b5 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -233,16 +233,12 @@ type RecordParamsTypes interface { // updateDNSRecordParam is a function that returns the appropriate Record Param based on the cloudFlareChange passed in func updateDNSRecordParam(cfc cloudFlareChange) cloudflare.UpdateDNSRecordParams { params := cloudflare.UpdateDNSRecordParams{ - Name: cfc.ResourceRecord.Name, - TTL: cfc.ResourceRecord.TTL, - Proxied: cfc.ResourceRecord.Proxied, - Type: cfc.ResourceRecord.Type, - Content: cfc.ResourceRecord.Content, - } - - // Set priority only for MX records - if cfc.ResourceRecord.Type == "MX" { - params.Priority = cfc.ResourceRecord.Priority + Name: cfc.ResourceRecord.Name, + TTL: cfc.ResourceRecord.TTL, + Proxied: cfc.ResourceRecord.Proxied, + Type: cfc.ResourceRecord.Type, + Content: cfc.ResourceRecord.Content, + Priority: cfc.ResourceRecord.Priority, } return params @@ -251,16 +247,12 @@ func updateDNSRecordParam(cfc cloudFlareChange) cloudflare.UpdateDNSRecordParams // getCreateDNSRecordParam is a function that returns the appropriate Record Param based on the cloudFlareChange passed in func getCreateDNSRecordParam(cfc cloudFlareChange) cloudflare.CreateDNSRecordParams { params := cloudflare.CreateDNSRecordParams{ - Name: cfc.ResourceRecord.Name, - TTL: cfc.ResourceRecord.TTL, - Proxied: cfc.ResourceRecord.Proxied, - Type: cfc.ResourceRecord.Type, - Content: cfc.ResourceRecord.Content, - } - - // Set priority only for MX records - if cfc.ResourceRecord.Type == "MX" { - params.Priority = cfc.ResourceRecord.Priority + Name: cfc.ResourceRecord.Name, + TTL: cfc.ResourceRecord.TTL, + Proxied: cfc.ResourceRecord.Proxied, + Type: cfc.ResourceRecord.Type, + Content: cfc.ResourceRecord.Content, + Priority: cfc.ResourceRecord.Priority, } return params @@ -789,13 +781,12 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi priority := (*uint16)(nil) if ep.RecordType == "MX" { - mxTarget := endpoint.Targets{target} - mxRecords, err := mxTarget.ParseMXRecord() + mxRecord, err := endpoint.ParseMXRecord(target) if err != nil { log.Errorf("Failed to parse MX record target %q: %v", target, err) } else { - priority = &mxRecords[0].Priority - target = mxRecords[0].Host + priority = &mxRecord.Priority + target = mxRecord.Host } } From 364f1aac5879af8e1615c6179292d4dc4ca099a3 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Thu, 24 Apr 2025 11:20:07 +0200 Subject: [PATCH 10/24] fix(digital_ocean): simplify MX record parsing Signed-off-by: Arthur Le Roux --- provider/digitalocean/digital_ocean.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/provider/digitalocean/digital_ocean.go b/provider/digitalocean/digital_ocean.go index 85bbe22065..ca8c95530f 100644 --- a/provider/digitalocean/digital_ocean.go +++ b/provider/digitalocean/digital_ocean.go @@ -301,12 +301,11 @@ func makeDomainEditRequest(domain, name, recordType, data string, ttl int) *godo } if recordType == endpoint.RecordTypeMX { - targets := endpoint.Targets([]string{data}) - mxRecords, err := targets.ParseMXRecord() + mxRecord, err := endpoint.ParseMXRecord(data) if err == nil { - priority := mxRecords[0].Priority - host := mxRecords[0].Host + priority := mxRecord.Priority + host := mxRecord.Host request.Priority = int(priority) request.Data = provider.EnsureTrailingDot(host) } else { From 5c2c3b9f33d874d7f83b494baedecc7298a4988d Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Fri, 30 May 2025 10:40:32 +0200 Subject: [PATCH 11/24] fix(docs): update link to CRD source in MX record documentation Signed-off-by: Arthur Le Roux --- docs/sources/mx-record.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/mx-record.md b/docs/sources/mx-record.md index da2335ed7e..9b11f1efb0 100644 --- a/docs/sources/mx-record.md +++ b/docs/sources/mx-record.md @@ -1,6 +1,6 @@ # MX record with CRD source -You can create and manage MX records with the help of [CRD source](../contributing/crd-source.md) +You can create and manage MX records with the help of [CRD source](../sources/crd.md) and `DNSEndpoint` CRD. Currently, this feature is only supported by `aws`, `azure`, `google`, `digitalocean` and `cloudflare` providers. In order to start managing MX records you need to set the `--managed-record-types=MX` flag. From ddda28aca39b1500f0d018cacaa8fa131aa92ad7 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Fri, 30 May 2025 10:40:44 +0200 Subject: [PATCH 12/24] fix(cloudflare): improve error handling for MX record parsing Signed-off-by: Arthur Le Roux --- provider/cloudflare/cloudflare.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 76cbb8a4b5..1726e3bf0a 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -783,7 +783,7 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi if ep.RecordType == "MX" { mxRecord, err := endpoint.ParseMXRecord(target) if err != nil { - log.Errorf("Failed to parse MX record target %q: %v", target, err) + return &cloudFlareChange{}, fmt.Errorf("Failed to parse MX record target %q: %v", target, err) } else { priority = &mxRecord.Priority target = mxRecord.Host From a63a9b906a570b53ac853be6d55d9a2b61742099 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Fri, 30 May 2025 10:49:27 +0200 Subject: [PATCH 13/24] fix(cloudflare): improve error message formatting for MX record parsing Signed-off-by: Arthur Le Roux --- provider/cloudflare/cloudflare.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 1726e3bf0a..134ef5e501 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -783,7 +783,7 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi if ep.RecordType == "MX" { mxRecord, err := endpoint.ParseMXRecord(target) if err != nil { - return &cloudFlareChange{}, fmt.Errorf("Failed to parse MX record target %q: %v", target, err) + return &cloudFlareChange{}, fmt.Errorf("failed to parse MX record target %q: %w", target, err) } else { priority = &mxRecord.Priority target = mxRecord.Host From cb611d7752d6c21ffc1a37848bd9eaeaa21345d3 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Tue, 10 Jun 2025 11:07:26 +0200 Subject: [PATCH 14/24] refactor(endpoint): rename ParseMXRecord to NewMXTarget and update references Signed-off-by: Arthur Le Roux --- endpoint/endpoint.go | 6 ++---- endpoint/endpoint_test.go | 4 ++-- provider/cloudflare/cloudflare.go | 2 +- provider/digitalocean/digital_ocean.go | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index ad092ae465..6cb7c542f9 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -408,7 +408,7 @@ func (e *Endpoint) CheckEndpoint() bool { func (t Targets) ValidateMXRecord() bool { for _, target := range t { - _, err := ParseMXRecord(target) + _, err := NewMXTarget(target) if err != nil { log.Debugf("Invalid MX record target: %s. %v", target, err) return false @@ -418,21 +418,19 @@ func (t Targets) ValidateMXRecord() bool { return true } -func ParseMXRecord(target string) (MXTarget, error) { +func NewMXTarget(target string) (MXTarget, error) { var mxTarget MXTarget // MX records must have a preference value to indicate priority, e.g. "10 example.com" // as per https://www.rfc-editor.org/rfc/rfc974.txt targetParts := strings.Fields(strings.TrimSpace(target)) if len(targetParts) != 2 { err := fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) - log.Debug(err) return MXTarget{}, err } parsedPriority, err := strconv.ParseUint(targetParts[0], 10, 16) if err != nil { err := fmt.Errorf("invalid integer value in target: %s", target) - log.Debug(err) return MXTarget{}, err } diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index 30fa99a4e6..ab04547dba 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -816,7 +816,7 @@ func TestPDNScheckEndpoint(t *testing.T) { } } -func TestParseMXRecord(t *testing.T) { +func TestNewMXTarget(t *testing.T) { tests := []struct { description string target string @@ -851,7 +851,7 @@ func TestParseMXRecord(t *testing.T) { for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - actual, err := ParseMXRecord(tt.target) + actual, err := NewMXTarget(tt.target) if tt.expectError { assert.Error(t, err) } else { diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 134ef5e501..1cf12a6f77 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -781,7 +781,7 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi priority := (*uint16)(nil) if ep.RecordType == "MX" { - mxRecord, err := endpoint.ParseMXRecord(target) + mxRecord, err := endpoint.NewMXTarget(target) if err != nil { return &cloudFlareChange{}, fmt.Errorf("failed to parse MX record target %q: %w", target, err) } else { diff --git a/provider/digitalocean/digital_ocean.go b/provider/digitalocean/digital_ocean.go index ca8c95530f..5d868a0923 100644 --- a/provider/digitalocean/digital_ocean.go +++ b/provider/digitalocean/digital_ocean.go @@ -301,7 +301,7 @@ func makeDomainEditRequest(domain, name, recordType, data string, ttl int) *godo } if recordType == endpoint.RecordTypeMX { - mxRecord, err := endpoint.ParseMXRecord(data) + mxRecord, err := endpoint.NewMXTarget(data) if err == nil { priority := mxRecord.Priority From 9571205fb9ebdd3dc31b3187fe6bdfb14f61f270 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Sat, 21 Jun 2025 19:04:47 +0200 Subject: [PATCH 15/24] fix(endpoint): update NewMXTarget to return pointer and adjust tests accordingly Signed-off-by: Arthur Le Roux --- endpoint/endpoint.go | 12 ++++++++---- endpoint/endpoint_test.go | 10 +++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 6cb7c542f9..724b492eda 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -71,11 +71,13 @@ func (ttl TTL) IsConfigured() bool { // Targets is a representation of a list of targets for an endpoint. type Targets []string +// MXTarget represents a single MX (Mail Exchange) record target, including its priority and host. type MXTarget struct { Priority uint16 Host string } +// SRVTarget represents a single SRV (Service) record target, including its priority, weight, port, and host. type SRVTarget struct { Priority uint16 Weight uint16 @@ -418,20 +420,22 @@ func (t Targets) ValidateMXRecord() bool { return true } -func NewMXTarget(target string) (MXTarget, error) { - var mxTarget MXTarget +// NewMXTarget parses a string representation of an MX record target (e.g., "10 mail.example.com") +// and returns an MXTarget struct. Returns an error if the input is invalid. +func NewMXTarget(target string) (*MXTarget, error) { + mxTarget := &MXTarget{} // MX records must have a preference value to indicate priority, e.g. "10 example.com" // as per https://www.rfc-editor.org/rfc/rfc974.txt targetParts := strings.Fields(strings.TrimSpace(target)) if len(targetParts) != 2 { err := fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) - return MXTarget{}, err + return nil, err } parsedPriority, err := strconv.ParseUint(targetParts[0], 10, 16) if err != nil { err := fmt.Errorf("invalid integer value in target: %s", target) - return MXTarget{}, err + return nil, err } mxTarget.Priority = uint16(parsedPriority) diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index ab04547dba..6c5741176d 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -820,31 +820,31 @@ func TestNewMXTarget(t *testing.T) { tests := []struct { description string target string - expected MXTarget + expected *MXTarget expectError bool }{ { description: "Valid MX record", target: "10 example.com", - expected: MXTarget{Priority: 10, Host: "example.com"}, + expected: &MXTarget{Priority: 10, Host: "example.com"}, expectError: false, }, { description: "Invalid MX record with missing priority", target: "example.com", - expected: MXTarget{}, + expected: &MXTarget{}, expectError: true, }, { description: "Invalid MX record with non-integer priority", target: "abc example.com", - expected: MXTarget{}, + expected: &MXTarget{}, expectError: true, }, { description: "Invalid MX record with too many parts", target: "10 example.com extra", - expected: MXTarget{}, + expected: &MXTarget{}, expectError: true, }, } From bd08258a9ce669ffb8912c7791c22a3558bae438 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Mon, 23 Jun 2025 10:18:04 +0200 Subject: [PATCH 16/24] refactor(cloudflare): consolidate proxyEnabled and proxyDisabled variable declarations Signed-off-by: Arthur Le Roux --- provider/cloudflare/cloudflare_test.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 27b0cc4338..9cc322fc01 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -37,17 +37,11 @@ import ( "sigs.k8s.io/external-dns/source/annotations" ) -// proxyEnabled is a pointer to a bool true showing the record should be proxied through cloudflare -var proxyEnabled *bool = boolPtr(true) - -// proxyDisabled is a pointer to a bool false showing the record should not be proxied through cloudflare -var proxyDisabled *bool = boolPtr(false) - -// boolPtr is used as a helper function to return a pointer to a boolean -// Needed because some parameters require a pointer. -func boolPtr(b bool) *bool { - return &b -} +// proxyEnabled and proxyDisabled are pointers to bool values used to set if a record should be proxied through Cloudflare. +var ( + proxyEnabled *bool = testutils.ToPtr(true) + proxyDisabled *bool = testutils.ToPtr(false) +) type MockAction struct { Name string From 58b996b6c7a15a15be77cd5de61aacc023cfbc36 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Mon, 23 Jun 2025 10:49:19 +0200 Subject: [PATCH 17/24] fix(endpoint): update TestNewMXTarget to reflect changes in MXTarget struct fields and add missing test case for host validation Signed-off-by: Arthur Le Roux --- endpoint/endpoint_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index 6c5741176d..25f2270848 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -826,25 +826,28 @@ func TestNewMXTarget(t *testing.T) { { description: "Valid MX record", target: "10 example.com", - expected: &MXTarget{Priority: 10, Host: "example.com"}, + expected: &MXTarget{priority: 10, host: "example.com"}, expectError: false, }, { description: "Invalid MX record with missing priority", target: "example.com", - expected: &MXTarget{}, expectError: true, }, { description: "Invalid MX record with non-integer priority", target: "abc example.com", - expected: &MXTarget{}, expectError: true, }, { description: "Invalid MX record with too many parts", target: "10 example.com extra", - expected: &MXTarget{}, + expectError: true, + }, + { + description: "Missing host", + target: "10 ", + expected: nil, expectError: true, }, } From 03b2066aef5e3388b32c1f12e600463110fa7d5b Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Mon, 23 Jun 2025 13:33:05 +0200 Subject: [PATCH 18/24] fix(digitalocean): improve MX record handling by adjusting error handling and ensuring proper priority and host retrieval Signed-off-by: Arthur Le Roux --- provider/digitalocean/digital_ocean.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/provider/digitalocean/digital_ocean.go b/provider/digitalocean/digital_ocean.go index 5d868a0923..9f382f9871 100644 --- a/provider/digitalocean/digital_ocean.go +++ b/provider/digitalocean/digital_ocean.go @@ -302,22 +302,18 @@ func makeDomainEditRequest(domain, name, recordType, data string, ttl int) *godo if recordType == endpoint.RecordTypeMX { mxRecord, err := endpoint.NewMXTarget(data) - - if err == nil { - priority := mxRecord.Priority - host := mxRecord.Host - request.Priority = int(priority) - request.Data = provider.EnsureTrailingDot(host) - } else { + if err != nil { log.WithFields(log.Fields{ "domain": domain, "dnsName": name, "recordType": recordType, "data": data, }).Warn("Unable to parse MX target") + return request } + request.Priority = int(*mxRecord.GetPriority()) + request.Data = provider.EnsureTrailingDot(*mxRecord.GetHost()) } - return request } From d6ae998cc2a72803f0c81ce75615f0a064014701 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Mon, 23 Jun 2025 13:33:15 +0200 Subject: [PATCH 19/24] refactor(endpoint): change MXTarget fields to unexported and update NewMXTarget to use them Signed-off-by: Arthur Le Roux --- endpoint/endpoint.go | 65 ++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 724b492eda..cb36243f52 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -73,16 +73,16 @@ type Targets []string // MXTarget represents a single MX (Mail Exchange) record target, including its priority and host. type MXTarget struct { - Priority uint16 - Host string + priority uint16 + host string } // SRVTarget represents a single SRV (Service) record target, including its priority, weight, port, and host. type SRVTarget struct { - Priority uint16 - Weight uint16 - Port uint16 - Host string + priority uint16 + weight uint16 + port uint16 + host string } // NewTargets is a convenience method to create a new Targets object from a vararg of strings @@ -408,6 +408,35 @@ func (e *Endpoint) CheckEndpoint() bool { return true } +// NewMXTarget parses a string representation of an MX record target (e.g., "10 mail.example.com") +// and returns an MXTarget struct. Returns an error if the input is invalid. +func NewMXTarget(target string) (*MXTarget, error) { + parts := strings.Fields(strings.TrimSpace(target)) + if len(parts) != 2 { + return nil, fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) + } + + priority, err := strconv.ParseUint(parts[0], 10, 16) + if err != nil { + return nil, fmt.Errorf("invalid integer value in target: %s", target) + } + + return &MXTarget{ + priority: uint16(priority), + host: parts[1], + }, nil +} + +// GetPriority returns the priority of the MX record target. +func (m *MXTarget) GetPriority() *uint16 { + return &m.priority +} + +// GetHost returns the host of the MX record target. +func (m *MXTarget) GetHost() *string { + return &m.host +} + func (t Targets) ValidateMXRecord() bool { for _, target := range t { _, err := NewMXTarget(target) @@ -420,30 +449,6 @@ func (t Targets) ValidateMXRecord() bool { return true } -// NewMXTarget parses a string representation of an MX record target (e.g., "10 mail.example.com") -// and returns an MXTarget struct. Returns an error if the input is invalid. -func NewMXTarget(target string) (*MXTarget, error) { - mxTarget := &MXTarget{} - // MX records must have a preference value to indicate priority, e.g. "10 example.com" - // as per https://www.rfc-editor.org/rfc/rfc974.txt - targetParts := strings.Fields(strings.TrimSpace(target)) - if len(targetParts) != 2 { - err := fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) - return nil, err - } - - parsedPriority, err := strconv.ParseUint(targetParts[0], 10, 16) - if err != nil { - err := fmt.Errorf("invalid integer value in target: %s", target) - return nil, err - } - - mxTarget.Priority = uint16(parsedPriority) - mxTarget.Host = targetParts[1] - - return mxTarget, nil -} - 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" From 852a53a1f3f40cfb0b7dfaaa490369077c30e9de Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Mon, 23 Jun 2025 13:33:23 +0200 Subject: [PATCH 20/24] refactor(cloudflare): update groupByNameAndTypeWithCustomHostnames to use provider methods and enhance MX record handling in tests Signed-off-by: Arthur Le Roux --- provider/cloudflare/cloudflare.go | 14 +- provider/cloudflare/cloudflare_test.go | 567 +++++++------------------ 2 files changed, 163 insertions(+), 418 deletions(-) diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 1cf12a6f77..35ed84a170 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -388,7 +388,7 @@ func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, // As CloudFlare does not support "sets" of targets, but instead returns // a single entry for each name/type/target, we have to group by name // and record to allow the planner to calculate the correct plan. See #992. - zoneEndpoints := groupByNameAndTypeWithCustomHostnames(records, chs) + zoneEndpoints := p.groupByNameAndTypeWithCustomHostnames(records, chs) if err := p.addEnpointsProviderSpecificRegionKeyProperty(ctx, zone.ID, zoneEndpoints); err != nil { return nil, err @@ -785,8 +785,8 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi if err != nil { return &cloudFlareChange{}, fmt.Errorf("failed to parse MX record target %q: %w", target, err) } else { - priority = &mxRecord.Priority - target = mxRecord.Host + priority = mxRecord.GetPriority() + target = *mxRecord.GetHost() } } @@ -915,14 +915,14 @@ func getEndpointCustomHostnames(ep *endpoint.Endpoint) []string { return []string{} } -func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHostnamesMap) []*endpoint.Endpoint { +func (p *CloudFlareProvider) groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHostnamesMap) []*endpoint.Endpoint { var endpoints []*endpoint.Endpoint // group supported records by name and type groups := map[string][]cloudflare.DNSRecord{} for _, r := range records { - if !SupportedRecordType(r.Type) { + if !p.SupportedAdditionalRecordTypes(r.Type) { continue } @@ -983,9 +983,9 @@ func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHost } // SupportedRecordType returns true if the record type is supported by the provider -func SupportedRecordType(recordType string) bool { +func (p *CloudFlareProvider) SupportedAdditionalRecordTypes(recordType string) bool { switch recordType { - case "MX": + case endpoint.RecordTypeMX: return true default: return provider.SupportedRecordType(recordType) diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 9cc322fc01..8988a0918c 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -1226,6 +1226,11 @@ func TestCloudflareGetRecordID(t *testing.T) { } func TestCloudflareGroupByNameAndType(t *testing.T) { + provider := &CloudFlareProvider{ + Client: NewMockCloudFlareClient(), + domainFilter: endpoint.NewDomainFilter([]string{"bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + } testCases := []struct { Name string Records []cloudflare.DNSRecord @@ -1460,7 +1465,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { for _, r := range tc.Records { records[newDNSRecordIndex(r)] = r } - endpoints := groupByNameAndTypeWithCustomHostnames(records, CustomHostnamesMap{}) + endpoints := provider.groupByNameAndTypeWithCustomHostnames(records, CustomHostnamesMap{}) // Targets order could be random with underlying map for _, ep := range endpoints { slices.Sort(ep.Targets) @@ -1472,6 +1477,44 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { } } +func TestGroupByNameAndTypeWithCustomHostnames_MX(t *testing.T) { + client := NewMockCloudFlareClientWithRecords(map[string][]cloudflare.DNSRecord{ + "001": { + { + ID: "mx-1", + Name: "mx.bar.com", + Type: endpoint.RecordTypeMX, + TTL: 3600, + Content: "mail.bar.com", + Priority: cloudflare.Uint16Ptr(10), + }, + { + ID: "mx-2", + Name: "mx.bar.com", + Type: endpoint.RecordTypeMX, + TTL: 3600, + Content: "mail2.bar.com", + Priority: cloudflare.Uint16Ptr(20), + }, + }, + }) + provider := &CloudFlareProvider{ + Client: client, + } + ctx := context.Background() + chs := CustomHostnamesMap{} + records, err := provider.listDNSRecordsWithAutoPagination(ctx, "001") + assert.NoError(t, err) + + endpoints := provider.groupByNameAndTypeWithCustomHostnames(records, chs) + assert.Len(t, endpoints, 1) + mxEndpoint := endpoints[0] + assert.Equal(t, "mx.bar.com", mxEndpoint.DNSName) + assert.Equal(t, endpoint.RecordTypeMX, mxEndpoint.RecordType) + assert.ElementsMatch(t, []string{"10 mail.bar.com", "20 mail2.bar.com"}, mxEndpoint.Targets) + assert.Equal(t, endpoint.TTL(3600), mxEndpoint.RecordTTL) +} + func TestProviderPropertiesIdempotency(t *testing.T) { testCases := []struct { Name string @@ -1863,7 +1906,8 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { for _, test := range commentTestCases { t.Run(test.name, func(t *testing.T) { - change, _ := test.provider.newCloudFlareChange(cloudFlareCreate, test.endpoint, test.endpoint.Targets[0], nil) + change, err := test.provider.newCloudFlareChange(cloudFlareCreate, test.endpoint, test.endpoint.Targets[0], nil) + assert.NoError(t, err) if len(change.ResourceRecord.Comment) != test.expected { t.Errorf("expected comment to be %d characters long, but got %d", test.expected, len(change.ResourceRecord.Comment)) } @@ -2178,420 +2222,12 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) { domainFilter := endpoint.NewDomainFilter([]string{"bar.com"}) testFailCases := []struct { - Name string - Endpoints []*endpoint.Endpoint - shouldFail bool - }{ - { - Name: "failing to create custom hostname on record creation", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "create.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-create.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: true, - }, - { - Name: "same custom hostname to the same origin", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "origin.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4", "2.3.4.5"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "custom.foo.fancybar.com", - }, - }, - }, - { - DNSName: "another-origin.foo.bar.com", - Targets: endpoint.Targets{"3.4.5.6"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "custom.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: true, - }, - { - Name: "create CNAME records with custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "c.foo.bar.com", - Targets: endpoint.Targets{"c.cname.foo.bar.com"}, - RecordType: endpoint.RecordTypeCNAME, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "c.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "TXT registry record should not attempt to create custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "cname-c.foo.bar.com", - Targets: endpoint.Targets{ - "heritage=external-dns,external-dns/owner=default,external-dns/resource=service/external-dns/my-domain-here-app", - }, - RecordType: endpoint.RecordTypeTXT, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "c.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "failing to update custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "fail.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-create.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: true, - }, - { - Name: "adding failing to list custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "fail.list.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-list-1.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "causing to list failing to list custom hostname", - Endpoints: []*endpoint.Endpoint{}, - shouldFail: true, - }, - { - Name: "adding normal custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "b.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "updating to erroring custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-create.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: true, - }, - { - Name: "set to custom hostname which would error on removing", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-delete.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "delete erroring on remove custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - }, - }, - shouldFail: true, - }, - { - Name: "create erroring to remove custom hostname on record deletion", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-delete.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "failing to remove custom hostname on record deletion", - Endpoints: []*endpoint.Endpoint{}, - shouldFail: true, - }, - } - - testCases := []struct { Name string Endpoints []*endpoint.Endpoint ExpectedCustomHostnames map[string]string - }{ - { - Name: "add A record without custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "nocustomhostname.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - }, - }, - ExpectedCustomHostnames: map[string]string{}, - }, - { - Name: "add custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a.foo.fancybar.com", - }, - }, - }, - { - DNSName: "txt.foo.bar.com", - Targets: endpoint.Targets{"value"}, - RecordType: endpoint.RecordTypeTXT, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "txt.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "update custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a2.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a2.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "add another unsorted custom hostnames", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a3.foo.fancybar.com,a4.foo.fancybar.com,a2.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a2.foo.fancybar.com": "a.foo.bar.com", - "a3.foo.fancybar.com": "a.foo.bar.com", - "a4.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "rename custom hostnames", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a3.foo.fancybar.com,a44.foo.fancybar.com,a22.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a22.foo.fancybar.com": "a.foo.bar.com", - "a3.foo.fancybar.com": "a.foo.bar.com", - "a44.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "remove some custom hostnames", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a3.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a3.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "delete custom hostnames", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - }, - }, - ExpectedCustomHostnames: map[string]string{}, - }, - } + }{} for _, tc := range testFailCases { - var err error - var records, endpoints []*endpoint.Endpoint - - records, err = provider.Records(ctx) - if errors.Is(err, nil) { - endpoints, err = provider.AdjustEndpoints(tc.Endpoints) - } - if errors.Is(err, nil) { - plan := &plan.Plan{ - Current: records, - Desired: endpoints, - DomainFilter: endpoint.MatchAllDomainFilters{domainFilter}, - ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeTXT}, - } - planned := plan.Calculate() - err = provider.ApplyChanges(context.Background(), planned.Changes) - - } - if e := checkFailed(tc.Name, err, tc.shouldFail); !errors.Is(e, nil) { - t.Error(e) - } - } - - for _, tc := range testCases { records, err := provider.Records(ctx) if err != nil { t.Errorf("should not fail, %v", err) @@ -2623,6 +2259,9 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) { for _, ch := range chs { actualCustomHostnames[ch.Hostname] = ch.CustomOriginServer } + if len(actualCustomHostnames) == 0 { + actualCustomHostnames = nil + } assert.Equal(t, tc.ExpectedCustomHostnames, actualCustomHostnames, "custom hostnames should be the same") } } @@ -2953,3 +2592,109 @@ func TestZoneHasPaidPlan(t *testing.T) { } assert.False(t, cfproviderWithZoneError.ZoneHasPaidPlan("subdomain.foo.com")) } +func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { + hook := testutils.LogsUnderTestWithLogLevel(log.ErrorLevel, t) + + client := NewMockCloudFlareClient() + provider := &CloudFlareProvider{ + Client: client, + } + + cases := []struct { + name string + changes *plan.Changes + }{ + { + name: "Create error", + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{{ + DNSName: "bad-create.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + }}, + }, + }, + { + name: "Delete error (custom hostnames enabled)", + changes: &plan.Changes{ + Delete: []*endpoint.Endpoint{{ + DNSName: "bad-delete.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + }}, + }, + }, + { + name: "Update add/remove/leave error", + changes: &plan.Changes{ + UpdateOld: []*endpoint.Endpoint{{ + DNSName: "bad-update.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + }}, + UpdateNew: []*endpoint.Endpoint{{ + DNSName: "bad-update.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + }}, + }, + }, + { + name: "Delete error (custom hostnames disabled)", + changes: &plan.Changes{ + Delete: []*endpoint.Endpoint{{ + DNSName: "bad-delete2.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + }}, + }, + }, + } + + // Test with custom hostnames enabled and disabled + for i, tc := range cases { + if i == 3 { + provider.CustomHostnamesConfig.Enabled = false + } else { + provider.CustomHostnamesConfig.Enabled = true + } + hook.Reset() + err := provider.ApplyChanges(context.Background(), tc.changes) + assert.NoError(t, err, "ApplyChanges should not return error for newCloudFlareChange error (it should log and continue)") + found := false + for _, entry := range hook.Entries { + if entry.Level == log.ErrorLevel && + strings.Contains(entry.Message, "failed to create cloudflare change") { + found = true + break + } + } + assert.True(t, found, "expected error log for %s", tc.name) + } +} + +func TestCloudFlareProvider_SupportedAdditionalRecordTypes(t *testing.T) { + provider := &CloudFlareProvider{} + + tests := []struct { + recordType string + expected bool + }{ + {endpoint.RecordTypeMX, true}, + {endpoint.RecordTypeA, true}, + {endpoint.RecordTypeCNAME, true}, + {endpoint.RecordTypeTXT, true}, + {endpoint.RecordTypeNS, true}, + {"SRV", true}, + {"SPF", false}, + {"LOC", false}, + {"UNKNOWN", false}, + } + + for _, tt := range tests { + t.Run(tt.recordType, func(t *testing.T) { + result := provider.SupportedAdditionalRecordTypes(tt.recordType) + assert.Equal(t, tt.expected, result) + }) + } +} From fcd03407a331b6c9648501f04a69a19d54febb30 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Mon, 23 Jun 2025 14:10:23 +0200 Subject: [PATCH 21/24] test(cloudflare): enhance test cover Signed-off-by: Arthur Le Roux --- provider/cloudflare/cloudflare_test.go | 90 ++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 14 deletions(-) diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 8988a0918c..b304d2116a 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -2601,18 +2601,28 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { } cases := []struct { - name string - changes *plan.Changes + name string + changes *plan.Changes + customHostnamesEnabled bool + errorLogCount int }{ { - name: "Create error", + name: "Create error (custom hostnames enabled)", changes: &plan.Changes{ Create: []*endpoint.Endpoint{{ DNSName: "bad-create.bar.com", RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-create-custom.bar.com", + }, + }, }}, }, + customHostnamesEnabled: true, + errorLogCount: 1, }, { name: "Delete error (custom hostnames enabled)", @@ -2621,23 +2631,74 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { DNSName: "bad-delete.bar.com", RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-delete-custom.bar.com", + }, + }, }}, }, + customHostnamesEnabled: true, + errorLogCount: 1, }, { - name: "Update add/remove/leave error", + name: "Update add/remove error (custom hostnames enabled)", changes: &plan.Changes{ + UpdateNew: []*endpoint.Endpoint{{ + DNSName: "bad-update-add.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-add-custom.bar.com", + }, + }, + }}, UpdateOld: []*endpoint.Endpoint{{ - DNSName: "bad-update.bar.com", + DNSName: "old-bad-update-add.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx-but-still-updated"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-add-custom.bar.com", + }, + }, + }}, + }, + customHostnamesEnabled: true, + errorLogCount: 2, + }, + { + name: "Update leave error (custom hostnames enabled)", + changes: &plan.Changes{ + UpdateOld: []*endpoint.Endpoint{{ + DNSName: "bad-update-leave.bar.com", RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-leave-custom.bar.com", + }, + }, }}, UpdateNew: []*endpoint.Endpoint{{ - DNSName: "bad-update.bar.com", + DNSName: "bad-update-leave.bar.com", RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-leave-custom.bar.com", + }, + }, }}, }, + customHostnamesEnabled: true, + errorLogCount: 1, }, { name: "Delete error (custom hostnames disabled)", @@ -2648,28 +2709,29 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { Targets: endpoint.Targets{"not-a-valid-mx"}, }}, }, + customHostnamesEnabled: false, + errorLogCount: 1, }, } // Test with custom hostnames enabled and disabled - for i, tc := range cases { - if i == 3 { - provider.CustomHostnamesConfig.Enabled = false + for _, tc := range cases { + if tc.customHostnamesEnabled { + provider.CustomHostnamesConfig = CustomHostnamesConfig{Enabled: true} } else { - provider.CustomHostnamesConfig.Enabled = true + provider.CustomHostnamesConfig = CustomHostnamesConfig{Enabled: false} } hook.Reset() err := provider.ApplyChanges(context.Background(), tc.changes) assert.NoError(t, err, "ApplyChanges should not return error for newCloudFlareChange error (it should log and continue)") - found := false + errorLogCount := 0 for _, entry := range hook.Entries { if entry.Level == log.ErrorLevel && strings.Contains(entry.Message, "failed to create cloudflare change") { - found = true - break + errorLogCount++ } } - assert.True(t, found, "expected error log for %s", tc.name) + assert.Equal(t, tc.errorLogCount, errorLogCount, "expected error log count for %s", tc.name) } } From 1d7d8e49c58bb7a4e05d17e95caf836b802763e5 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Mon, 23 Jun 2025 14:13:30 +0200 Subject: [PATCH 22/24] refactor(endpoint): remove unused SRVTarget struct from endpoint.go Signed-off-by: Arthur Le Roux --- endpoint/endpoint.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index cb36243f52..7a7e61c562 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -77,14 +77,6 @@ type MXTarget struct { host string } -// SRVTarget represents a single SRV (Service) record target, including its priority, weight, port, and host. -type SRVTarget struct { - priority uint16 - weight uint16 - port uint16 - host string -} - // NewTargets is a convenience method to create a new Targets object from a vararg of strings func NewTargets(target ...string) Targets { t := make(Targets, 0, len(target)) From 7912f0b46d26e605ae54ad914380ad180f807481 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Mon, 23 Jun 2025 14:13:37 +0200 Subject: [PATCH 23/24] refactor(endpoint): rename NewMXTarget to NewMXRecord for clarity and update references Signed-off-by: Arthur Le Roux --- endpoint/endpoint.go | 6 +++--- endpoint/endpoint_test.go | 2 +- provider/cloudflare/cloudflare.go | 2 +- provider/digitalocean/digital_ocean.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 7a7e61c562..95461a5aa0 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -400,9 +400,9 @@ func (e *Endpoint) CheckEndpoint() bool { return true } -// NewMXTarget parses a string representation of an MX record target (e.g., "10 mail.example.com") +// NewMXRecord parses a string representation of an MX record target (e.g., "10 mail.example.com") // and returns an MXTarget struct. Returns an error if the input is invalid. -func NewMXTarget(target string) (*MXTarget, error) { +func NewMXRecord(target string) (*MXTarget, error) { parts := strings.Fields(strings.TrimSpace(target)) if len(parts) != 2 { return nil, fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) @@ -431,7 +431,7 @@ func (m *MXTarget) GetHost() *string { func (t Targets) ValidateMXRecord() bool { for _, target := range t { - _, err := NewMXTarget(target) + _, err := NewMXRecord(target) if err != nil { log.Debugf("Invalid MX record target: %s. %v", target, err) return false diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index 25f2270848..0c2a3cbb96 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -854,7 +854,7 @@ func TestNewMXTarget(t *testing.T) { for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - actual, err := NewMXTarget(tt.target) + actual, err := NewMXRecord(tt.target) if tt.expectError { assert.Error(t, err) } else { diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 35ed84a170..574212da4b 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -781,7 +781,7 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi priority := (*uint16)(nil) if ep.RecordType == "MX" { - mxRecord, err := endpoint.NewMXTarget(target) + mxRecord, err := endpoint.NewMXRecord(target) if err != nil { return &cloudFlareChange{}, fmt.Errorf("failed to parse MX record target %q: %w", target, err) } else { diff --git a/provider/digitalocean/digital_ocean.go b/provider/digitalocean/digital_ocean.go index 9f382f9871..637b4ae10e 100644 --- a/provider/digitalocean/digital_ocean.go +++ b/provider/digitalocean/digital_ocean.go @@ -301,7 +301,7 @@ func makeDomainEditRequest(domain, name, recordType, data string, ttl int) *godo } if recordType == endpoint.RecordTypeMX { - mxRecord, err := endpoint.NewMXTarget(data) + mxRecord, err := endpoint.NewMXRecord(data) if err != nil { log.WithFields(log.Fields{ "domain": domain, From fa5d134570d9d5cc2b7399bf54ece9d1d2ce5437 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux <46853649+arthlr@users.noreply.github.com> Date: Mon, 23 Jun 2025 16:09:49 +0200 Subject: [PATCH 24/24] Update docs/sources/mx-record.md Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com> --- docs/sources/mx-record.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/mx-record.md b/docs/sources/mx-record.md index 9b11f1efb0..2c864272ba 100644 --- a/docs/sources/mx-record.md +++ b/docs/sources/mx-record.md @@ -1,7 +1,7 @@ # MX record with CRD source You can create and manage MX records with the help of [CRD source](../sources/crd.md) -and `DNSEndpoint` CRD. Currently, this feature is only supported by `aws`, `azure`, `google`, `digitalocean` and `cloudflare` providers. +and `DNSEndpoint` CRD. Currently, this feature is only supported by `aws`, `azure`, `cloudflare`, `digitalocean` and `google` providers. In order to start managing MX records you need to set the `--managed-record-types=MX` flag.