fix: support ownership recognition of txt records

[HartmannVolker]

What does it do ?

Adds support for managing txt records in a txt registry properly

Motivation

The support was already working partly, but was having issues when checking ownership records of txt records. This breaks the general management of a zone in external dns, because other records aren't properly managed in that case as well.

More

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

[k8s-ci-robot]

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

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.

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

[u-kai]

/ok-to-test

[coveralls]

Coverage Report for CI Build 25739840227

Coverage decreased (-0.005%) to 80.583%

Details

• Coverage decreased (-0.005%) from the base build.
• Patch coverage: No coverable lines changed in this PR.
• 1 coverage regression across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
openshift_route.go	1	82.93%

---

Coverage Stats

Relevant Lines:	21378
Covered Lines:	17227
Line Coverage:	80.58%
Coverage Strength:	1452.7 hits per line

---

💛 - Coveralls

[u-kai]

I think this change has no effect because ToEndpointName is not called by the TXTRegistry when processing TXT records.

If I’m mistaken, sorry about that.

[HartmannVolker]

I think this change has no effect because ToEndpointName is not called by the TXTRegistry when processing TXT records.

If I’m mistaken, sorry about that.

I think it's called in the Records function of the regisry.go. The issue is described in #6180 as well. Sorry, I forgot to link it before.

[u-kai]

Sorry for my incorrect comment earlier.
Looking at it more carefully, your fix does achieve what you intended.

I was initially concerned that supporting TXT records in the registry might create a recursive structure (TXT records managing TXT records),
but the heritage label check already prevents that from being an issue.

One thing I'm still curious about: do you know why TXT was excluded from supportedRecords in the first place?
I couldn't find a clear reason, and understanding the original intent would give me more confidence.
That said, I don't see a practical problem with the fix as-is, so LGTM from my side.

@mloiseleur Do you have any idea why TXT was excluded from supportedRecords historically?

[HartmannVolker]

One thing I'm still curious about: do you know why TXT was excluded from supportedRecords in the first place? I

Unfortunately I don't, but I would like to understand the reason as well. Because TXT records are already partially supported, as in you can create them using the DNSEndpoint CRD. This fix should help external-dns properly manage them after it created them in the first place.

[ivankatliarchuk]

Could you share similar results for this PR #5085 (comment)?

[HartmannVolker]

I'm running this branch in one of my clusters since last week and tested the following:

1. Create new ingress on an apex domain

   apiVersion: networking.k8s.io/v1
   kind: Ingress
   metadata:
     annotations:
       cert-manager.io/cluster-issuer: letsencrypt
       nginx.ingress.kubernetes.io/auth-secret: http-basic-auth
       nginx.ingress.kubernetes.io/auth-type: basic
     name: external-dns-test-ingress
     namespace: external-dns
   spec:
     rules:
       - host: external-dns-test.runs.onstackit.cloud
         http:
           paths:
             - backend:
                 service:
                   name: external-dns
                   port:
                     name: http
               path: /
               pathType: Prefix
     tls:
       - hosts:
           - external-dns-test.runs.onstackit.cloud
         secretName: external-dns-tls

2. Check that A-Record is correctly created

   Logs in STACKIT Webhook container:

   {"component":"stackitprovider","record":"ed-a.external-dns-test.runs.onstackit.cloud","content":"\"heritage=external-dns,external-dns/owner=non-prod-external-dns,external-dns/resource=ingress/external-dns/external-dns-test-ingress\"","type":"TXT","action":"CREATE","id":"c96d41d3-61ea-4b8e-9980-054ff1677e86"}
   {"component":"stackitprovider","record":"external-dns-test.runs.onstackit.cloud","content":"<redacted>","type":"A","action":"CREATE","id":"c96d41d3-61ea-4b8e-9980-054ff1677e86"}

   Checked that the Record was successfully created in STACKIT Portal ✔️

3. Added a DNSEndpoint to create a TXT Record

   apiVersion: externaldns.k8s.io/v1alpha1
   kind: DNSEndpoint
   metadata:
     name: external-dns-txt-test
     namespace: external-dns
   spec:
     endpoints:
       - dnsName: external-dns-test.runs.onstackit.cloud
         recordTTL: 300
         recordType: TXT
         targets:
           - "Managed TXT Record"

4. Check that TXT-Record is correctly created

   Logs in webhook container:

   {"component":"stackitprovider","record":"ed-txt.external-dns-test.runs.onstackit.cloud","content":"\"heritage=external-dns,external-dns/owner=non-prod-external-dns,external-dns/resource=crd/external-dns/external-dns-txt-test\"","type":"TXT","action":"CREATE","id":"c96d41d3-61ea-4b8e-9980-054ff1677e86"}
   {"component":"stackitprovider","record":"external-dns-test.runs.onstackit.cloud","content":"Managed TXT Record","type":"TXT","action":"CREATE","id":"c96d41d3-61ea-4b8e-9980-054ff1677e86"}

   Checked that the record was created in the STACKIT Portal

   

5. Delete DNSEndpoint resource

   Logs in webhook container:

   {"component":"stackitprovider","record":"ed-txt.external-dns-test.runs.onstackit.cloud.","content":"\"heritage=external-dns,external-dns/owner=non-prod-external-dns,external-dns/resource=crd/external-dns/external-dns-txt-test\"","type":"TXT","action":"DELETE","id":"d9c219e8-654a-4ffd-92af-2284ce1bd49e"}
   {"component":"stackitprovider","record":"external-dns-test.runs.onstackit.cloud.","content":"Managed TXT Record","type":"TXT","action":"DELETE","id":"4405b3b3-2670-493c-9f14-3b29414d384c"}
   {"component":"stackitprovider","record":"ed-txt.external-dns-test.runs.onstackit.cloud.","content":"\"heritage=external-dns,external-dns/owner=non-prod-external-dns,external-dns/resource=crd/external-dns/external-dns-txt-test\"","type":"TXT","action":"DELETE","id":"d9c219e8-654a-4ffd-92af-2284ce1bd49e"}

6. Delete the ingress

   Logs of the webhook container:

   {"component":"stackitprovider","record":"external-dns-test.runs.onstackit.cloud.","content":"<redacted>","type":"A","action":"DELETE","id":"388772d7-81c3-49e6-a244-5dfea0461720"}
   {"component":"stackitprovider","record":"ed-a.external-dns-test.runs.onstackit.cloud.","content":"\"heritage=external-dns,external-dns/owner=non-prod-external-dns,external-dns/resource=ingress/external-dns/external-dns-test-ingress\"","type":"TXT","action":"DELETE","id":"dac33753-040f-4c7c-bcd1-d0a008eab6bc"}
   {"component":"stackitprovider","record":"external-dns-test.runs.onstackit.cloud.","content":"<redacted>","type":"A","action":"DELETE","id":"388772d7-81c3-49e6-a244-5dfea0461720"}
   {"component":"stackitprovider"}
   {"component":"stackitprovider","record":"ed-a.external-dns-test.runs.onstackit.cloud.","content":"\"heritage=external-dns,external-dns/owner=non-prod-external-dns,external-dns/resource=ingress/external-dns/external-dns-test-ingress\"","type":"TXT","action":"DELETE","id":"dac33753-040f-4c7c-bcd1-d0a008eab6bc"}

7. Checked that all records and ownership records have been cleaned up ✔️