Add support for NS delegation and MX records

[thriqon]

Code was already almost ready for NS and MX records, only canonicalization was missing.

The code for canonicalization was slightly buggy, refactored and fixed.

Tested with a real Designate setup.

[linwalth]

Question: Does the webhook provider in external-DNS even support management of MX records?
https://kubernetes-sigs.github.io/external-dns/latest/docs/sources/mx-record/ does not list webhook provider.

But if it works, and this is up to the maintainers' standards, please merge this soon.
Our team also need this feature. :)

Edit: I asked a question to the community here: kubernetes-sigs/external-dns#6028

[thriqon]

@linwalth , we' re doing it successfully right now :) I think the limitation in the docs is not actually true.

[frittentheke]

@linwalth , we' re doing it successfully right now :) I think the limitation in the docs is not actually true.

And I shall get to this PR and the other issues by the end of this week so the webhook will also do it all ;-)

[linwalth]

@thriqon I concur. Elsewise webhook providers for mikrotik or hetzner would not be able to create MX records either. Probably just lack of documentation on external-DNS side.

@frittentheke Thank you very much!

[frittentheke]

Sorry this all took me a while. I had to (re-)create a proper test setup with K8s + DevStack since external-dns does NOT support all records types via the fake source -- see kubernetes-sigs/external-dns#6042.

And to make MX records, which usually are APEX records usable, there is more work needed on the external-dns side needed, I believe:

• Creating dns records for apex / root domains kubernetes-sigs/external-dns#449
• feat(txt-registry): add apex record delegation check kubernetes-sigs/external-dns#5864
• feat: add --txt-prefix-override flag to support apex domains safely kubernetes-sigs/external-dns#6026

as I ran into these issues when testing with examples similar to e.g. https://kubernetes-sigs.github.io/external-dns/latest/docs/sources/mx-record/ and https://kubernetes-sigs.github.io/external-dns/latest/docs/sources/ns-record/

Do you mind sharing your custom resources (redact the domain if you want), so I can reproduce in which cases your PR should work?

[frittentheke]

@thriqon thanks a bunch for your time and contribution.
In case you did not notice, I merged a few unrelated changes and updates and also released v2.1.0 of the webhook and rebased your branch.

[frittentheke]

Unfortunately this will need further adjustment as I noticed we don't even support all record types supported by external-dns (https://github.com/kubernetes-sigs/external-dns/blob/e22cd737cd666db7394df506cd3f1a596a57a184/endpoint/endpoint.go#L32-L65 ?).

Not your fault and not part of the change, but since I just looked a little closer at the code, this really needs fixing.

I recently also fiddled with the proper format of SRV records emitted by external-dns itself, see
kubernetes-sigs/external-dns@fde978f, but there is not even SRV support in this webhook provider yet.

After all, adding support for more records types can also happen after your MR is merged.

[linwalth]

😬 Hello! It's been a while ... how is this coming along?

[frittentheke]

😬 Hello! It's been a while ... how is this coming along?

Sorry @linwalth this got a little out of focus.
With kubernetes-sigs/external-dns#6308 coming in to make CI / testing of all records types a little easier, I shall pick up on the backlog of things for this webhook provider soon.

Sorry 'bout the delay and thanks for your patience.

[linwalth]

No worries. Thank you for the update!

[linwalth]

Hello! It's been another while ... how is this coming along? ;)

[frittentheke]

Hello! It's been another while ... how is this coming along? ;)

I plan on working through all of the backlog towards the end of the week. There were more PRs and suggestions and I want them all merged or at least reviewed.

[frittentheke]

@thriqon @linwalth I am again incredibly sorry, this PR was sitting here for so long.
I finally got around to work this webhook provider again, including this PR adding support for MX and NS records!

I initially used the recently added support for most / all record types in the Fake source (kubernetes-sigs/external-dns#6042) to smoke test your code (against a production OpenStack cloud).

While an apex MX record was indeed added, there seems to be an issue with the TXT registry supporting these apex records properly (within the zone itself) - see issue: kubernetes-sigs/external-dns#449 and the requirement to configure e.g. --txt-prefix="someprefix-%{record_type}." to even support apex records with the external-dns TXT registry.
This being non-default and also likely be a breaking change to existing TXT records, it's really a bummer for the MX record capability ... but one thing after the other ...

Since all of this has lots of string-massaging moving parts, my test / reproduction setup now consists of:

1. A KIND Kubernetes cluster (with external-dns + this webhook in any suitable shape or form, e.g. your PR applied)
2. A Devstack OpenStack with Designate and also a production OpenStack

Do you mind sharing your custom resources (redact the domain if you want), so I can reproduce the various new capabilities your PR adds?

@thriqon May I kindly ask again for your CRs or other test data to verify your PR?
I just used a few DNSEndpoint CRs and already seem to be running into issues ...

I set the relevant values for the external-dns-1.21.1 Helm chart to:

[...]

sources:
  - service
  - ingress
  - crd

policy: sync

registry: txt
txtOwnerId:  kind
txtPrefix:  "xdns-%{record_type}." 

[...]

1. For the new NS records capability I used

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint
metadata:
  name: example-ns-record
spec:
  endpoints:
  - dnsName: zone.example.com
    recordTTL: 300
    recordType: NS
    targets:
    - ns123.some.dnsprovider.test
    - ns456.other.hoster.test

which seems to work fine to add, remove and also update the corresponding records.

2. Regarding MX records I was not so lucky.

a) Setting an MX record for a non-apex host like

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint
metadata:
  name: example-mx-record
spec:
  endpoints:
  - dnsName: mail.example.com
    recordTTL: 60
    recordType: MX
    targets:
      - "10 mailhost1.example.com"
      - "20 mailhost2.subdomain.someotherdomain.test"

seems to work just fine (create, update, delete).

But trying it with the apex like

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint
metadata:
  name: example-apex-mx-record
spec:
  endpoints:
  - dnsName: example.com
    recordTTL: 60
    recordType: MX
    targets:
      - "10 mailhost1.example.com"
      - "99 mailhost2.subdomain.anewprovider.test"

it seems the record even attempted to be created via the webhook provider.
I am unsure whether this might actually be an issue with external-dns, but still, the capability to set MX records only makes sense if it can also do apex records.