feat(cloudflare): add support for MX records #5283

arthlr · 2025-04-13T09:06:14Z

Hi there! 👋

Description

This PR introduces support for MX records for the Cloudflare provider.

Cloudflare can now manage MX records via the DNSEndpoint CRD.
Adds logic to parse MX records and extract priority and mail host
Extends unit tests to validate proper MX record creation and priority
Updates the documentation to reflect support for cloudflare in MX record management.

Fixes #5282

Checklist

Unit tests updated
End user documentation updated (docs/sources/mx-record.md)

Thanks for checking this out! 🙏
Looking forward to your feedback — happy to make any changes if needed.

k8s-ci-robot · 2025-04-13T09:06:20Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign szuecs for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2025-04-13T09:06:23Z

Hi @arthlr. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

mloiseleur · 2025-04-13T14:55:24Z

/ok-to-test
@AndrewCharlesHay @mrozentsvayg do you think you can review this PR ?

registry/txt.go

arthlr · 2025-04-13T19:22:47Z

/retest

provider/cloudflare/cloudflare.go

mrozentsvayg · 2025-04-14T16:18:40Z

provider/cloudflare/cloudflare.go

+
+	priority := (*uint16)(nil)
+	if ep.RecordType == "MX" {
+		parsedPriority, mailHost, err := parseMXRecord(target)


There is already similar logic in endpoint.ValidateMXRecord()
I would suggest to deduplicate it.

2 more specific provider implementations of the parsing standard record:
https://github.com/kubernetes-sigs/external-dns/blob/master/provider/azure/common.go#L31-L47
https://github.com/kubernetes-sigs/external-dns/blob/master/provider/digitalocean/digital_ocean.go#L665-L678

Ok I will deduplicate then 👍

I have deduplicated the logic of endpoint.ValidateMXRecord() by changing a bit the logic and the return values. However, I have added tests to increase the cover of this part.

provider/cloudflare/cloudflare.go

mrozentsvayg · 2025-04-14T16:29:28Z

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}


Not sure about the idea to use txt registry for the txt records.
If it's not an integral part of the PR, i would postpone it until we better understand the txt registry roadmap, especially the part of deprecating old records.

I'll remove it then 👍
However, we'll need it one day (I would like to use MX and TXT records to automatically deploy sengrid-like DNS records)

using the TXT registry, specifically?

Yes, I am not using AWS and still don't want a noop registry.
Then, a TXT registry, to store the metadata of TXT records created by external-dns is must-have for my team and I.

endpoint/endpoint.go

provider/cloudflare/cloudflare.go

mrozentsvayg · 2025-04-16T18:45:58Z

provider/cloudflare/cloudflare.go

@@ -801,7 +839,7 @@ func (p *CloudFlareProvider) newCustomHostname(customHostname string, origin str
 	}
 }

-func (p *CloudFlareProvider) newCloudFlareChange(action string, ep *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange {
+func (p *CloudFlareProvider) newCloudFlareChange(action string, ep *endpoint.Endpoint, target string, current *endpoint.Endpoint) (*cloudFlareChange, error) {


why do we need to add error to the return values?
it's always nil, and this method is not intended to ever fail

I think we should add an error, because the behavior of p.newCloudFlareChange changes. Since we're now parsing the MX record, there's a possibility that the parsing might fail. In such cases, we need to explicitly return that the change won't occur because the parsing didn't succeed.

mloiseleur · 2025-05-28T20:34:24Z

@mrozentsvayg anything left on your side ?
@AndrewCharlesHay any comments on this PR ?

provider/cloudflare/cloudflare.go

Signed-off-by: Arthur Le Roux <[email protected]>

…red targets Signed-off-by: Arthur Le Roux <[email protected]>

Signed-off-by: Arthur Le Roux <[email protected]>

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 13, 2025

k8s-ci-robot requested review from ivankatliarchuk and mloiseleur April 13, 2025 09:06

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 13, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 13, 2025

mloiseleur reviewed Apr 13, 2025

View reviewed changes

registry/txt.go Outdated Show resolved Hide resolved

arthlr force-pushed the feat/mx-record branch 2 times, most recently from 14185d5 to 4f51703 Compare April 13, 2025 19:08

mrozentsvayg reviewed Apr 14, 2025

View reviewed changes

arthlr force-pushed the feat/mx-record branch from c071624 to 2cc854b Compare April 15, 2025 17:02

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 16, 2025

mrozentsvayg reviewed Apr 16, 2025

View reviewed changes

endpoint/endpoint.go Outdated Show resolved Hide resolved

mrozentsvayg reviewed Apr 16, 2025

View reviewed changes

endpoint/endpoint.go Outdated Show resolved Hide resolved

mrozentsvayg reviewed Apr 16, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 26, 2025

arthlr requested a review from mrozentsvayg April 30, 2025 12:01

AndrewCharlesHay reviewed May 28, 2025

View reviewed changes

provider/cloudflare/cloudflare.go Outdated Show resolved Hide resolved

arthlr added 5 commits May 30, 2025 10:25

feat(cloudflare): add support for MX records

d5c3133

Signed-off-by: Arthur Le Roux <[email protected]>

test(txt): add additional TXT and MX record test cases

96f615f

Signed-off-by: Arthur Le Roux <[email protected]>

feat(endpoint): implement parsing for MX and SRV records with structu…

263cb11

…red targets Signed-off-by: Arthur Le Roux <[email protected]>

fix(txt): remove TXT record type from supported types in NewTXTRegistry

937d583

Signed-off-by: Arthur Le Roux <[email protected]>

refactor(digitalocean): streamline MX record handling

3d92e77

Signed-off-by: Arthur Le Roux <[email protected]>

arthlr added 7 commits May 30, 2025 10:30

refactor(cloudflare): improve error handling in change creation

c3e158a

Signed-off-by: Arthur Le Roux <[email protected]>

fix(endpoint): return all parsed SRV targets instead of a single target

2e27e67

Signed-off-by: Arthur Le Roux <[email protected]>

test(endpoint): add parsing tests for MX and SRV records

fe0b31d

Signed-off-by: Arthur Le Roux <[email protected]>

fix(endpoint): streamline MX and SRV record validation and parsing

362f601

Signed-off-by: Arthur Le Roux <[email protected]>

fix(digital_ocean): simplify MX record parsing

6300560

Signed-off-by: Arthur Le Roux <[email protected]>

fix(docs): update link to CRD source in MX record documentation

f3a3ac2

Signed-off-by: Arthur Le Roux <[email protected]>

fix(cloudflare): improve error handling for MX record parsing

a32f388

Signed-off-by: Arthur Le Roux <[email protected]>

arthlr force-pushed the feat/mx-record branch from 519c016 to a32f388 Compare May 30, 2025 08:40

k8s-ci-robot added docs provider and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 30, 2025

fix(cloudflare): improve error message formatting for MX record parsing

ad8ee7c

Signed-off-by: Arthur Le Roux <[email protected]>

feat(cloudflare): add support for MX records #5283

Are you sure you want to change the base?

feat(cloudflare): add support for MX records #5283

Uh oh!

Conversation

arthlr commented Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Apr 13, 2025

Uh oh!

k8s-ci-robot commented Apr 13, 2025

Uh oh!

mloiseleur commented Apr 13, 2025

Uh oh!

Uh oh!

arthlr commented Apr 13, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mloiseleur commented May 28, 2025

Uh oh!

Uh oh!

Uh oh!

arthlr commented Apr 13, 2025 •

edited

Loading