@hdfln , thank you for contributing! Please add documentation to "CNAME Field Rewrites" section of the README.md for the plugin.

@greenpau, thank you for the feedback! I've added a clarification to the "CNAME Field Rewrites" section describing how answer records are handled during the CNAME rewrite.

@hdfln , please see DCO.

@hdfln , how do you know that DNS clients will resolve my-app.com to my-app.com.cdn.example.net. properly?

my-app.com.                  200  IN  CNAME  my-app.example.
my-app.example.              200  IN  CNAME  my-app.com.cdn.example.net.

Which of the common client libraries might have an issue with this?

Also, should this behavior be default or configurable?

@johnbelamaric , what do you think about this one?

@greenpau, thank you for the review!

please see DCO.

Thank you. I fixed my commit.

how do you know that DNS clients will resolve my-app.com to my-app.com.cdn.example.net. properly?

I tested with dig and nslookup using a similar CNAME chain structure (with different domain names). With this fix, both tools displayed the complete response including the final IP address.

Which of the common client libraries might have an issue with this?

I tested with dig and nslookup. Without this fix, both tools displayed an incomplete response missing the final A record when resolving CNAME chain. The issue was not with the clients, but with CoreDNS: without this fix, the response of CoreDNS did not include all records from the upstream lookup after rewriting the CNAME target.

Also, should this behavior be default or configurable?

I believe this should be the default behavior. I don't see a valid use case where incomplete resolution would be the desired outcome.

I tested with dig and nslookup using a similar CNAME chain structure (with different domain names). With this fix, both tools displayed the complete response including the final IP address.

@hdfln , there is slight difference between "tools" (dig and nslookup) vs "libraries" (e.g. miekg/dns, c-ares, etc.)

Say one of these libraries does not do chain resolution below for whatever reason. There could be an issue.

my-app.com.                  200  IN  CNAME  my-app.example.
my-app.example.              200  IN  CNAME  my-app.com.cdn.example.net.

I tested with dig and nslookup. Without this fix, both tools displayed an incomplete response missing the final A record when resolving CNAME chain. The issue was not with the clients, but with CoreDNS: without this fix, the response of CoreDNS did not include all records from the upstream lookup after rewriting the CNAME target.

👍

there is slight difference between "tools" (dig and nslookup) vs "libraries" (e.g. miekg/dns, c-ares, etc.)
Say one of these libraries does not do chain resolution below for whatever reason. There could be an issue.

@greenpau, thank you for your clarification about libraries.
I'm not entirely sure I understand your concern correctly, but clients normally do not perform chain resolution themselves.
They simply receive and handle the complete response from a full-service resolver like CoreDNS.
Client libraries expect to receive a complete response, so the previous behavior (returning an incomplete response) would have been problematic to client libraries.

Does this address your concern, or did I misunderstand your point?

@hdfln , that is the concern. When a single response contains a chain of resolution for CNAME.

@greenpau, thank you for clarifying.

Is your concern about DNS client libraries that cannot handle CNAME chains?

From my understanding, DNS client libraries do not have issues with CNAME chains.
They expect a complete answer from recursive resolvers, so incomplete responses (like the previous behavior) would be more problematic.

I tested with miekg/dns just in case, and it received the CNAME chain correctly.

Does this address your concern?

@hdfln , you are proposing a change to defaults. Imagine that some client library that supports 20% of the Internet traffic does not do the chains for whatever reason. We merge it. That will not look good. I am asking you to consider doing the due diligence, understand, and then share the impact of the change.

@greenpau , thank you again for your feedback.
I have already checked that miekg/dns can handle CNAME chain, so I will also test c-ares and let you know.

@greenpau, I tested with c-ares 1.34.5 and it successfully received and parsed the complete CNAME chain from CoreDNS.
Is there anything else that I should check?

@greenpau I don't quite understand your concern. Imagine the target of the rewrite was queried directly, so without a rewrite rule. Then wouldn't the full upstream response be exactly what would be returned? So how could including all upstream records after a rewrite be any different for clients/libraries?

@mwijngaard , the main principle is “not knowing what you don’t know”. Somehow, “fix” was never an issue for many years. The rewrites worked as is. Now, this changes the default behavior for all CNAME rewriting. I want to make sure that this does not impact the users’ community. Obviously this change works for @hdfln. The question is whether it works for the rest of us?

@mwijngaard , how did you run into this issue? I saw you created a duplicate issue. What is the pattern you saw that necessitated this PR? On one side, it could be a coincidence that you both ran into it. On the other side, is there something else that made this pattern a thing?

@hdfln , let me do some testing of my own. Will get back to you next week.

@greenpau I use tailscale egress proxies to connect kubernetes workloads to an ip restricted server. The domains hosted on the server use a cname record to resolve to the server, like:

• domain1.example.com CNAME server1.example.com
• server1.example.com A 10.11.12.13

When my managed kubernetes provider had coredns <1.11, I used to use a query rewrite for each domain, but now that it has a more recent version I wanted to make use of the existing CNAME structure to just require 1 rewrite rule per server, instead of 1 rewrite rule per domain.

However, the tailscale kubernetes egress proxy mechanism uses a kubernetes service, which is basically a CNAME that points to a specific pod the currently runs the proxy:

• egress-server1.ns.svc.cluster.local CNAME egress-server1-123456-0.ns.svc.cluster.local
• egress-server1-123456-0.ns.svc.cluster.local A 10.11.12.13

So when I use a cname rewrite rule, the end result should be:

• domain1.example.com CNAME egress-server1.ns.svc.cluster.local
• egress-server1.ns.svc.cluster.local CNAME egress-server1-123456-0.ns.svc.cluster.local
• egress-server1-123456-0.ns.svc.cluster.local A 10.11.12.13

But because currently, only the exactly matching upstream record is returned, the A record is left out.

My kubernetes pods/containers use the coredns running in the cluster as their dns resolver directly, and don't have any other mechanism that facilitates recursion, so that doesn't work.

As for both of us running into it at roughly the same time after cname rewrites having been implemented more than 2 years ago, I think that's mostly a coincidence, but it might have something to do with which coredns version ships with kubernetes distributions.

I hope that helps for some context.

@hdfln , thank you for contributing 👍