Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the CoreDNS maintained alternate plugin to the build by default. #6549

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ThreeSixes
Copy link

1. Why is this pull request needed and what does it do?

This PR compiles the CoreDNS alternate package as part of the standard binary and Docker image that ship with various distributions or CoreDNS. This is especially useful for Kubernetes cluster operators that provide internal DNS servers which resolve specific domains locally to make sure requests stay inside a specific network or cluster unless the host resolving the address is outside the cluster for both fault tolerance and other purposes, but also leverage something like cert-manager which requires external TXT records to be generated and resolved for the DNS-01 resolution mechanism to function. Using the alternate plugin provides a mechanism to resolve entries that are only available on public DNS servers to allow these mechanisms to appropriately function without maintaining self-compiled and hosted versions of CoreDNS.

2. Which issues (if any) are related?

None

3. Which documentation changes (if any) need to be made?

Any relevant documentation about included plugins should be updated to include information indicating the alternate plugin has been included and should link to the repo.

4. Does this introduce a backward incompatible change or deprecation?

No.

Signed-off-by: ThreeSixes <joshlucy@gmail.com>
@ThreeSixes ThreeSixes changed the title Enable the alternate plugin by default. Add the CoreDNS maintained alternate plugin to the build by default. Mar 13, 2024
@ThreeSixes
Copy link
Author

@chrisohaver here's the new PR.

@chrisohaver
Copy link
Member

I'm not opposed to making alternate built-in by default. I think it's one of the more commonly used external plugins.

If we do leave alternate in a separate repository and include it as we would an external plugin, we'll need to work out how we want to handle the docs. I think the easiest way would be to create /plugins/alternate/README.md in this repo, copying/tweaking the README.md from the alternate repo. This should allow it to get automatically picked up by coredns.io site builder.

Another thing to consider is if we should extend the forward plugin to handle the alternate feature as a new configurable option, instead of it being a separate plugin. I suspect it was originally proposed to be, but then implemented as a separate plugin after not being accepted at the time.

@ThreeSixes ThreeSixes closed this Mar 13, 2024
@ThreeSixes ThreeSixes reopened this Mar 13, 2024
@chrisohaver
Copy link
Member

chrisohaver commented Apr 26, 2024

I think it would be significantly less code to add an alternate option to the existing forward plugin, than to include the alternate plugin as a separate plugin. I think this also may be more intuitive from a configuration standpoint.

One way to do it would be to add the option alternate RCODE_1 [,RCODE_2, RCODE_3...], which will pass the request to the next forward instance before writing to client if the response from upstream matches any of the RCODEs. This would also allow chaining more than one alternate without having to leverage local forwarding and multiple server blocks.

e.g. The following would try 1.2.3.4 first. If the response is NXDOMAIN, try 5.6.7.8. If the response from 5.6.7.8 is NXDOMAIN, try 9.0.1.2.

. {
  forward . 1.2.3.4 {
    alternate NXDOMAIN
  }
  forward . 5.6.7.8 {
    alternate NXDOMAIN
  }
  forward . 9.0.1.2 {
  }
}

dihmandrake added a commit to dihmandrake/coredns that referenced this pull request May 3, 2024
    coredns#6549 (comment)

This still needs some work and especially tests
dihmandrake added a commit to dihmandrake/coredns that referenced this pull request May 14, 2024
Allows to the forward plugin to execute the next plugin based on the return code. Similar to the externally mainted alternate plugin https://github.com/coredns/alternate

Based on the idea of chrisohaver@ in coredns#6549 (comment)

I am having issues adding a proper test for functionallity. Primarilly, I do not now the code base enough and having multiple `dnstest.NewServer` with ResponseWriter does not work. From my testing these are "Singletons" and only the last defined response writer is used for all servers
dihmandrake added a commit to dihmandrake/coredns that referenced this pull request May 14, 2024
Allows the forward plugin to execute the next plugin based on the return code. Similar to the externally mainted alternate plugin https://github.com/coredns/alternate

Based on the idea of chrisohaver@ in coredns#6549 (comment)

I am having issues adding a proper test for functionality. Primarily, I do not know the code base enough and having multiple `dnstest.NewServer` with ResponseWriter does not work. From my testing these are "Singletons'' and only the last defined response writer is used for all servers
dihmandrake added a commit to dihmandrake/coredns that referenced this pull request May 14, 2024
Allows the forward plugin to execute the next plugin based on the return code. Similar to the externally mainted alternate plugin https://github.com/coredns/alternate

Based on the idea of chrisohaver@ in coredns#6549 (comment)

I am having issues adding a proper test for functionality. Primarily, I do not know the code base enough and having multiple `dnstest.NewServer` with ResponseWriter does not work. From my testing these are "Singletons'' and only the last defined response writer is used for all servers

Signed-off-by: jasper.bernhardt@gmail.com <ajbe@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants