feat(cloudflare): Suppport DNS record comments

[7onn]

Description
This will add a flags to be used on the Cloudflare provider for making comment available for DNS records. That's helpful when you already have a lot of records and you'd like to track what external-dns is provisioning there and why.

Comment can be set as follows

• As a program flag --cloudflare-record-comment="Ingresses for my domain"
• Or as Ingress annotations, taking precedence over the program args

annotations:
  external-dns.alpha.kubernetes.io/cloudflare-record-comment: "An Ingress for my-app"

Fixes #3934

This is a subset of #5359
Tags were posing issues to work without causing a config drift and endless syncs; Will address these separately on a subsequent pull request.

Checklist

• Unit tests updated
• End user documentation updated

[k8s-ci-robot]

Hi @7onn. 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.

[ivankatliarchuk]

/ok-to-test

[ivankatliarchuk]

rest is reviewed here #5359

[ivankatliarchuk]

Any chance to move this logic under struct? To reduce complexity of current method?

Example

func (cfg DNSRecordsConfig) trimAndValidateComment( isPaidZone bool, comment, dnsName string) string {
	if len(comment) > freeZoneMaxCommentLength {
		if !isPaidZone {
			log.Warnf("DNS record comment is invalid. Trimming comment of %s. To avoid endless syncs, please set it to less than %d chars.", dnsName, freeZoneMaxCommentLength)
			return comment[:freeZoneMaxCommentLength-1]
		} else if len(comment) > paidZoneMaxCommentLength {
			log.Warnf("DNS record comment is invalid. Trimming comment of %s. To avoid endless syncs, please set it to less than %d chars.", dnsName, paidZoneMaxCommentLength)
			return comment[:paidZoneMaxCommentLength-1]
		}
	}
	return comment
}

[7onn]

updated! :) also improved the new test cases by iterating comment cases and covering all possibilities.

[ivankatliarchuk]

/label tide/merge-method-squash

[ivankatliarchuk]

/lgtm

I suggest to open a PR with documentation/examples update. If no plans to add docs as part of this PR

[ivankatliarchuk]

/lgtm

[ivankatliarchuk]

optional, as there are so many comments already. depends on readability/personal preference, but sometimes it's easier to read when there is a reduced nesting

example

if len(comment) <= freeZoneMaxCommentLength { return # early return }
if / else {}

which will be something like with early return

if len(comment) <= freeZoneMaxCommentLength {
	return comment
}

maxLength := freeZoneMaxCommentLength
if paidZone(dnsName) {
	maxLength = paidZoneMaxCommentLength
}

if len(comment) > maxLength {
	log.Warnf("DNS record comment is invalid. Trimming comment of %s. To avoid endless syncs, please set it to less than %d chars.", dnsName, maxLength)
	return comment[:maxLength]
}

return comment

[mloiseleur]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ivankatliarchuk, mloiseleur

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mloiseleur]

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

[mrozentsvayg]

there seems to be a bug preventing the cmd line flag --cloudflare-record-comment to work - it's stored in CloudflareRecordComment, but another introduced (and unset) CloudflareDNSRecordsComment is being used instead.
the flag is also not covered by tests unfortunately.

@7onn please let me know if you'll pick it up from there, or prefer the issue to be created or me to fix it

[7onn]

there seems to be a bug preventing the cmd line flag --cloudflare-record-comment to work - it's stored in CloudflareRecordComment, but another introduced (and unset) CloudflareDNSRecordsComment is being used instead. the flag is also not covered by tests unfortunately.

@7onn please let me know if you'll pick it up from there, or prefer the issue to be created or me to fix it

it was fixed already! just waiting for a patch!!! https://github.com/kubernetes-sigs/external-dns/pull/5582/files