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

Support setting external-dns.alpha.kubernetes.io/target on IngressClass #2717

Closed
mac-chaffee opened this issue Apr 20, 2022 · 3 comments
Closed
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@mac-chaffee
Copy link
Contributor

What would you like to be added:

Currently, this is how external-dns determines what hostname/IP to put in the DNS record that it creates for ingresses:

targets := getTargetsFromTargetAnnotation(ing.Annotations)
if len(targets) == 0 {
targets = targetsFromIngressStatus(ing.Status)
}

That code will either:

  1. Create a CNAME record using the value of external-dns.alpha.kubernetes.io/target on that particular Ingress, or
  2. Create an A/AAAA record using the status.loadBalancer.ingress.ip field on that particular ingress

I think it would be a good idea to add another option: Create a CNAME record using the value of external-dns.alpha.kubernetes.io/target on the IngressClass (taking precedence over the status field but not taking precedence over the target annotation on the actual Ingress).

Why is this needed:

Having a bunch of A records to the same IP seems more fragile than having a bunch of CNAMEs to your ingress's hostname. That can also support DNS-based load balancing as well.

You can force external-dns to create CNAMEs, but only if EVERY ingress has that external-dns.alpha.kubernetes.io/target annotation on it, which would be quite annoying to roll out to existing applications.

But if I could just add the external-dns.alpha.kubernetes.io/target annotation on the IngressClass and have external-DNS update all current and future ingresses to use CNAMEs, that would be much smoother.

Seems the work required would just be to add a getTargetsFromIngressClassTargetAnnotation() function and call it on line 176 in that code snippet above. Thoughts?

@mac-chaffee mac-chaffee added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 20, 2022
@balonik
Copy link

balonik commented Apr 21, 2022

Just a note that it creates CNAME record if you have status.loadbalancer.hostname on your Ingress resources. This would also require to update RBAC, because external-dns doesn't have rights to read IngressClass.

@mac-chaffee
Copy link
Contributor Author

I believe that only works for some loadbalancer providers. For example, metallb doesn't support hostnames, only IP address: metallb/metallb#1038

@mac-chaffee
Copy link
Contributor Author

Turns out that the reason my DNS records were coming from MetalLB is that my ingress provider (ingress-nginx) had the --publish-service flag, which places the loadBalancerIP in the status field of every Ingress.

But that behavior can be changed by removing that flag and adding the --publish-status-address=myhost.example.com flag. Then external-dns will create CNAMEs.

All ingress-nginx flags: https://kubernetes.github.io/ingress-nginx/user-guide/cli-arguments/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

2 participants