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 support for Traefik 2's IngressRoute, IngressRouteTCP and IngressRouteUDP #3055

Merged
merged 19 commits into from
Jun 16, 2023

Conversation

ThomasK33
Copy link
Contributor

Description

Added support for Traefik 2's custom resource definition.
The traefik-proxy source extracts hostnames from annotations and Traefik's host rule syntax.

Fixes #2286
Fixes traefik/traefik#4655

Checklist

  • Unit tests updated
  • End user documentation updated

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 28, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Sep 28, 2022
@k8s-ci-robot
Copy link
Contributor

Welcome @ThomasK33!

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 k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 28, 2022
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 28, 2022
@ThomasK33
Copy link
Contributor Author

@seanmalloy @stevehipwell @szuecs @Raffo, could someone of you please review this?

Copy link
Contributor

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasK33 the Helm chart changes look good to me, but I think you probably want to separate them into a different PR as the release cycle of the Helm chart differs from the binary.

Sorry but I'm not qualified to review the rest of the code.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 5, 2023
@mfin
Copy link

mfin commented Jan 15, 2023

@ThomasK33 @stevehipwell any updates here? Would love this feature.

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 15, 2023
@ThomasK33
Copy link
Contributor Author

/assign @szuecs

@szuecs szuecs removed their assignment Jan 16, 2023
@szuecs
Copy link
Contributor

szuecs commented Jan 16, 2023

/assign njuettner

@szuecs
Copy link
Contributor

szuecs commented Jan 16, 2023

@njuettner can you click the "run tests" button please?
Feel free to unassign afterwards.

@ThomasK33 as all other new providers get the same answer:
we plan to not add any new providers anymore, but create a way of getting providers out of tree. See also #3063

@ThomasK33
Copy link
Contributor Author

Hey @szuecs,
Thanks for the feedback.

As this is not a new provider but a new source, the aforementioned #3063 PR is not applicable here.

Could you, @njuettner, @Raffo, or @seanmalloy, please review this PR?
It's been waiting for more than four months now.

@ThomasK33
Copy link
Contributor Author

/assign @szuecs

Copy link
Contributor

@Raffo Raffo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a bunch of comments. I mostly skimmed the code and it looks legit.

docs/tutorials/traefik-proxy.md Outdated Show resolved Hide resolved

## Manifest (for clusters with RBAC enabled)

Could be change if you have mulitple sources
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what this sentence mean and it contains a typo. Can you please correct it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a fair point. I referenced the gloo proxy tutorial and carried it over.

Would you like me to fix/remove it from them as well?

docs/tutorials/traefik-proxy.md Outdated Show resolved Hide resolved
source/traefik_proxy.go Show resolved Hide resolved
@pavlospt
Copy link

Bumping for visibility. Should we expect a merge of this PR in the near future? Asking since our current implementation is using Ingress due to this limitation.

@Stormfox2
Copy link

This is not yet implemented in the helm char yet, right?

@heliochronix
Copy link

I don't think this has been included in any releases yet. I had to build it myself. I still haven't gotten it to work just yet though...

@anthonycorbacho
Copy link
Contributor

anthonycorbacho commented Aug 10, 2023

@heliochronix I made it work by adding these annotation to my ingressroute

    external-dns.alpha.kubernetes.io/target: <traefik target>
    kubernetes.io/ingress.class: traefik

so it looks like this

apiVersion: traefik.io/v1alpha1
kind: IngressRoute
metadata:
  name: myapp
  namespace: ns
  annotations:
    external-dns.alpha.kubernetes.io/target: traefik.example.com
    kubernetes.io/ingress.class: traefik
spec:
  entryPoints:
    - web
    - websecure
  routes:
    - match: Host(`app.example.com`)
      kind: Rule
      services:
        - name: app
          namespace: ns
          port: port

I also had to build from master to be able to make it work

@heliochronix
Copy link

Fantastic, thank you for the clarification! I see you updated the documentation as well with the example. I think the key thing I was missing was the /target:. Can it not discover that from the resource? Is that an improvement that could be made to external-dns? It seems like it discovers the target fine for regular Ingress resources.

@anthonycorbacho
Copy link
Contributor

@heliochronix That would be a very nice improvement indeed, i spent quite some time around this.

@cdenneen
Copy link

@ThomasK33 the /target annotation is for setting where you want to resolve to but if you are setting up the IngressRoute for the built-in dashboard you can't have traefik going to itself... you need the /target essentially going to the traefik LB that was setup.

So while this works if you have a record already setup for traefik.example.com it doesn't help create the traefik.example.com domain to begin with.

apiVersion: traefik.io/v1alpha1
kind: IngressRoute
metadata:
  annotations:
    external-dns.alpha.kubernetes.io/hostname: traefik.example.com
    kubernetes.io/ingress.class: traefik
    meta.helm.sh/release-name: traefik
    meta.helm.sh/release-namespace: platform
  labels:
    app.kubernetes.io/instance: traefik
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: traefik
    helm.sh/chart: traefik-25.0.0
  name: traefik-dashboard
  namespace: traefik
spec:
  entryPoints:
  - traefik
  - websecure
  routes:
  - kind: Rule
    match: Host(`traefik.example.com`)
    services:
    - kind: TraefikService
      name: api@internal

Currently my traefik is deployed creating an AWS NLB which can be used with Ingress and Traefik using the following helm values:

providers:
  kubernetesIngress:
    publishedService:
      enabled: true

technically the app.example.com example should be able to use the Host rule or a /hostname annotation like above and set the CNAME to the service LB address. You can always use /target to override that behavior but without /target the default should be to use the serviceLB address as /target unless otherwise specified.

time="2023-11-30T22:40:00Z" level=debug msg="No endpoints could be generated from Host traefik/traefik-dashboard"

@phyzical
Copy link

phyzical commented Jan 24, 2024

is building from source still required?

i gave it a go but

external-dns.alpha.kubernetes.io/target: <traefik target>
kubernetes.io/ingress.class: traefik

seems to have no effect, no logs in external dns

Edit: Opps, i overlooked the fact i didnt request traefik-proxy as a source.

Once i did all worked as expected

@z0mt3c
Copy link

z0mt3c commented Feb 6, 2024

y do we need the target annotation at all? where does ingress get the external lb hostname from? and y do we create cnames instead of a records?

@cdenneen
Copy link

cdenneen commented Feb 6, 2024

Ingress gets the external lb hostname from the Ingress Controller Loadbalancer status, so you shouldn't be required to hardcode a target like this.

@phyzical
Copy link

phyzical commented Feb 7, 2024

i dont see a status added to IngressRoute kind's which is why target is required

@z0mt3c
Copy link

z0mt3c commented Feb 7, 2024

i dont see a status added to IngressRoute kind's which is why target is required

Yeah, that's what I thought too - where does the status for ingress resources come from?

@cdenneen
Copy link

cdenneen commented Feb 7, 2024 via email

@phyzical
Copy link

phyzical commented Feb 8, 2024

@cdenneen

are you talking about this? if so i do have it enabled if not what is this flag?

providers:
  kubernetesIngress:
    publishedService:
      enabled: true

@cdenneen
Copy link

cdenneen commented Feb 8, 2024 via email

@phyzical
Copy link

phyzical commented Feb 9, 2024

@z0mt3c what we do is we have 1 ingress that creates the alb, external dns sets domain i.e lb.MYDOMAN then each app's ingressroute use the target: lb.MYDOMAIN

so then external dns just goes okay lets copy what lb.MYDOMAIN points at.

@z0mt3c
Copy link

z0mt3c commented Feb 28, 2024

@phyzical yeah had the same setup... but it sets cname- instead of a-records, doesn't it?

@phyzical
Copy link

Yes it's cnames

@HWiese1980
Copy link

Is giving the kubernetes.io/ingress.class annotation needed? First of all I would expect IngressRoute to use Traefik as default anyway since it is part of Traefik's CRDs. Furthermore, the default ingress class in my cluster is in fact traefik.

@sourcehawk
Copy link

Can someone explain what actually got merged here and how to use it? I'm trying to get a record into route53 with my grpc endpoint but clearly I'm not understanding something.

apiVersion: traefik.containo.us/v1alpha1
kind: IngressRoute
metadata:
  annotations:
    # This one has no effect
    # external-dns.alpha.kubernetes.io/hostname: thanos-remote-grpc.example.com
    # This one crashes external dns pod * cname loop error
    # external-dns.alpha.kubernetes.io/target: thanos-remote-grpc.example.com
    kubernetes.io/ingress.class: traefik
  name: thanos-receive-remote-grpc
spec:
  entryPoints:
  - websecure
  routes:
  - kind: Rule
    match: Host(`thanos-remote-grpc.example.com`)
    services:
    - name: monitoring-thanos-receive
      namespace: monitoring
      passHostHeader: true
      port: 19291
      scheme: h2c
  tls:
    secretName: letsencrypt
  • IngressRoute resources do not get picked up by external-dns even if the source traefik-proxy is added to the helm chart deployment.
  • The description of the usage of hostname and target annotations are a bit too vague for me to understand their purpose. I tried using the target label and set the desired record I want created but that results in cname loop error in external dns.

@DrummyFloyd
Copy link

DrummyFloyd commented Apr 5, 2024

@sourcehawk
#4035 (comment)

from what i understood ( i made it work like this )

thinnk it's a limitation atm, ingressRoute make only CNAME made by A records from the IP traefik svc

@sourcehawk
Copy link

sourcehawk commented Apr 6, 2024

@DrummyFloyd Can you explain the two domains being used? I do not understand why all these examples are using two different domains completely unrelated to one another. I am trying to expose my service at thanos-remote-grpc.example.com, why should there be something else in the /target annotation and what is that actually pointing to?

@cdenneen
Copy link

cdenneen commented Apr 6, 2024

The /target is the destination for the Host match thanos-remote-grpc.example.com your setting up in the IngressRoute which is why the CNAME loop(you can't have it CNAME to itself). If you know the endpoint /target works.
In my case the endpoint is the ALB setup by the traefik controller dns name which I don't know and why I haven't found a way for this to work. But if you know your /target then this would work.

@sourcehawk
Copy link

sourcehawk commented Apr 6, 2024

I found a workaround which works alright. Create an ingress along side the IngressRoute but make sure the grpc service has higher priority than the http ingress. I think it can also be used in case there are multiple endpoints on the same service where some are http/https and others grpc.

# Using this to get external dns to create a dns record (ingressroute not supported)
kind: Ingress
apiVersion: networking.k8s.io/v1
metadata:
  annotations:
    traefik.ingress.kubernetes.io/router.entrypoints: websecure
    traefik.ingress.kubernetes.io/router.priority: "10"
    traefik.ingress.kubernetes.io/router.tls: "true"
    cert-manager.io/cluster-issuer: letsencrypt
  name: thanos-query-grpc-dummy
spec:
  rules:
  - host: thanos-query-grpc.example.com
    http:
      paths:
      - path: /
        pathType: ImplementationSpecific
        backend:
          service:
            name: monitoring-thanos-query-grpc
            port:
              # could probably specify different port here as well?
              number: 10901
  tls:
    - secretName: letsencrypt
      hosts:
        - example.com
        - "*.example.com"
---
apiVersion: traefik.containo.us/v1alpha1
kind: IngressRoute
metadata:
  name: thanos-query-grpc
spec:
  entryPoints:
    - websecure
  routes:
    - match: Host(`thanos-query-grpc.example.com`) && Headers(`Content-Type`, `application/grpc`)
      kind: Rule
      priority: 11
      services:
        - name: monitoring-thanos-query-grpc
          namespace: monitoring
          port: 10901
          scheme: h2c
          passHostHeader: true
  tls:
    secretName: letsencrypt

@cdenneen
Copy link

cdenneen commented Apr 6, 2024

You're using the Ingress to create the DNS entry and the IngressRoute is just jumping on board. The underlying issue is that Ingress resources will get the controller SVC LB hostname from its status and External-DNS uses that to create the target for the dns entry. IngressRoute however doesn't seem to get the LB hostname from the controller SVC and that's why you have to use a /target. Others have done this by creating the Ingress with a DNS set to lb.domain.com and then set /target on IngressRoute to point Host match of app.domain.com to lb.domain.com. Either way you're using utilizing the fact that Ingress actually gets the hostname back from controller svc. Why can't IngressRoute do the same and avoid all this?

@sourcehawk
Copy link

I opted for this approach because having to know the loadbalancer name by manually picking it out of AWS defeats the point of using external dns for automation.

@jgsqware
Copy link

Hello

is this officialy supported?
I cannot make it work with latests ExternalDNS and traefik v3, either without annotation or with hostname and target annotation.

It works correctly with traditional Ingress Resources

@itspngu
Copy link
Contributor

itspngu commented Jun 19, 2024

It works, but it's a bit finnicky. The important steps are:

  1. Ensure the Kubernetes Service (usually of type LoadBalancer) backing your Traefik deployment has an existing DNS entry. This can be done via external-dns by adding a /hostname annotation to it:
apiVersion: v1
kind: Service
metadata:
  annotations:
    external-dns.alpha.kubernetes.io/hostname: traefik.example.com
  labels:
    app.kubernetes.io/name: traefik
  name: traefik
spec:
 type: LoadBalancer
  ports:
  - name: web
    port: 80
    protocol: TCP
    targetPort: web
  - name: websecure
    port: 443
    protocol: TCP
    targetPort: websecure
  selector:
    app.kubernetes.io/name: traefik
  1. Ensure your deployment of external-dns is running with --source=traefik-proxy in its commandline (specifying --source more than once is valid).
  2. (optional) If using the Traefik Helm chart version 28.0.0 or newer, you need to disable lookup of the deprecated traefik.containo.us/v1alpha1 resources, otherwise external-dns will crashloop. Add --traefik-disable-legacy to its commandline.
  3. Create an IngressRoute resource and add /hostname + /target annotations to it. This will make external-dns create a CNAME record for your IngressRoute, resolving what's specified in /hostname to /target. You'll most likely want it to end up at the IP address of the service from (1), so use:
apiVersion: traefik.io/v1alpha1
kind: IngressRoute
metadata:
  name: test
  annotations:
    external-dns.alpha.kubernetes.io/hostname: "test.example.com"
    external-dns.alpha.kubernetes.io/target: "traefik.example.com"
spec:
  entryPoints:
    - web
    - websecure
  routes:
    - match: Host(`test.example.com`)
      kind: Rule
      services:
        - name: my-service
          port: http

Tested and working on k8s v1.28.6, Traefik v3.0.2 (Helm chart v28.3.0), external-dns v0.14.2.

dig test.example.com

; <<>> DiG 9.18.24-0ubuntu0.22.04.1-Ubuntu <<>> test.example.com
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 3231
;; flags: qr rd ra; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 65494
;; QUESTION SECTION:
;test.example.com. IN A

;; ANSWER SECTION:
test.example.com. 300 IN CNAME	traefik.example.com.
traefik.example.com. 300 IN A	1.2.3.4

;; Query time: 36 msec
;; SERVER: 127.0.0.53#53(127.0.0.53) (UDP)
;; WHEN: Wed Jun 19 17:52:12 CEST 2024
;; MSG SIZE  rcvd: 116

@safts
Copy link

safts commented Jul 13, 2024

Thanks @itspngu , that worked.

In my case though, I had to also add service to source (although now I realize you probably meant to append traefik-proxy, not add that alone).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure KubernetesCRD provider works with external-dns external-dns with a Traefik 2.0 IngressRoute