fix(cloudflare): handle 81058 identical record error for region migration

[AndrewCharlesHay]

What does it do ?

Solves #6091

Motivation

More

• Yes, this PR title follows Conventional Commits
• Yes, I added unit tests
• Yes, I updated end user documentation accordingly

[k8s-ci-robot]

[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.

Details

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

[coveralls]

Pull Request Test Coverage Report for Build 20928128947

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 570 unchanged lines in 26 files lost coverage.
• Overall coverage increased (+0.08%) to 78.982%

---

Files with Coverage Reduction	New Missed Lines	%
apis/externaldns/types.go	1	99.62%
utils.go	1	96.0%
pod.go	2	95.54%
v1alpha1/groupversion_info.go	3	0.0%
informers/informers.go	4	81.82%
openshift_route.go	7	84.78%
node.go	9	90.48%
f5_virtualserver.go	9	79.21%
skipper_routegroup.go	9	38.84%
f5_transportserver.go	9	77.42%

Totals
Change from base Build 20810601084:	0.08%
Covered Lines:	16027
Relevant Lines:	20292

---

💛 - Coveralls

[mloiseleur]

To me, there is an issue here.
ExternalDNS should never decide to delete a record created externally (by the user or other app).
This is not what most users will expect and may lead to severe production or security issue.

[AndrewCharlesHay]

@mloiseleur Thank you for the feedback.
I have updated the PR to put this behavior behind an explicit opt-in flag: --cloudflare-region-key-conflict-resolution.

• Default behavior: Log the conflict error (81058) and fail gracefully (no deletion), advising the user to enable the flag if desired.
• With Flag: Perform the lookup-and-delete resolution strategy to unblock the region-key record creation.

This ensures operators consciously opt-in to the potential destructiveness of replacing global records with regional ones.

[AndrewCharlesHay]

@mloiseleur Does that sound good?

[ivankatliarchuk]

This PR introduces "lost ownership" problem. The external dns has a mechanism to identify the ownership. If record is not owned by the "external-dns" or there is a problem with the record - external-dns is not responsible for fixing it.

In this case, most likely, if there is an issue - log the problem, throw soft error and move on.

[ivankatliarchuk]

I found this comment #5459 (comment). Could be relevant, that describes overl external-dns behaviour in similar cases.

[ivankatliarchuk]

Probably worth to add to the docs somewhere, so it's super clear.

If we think, it is worth to challange status quo - most likely worth to start a discussion in the slack or short proposal https://github.com/kubernetes-sigs/external-dns/tree/master/docs/proposal

[AndrewCharlesHay]

I think we have something misconfigured on our system. External DNS is updating the region in place now. I'm going to close this PR

[k8s-ci-robot]

PR needs rebase.

Details

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.