feat: add generic PTR record support for all providers

[clwluvw]

What does it do ?

Introduces a provider-agnostic PTR record mechanism that wraps any provider as a decorator. A new --create-ptr flag accepts three modes: "off" (default), "always" (PTR for all A/AAAA records), and "annotation" (PTR only when the
source resource is annotated with external-dns.alpha.kubernetes.io/create-ptr).
The RFC2136 provider's built-in PTR logic is removed in favor of this generic approach, and the old --rfc2136-create-ptr flag is preserved for backward compatibility.

Motivation

PTR record creation was previously hardcoded into the RFC2136 provider only. Any other provider managing reverse DNS zones (e.g. PowerDNS) had no way to create PTR records automatically. This change makes PTR support available to all providers.

More

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

[k8s-ci-robot]

Welcome @clwluvw!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

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

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

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.

[ivankatliarchuk]

The main challenge is - to test at least in 3-4 different providers.

[ivankatliarchuk]

Year is not right.
Could you move it to blueprints folder this logic?

[ivankatliarchuk]

I'm not yet sure, could be even in endpoint package. Hard to say what the correct location is at the moment

[clwluvw]

We can't move it to blueprint, PTRProvider wraps provider.Provider and we will endup in circular dependency. Do you want me to put it under its own provider pkg?

[ivankatliarchuk]

’d say this is a very unusual approach from a coding perspective. If this was AI-generated(we not against it, but worth to mention that), it would be worth spending more time understanding the existing codebase first and consider few design options.

Custom functions for things like reverseaddr, comments that are longer than the code itself, a high degree of coupling, and the introduction of a provider wrapper. Honestly, this approach is very unusual.

Layers and designs explained here https://github.com/kubernetes-sigs/external-dns/tree/master/docs/contributing

[ivankatliarchuk]

/ok-to-test

[ivankatliarchuk]

true|false

[clwluvw]

@ivankatliarchuk - the option can take always, annotation, and off as explained in the PR to act differently. it doesn't operate only on two modes.

[ivankatliarchuk]

You are breaking project contract. Not sure. Will w8 for other reviewers then

[clwluvw]

@ivankatliarchuk - I have changed to to boolean. my only concern was to have a control so not always create ptr records as you might want to not have ptr for certain domains and so on. so with what you described in the project contract, now annotation will control it if it is false and will always create ptr records if the flag is there. please check my last commit.

[ivankatliarchuk]

If there is an annotation on a resource - it should be applied. If the user have a controll over annotation value - it could simply remove the annotation.

So if we have flag --no-create-ptr, the external dns will not create PTR record, unless the resource has specific annotation. If we have --create-ptr with annotation set to false -> ptr record will not be created.

This is very new approach for a project to controll target type over annotation. It will take a while for sure.

[clwluvw]

It will take a while for sure.

you mean you agree with the approach or hold a different opinion? :)

[ivankatliarchuk]

I’m not particularly strong on adding new features - I tend to focus more on redesign and standardisation. It might make sense to introduce a new annotation, but right now there’s a lack of design behind it.

For example, why not consider a more generic name like target-type, which could support multiple values in the future (e.g. PTR, NAPTR, etc.)?

Also, it’s much easier to review and reason about changes when they’re submitted as small, isolated PRs. Otherwise, it becomes difficult to assess the risk and potential impact across millions of deployments.

How I would do it myself

• PR rename flag, and move it out of the rfc provider
• PR to add filter
• PR to propose new annotation. For example use cases are not clear

^ all this chagnes will require testing, and end-2-end testing evidences, this is on top of unit tests coverage

[clwluvw]

Also, it’s much easier to review and reason about changes when they’re submitted as small, isolated PRs.

i tried to do that via commits, but fine also to do it via PR if it still makes sense.

^ all this chagnes will require testing, and end-2-end testing evidences, this is on top of unit tests coverage

Right, i also discovered some via testing in real with powerdns which i couldn't find them with unit testing. i thought the project already have a concept to mock other APIs. but if not we need to come with a concept for that otherwise it would be hard to say "i test it and it worked :D"

but right now there’s a lack of design behind it.

Agreed, the current implementation was my original thought about how to tackle it. i submited to get some initial feedback around it. thanks for that. but would be intresting to hear your ideas. I'm basically open to any implementation as my original concern was about PTR records, but i can see how it can be more generic.

[ivankatliarchuk]

I'm not yet sure, could be even in endpoint package. Hard to say what the correct location is at the moment

[ivankatliarchuk]

Move this endpoint.DomainFilter support to separate PR please

[clwluvw]

Opened #6234

[ivankatliarchuk]

What are the other options, aka annotations names?

[clwluvw]

In the last commit, true will opt-in and false will opt-out.

[ivankatliarchuk]

I mean the annotation name. We are using nouns. But create-ptr starts with a verb (create-), breaking the convention. The closest parallel is alias — you set alias: "true" to create an alias record. We don't call it create-alias. This is why I was asking, which other annotation names you have considered?

The --create-ptr CLI flag name most likley makes sense as a boolean flag, but the annotation doesn't need the create- prefix.

[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 ask for approval from ivankatliarchuk. 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

[ivankatliarchuk]

worth to review where DNSEndpoint supports PTR as well

[clwluvw]

worth to review where DNSEndpoint supports PTR as well

AFAIU, we can use:

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint
spec:
  endpoints:
  - dnsName: 2.49.168.192.in-addr.arpa
    recordType: PTR
    targets:
    - web.example.com

also we can do something like:

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint
metadata:
  name: web
spec:
  endpoints:
  - dnsName: web.example.com
    recordType: A
    targets:
    - 192.168.49.2
    providerSpecific:
    - name: record-type
      value: ptr

[ivankatliarchuk]

worth to review where DNSEndpoint supports PTR as well

AFAIU, we can use:

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint
spec:
  endpoints:
  - dnsName: 2.49.168.192.in-addr.arpa
    recordType: PTR
    targets:
    - web.example.com

also we can do something like:

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint
metadata:
  name: web
spec:
  endpoints:
  - dnsName: web.example.com
    recordType: A
    targets:
    - 192.168.49.2
    providerSpecific:
    - name: record-type
      value: ptr

Yeah, this is where we need other maintainers, but not sure whey someone is free. This case

apiVersion: externaldns.k8s.io/v1alpha1
kind: DNSEndpoint
spec:
  endpoints:
  - dnsName: 2.49.168.192.in-addr.arpa
    recordType: PTR

^ 100% we should support if we do not support, dedicated PR is required, as it is what DNSEndpoint is for.

In regards below example, second opinion is needed.

 providerSpecific:
   - name: record-type
      value: ptr

[clwluvw]

@ivankatliarchuk - thanks let me know if something is still needed from my side. After #6234 is merged I can rebase and do another test like e2e.sh there to test these scenarios on powerdns.