search

traefik / traefik

The Cloud Native Application Proxy
https://traefik.io
MIT License
49.07k stars 4.93k forks source link

Traefik v3 breaks Kubernetes Ingress Prefix Path Regex #10672

Open fkollmann opened 2 months ago

fkollmann commented 2 months ago

Welcome!

What did you do?

I upgraded from Traefik v2 to v3. Then I realized that all Kubernetes Ingress configurations which use a regex within as part of the prefix path (pathType=Prefix) break.

Workaround

Add the following lines to the static config:

core:
  defaultRuleSyntax: v2

Suggestions

  1. Revert to previous behavior for pathType=Prefix
  2. Have an option for the Kubernetes Ingress provider to default to v2 behavior, by default
  3. Have the Kubernetes Ingress provider to use PathRegexp('^...') insternally, instead
  4. Add new PathRegexp value to traefik.ingress.kubernetes.io/router.pathmatcher annotation

What did you see instead?

Kubernetes Ingress configurations which use a regex within as part of the Prefix path no longer work.

What version of Traefik are you using?

v3.0.0

What is your environment & configuration?

providers:
  kubernetesIngress:
    ingressClass: traefik
    allowEmptyServices: true
    allowExternalNameServices: true
    throttleDuration: "3s"

Kubernetes Ingress Example:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: catalogservice-ingress
  namespace: uplift
  annotations:
    traefik.ingress.kubernetes.io/router.entrypoints: https
    traefik.ingress.kubernetes.io/router.tls: "true"
spec:
  ingressClassName: traefik
  rules:
    - host: 'api.xxx'
      http:
        paths:
          - path: /xxx/v1/yyyy/{customer:[a-z0-9]+}/push/
            pathType: Prefix
            backend:
              service:
                name: catalogservice-service
                port:
                  name: http

If applicable, please paste the log output in DEBUG level

No response

emilevauge commented 2 months ago

Hey @fkollmann, As it's explained in the migration guide, defaultRuleSyntax needs to be set to v2 if you want to keep the v2 format by default. This should be used temporarily, while migrating to the default v3 syntax.

jim-barber-he commented 2 months ago

If you set defaultRuleSyntax to be v2 will Ingresses using v3 syntax (whatever that may be, I haven't found how it differs yet) still work? I'm trying to work out how you'd migrate without an outage.

jim-barber-he commented 2 months ago

I feel that this bug shouldn't have been closed? I cannot get regex's working for an Ingress using Traefik v3 Using the example given for this issue:

          - path: /xxx/v1/yyyy/{customer:[a-z0-9]+}/push/

I've tried writing it two other ways and the v3 parser does not treat either as a regex:

          - path: /xxx/v1/yyyy/{[a-z0-9]+}/push/
          - path: /xxx/v1/yyyy/[a-z0-9]+/push/
nmengin commented 2 months ago

Hello @jim-barber-he,

Could you confirm that the bug you described may be fixed by this PR?

HalloTschuess commented 2 months ago

Hi @nmengin,

Author of the mentioned PR here. I don't think my PR fixes this issue. Further more it might even make the workaround (defaultRuleSyntax: v2) impossible. Therefore I would remove my changes regarding ingress configuration from my PR.

Digging through the code it seems like that with the current implementation it is not possible to configure ingress paths with regex matching. Only Path(...) and PathPrefix(...) are used internally.

Maybe using path type ImplementationSpecific in combination with the annotation traefik.ingress.kubernetes.io/router.pathmatcher could be another workaround. But this feels kinda hacky and the documentation says only Path and PathPrefix should be used as path matchers.

To reiterate, it is not mentioned in the migration guide that it is no longer possible to use regex for routing in this use case. So I think this is a real bug and should be reopened.

Disclaimer: Unfortunately I have no experience with Kubernetes ingress configuration. I purely went with what's in the code and might have missed something. So take this with a grain of salt.

nmengin commented 1 month ago

Hello @HalloTschuess,

Thank you for your feedback. I re-opened the issue to discuss it during the next triage with the other maintainers.

So I think this is a real bug and should be reopened.

From what I understand in this issue, such behavior is not expected indeed.

corybolar commented 2 weeks ago

I do not believe #10689 resolved the issue. I am unable to have ingress configurations with regex paths without setting defaultRuleSyntax: v2 using traefik 3.0.2.

rtribotte commented 4 days ago

Hi @fkollmann @jim-barber-he @HalloTschuess @corybolar,

We have discussed this issue among the maintainers. This issue is a consequence of a major change in v3 to make it easier to use the rule matcher syntax. To do this, we have introduced a mechanism to switch the interpreted version of the rule matchers to make migration easier.

Unfortunately, in the case of the Kubernetes Ingress provider, where Traefik reinterprets the Kubernetes resources according to its dynamic configuration, there is no way to mitigate the disruptive change to the rule syntax of the router with the Prefix path type by fixing the version that the rule should be interpreted.

We understand that this is a real concern when dealing with the migration from v2 to v3, and we suggest the following:

HalloTschuess commented 4 days ago

Hi @rtribotte @corybolar ,

I'm not quite sure why #10689 should be reverted. It addresses an entirely different issue (#10688) and was never intended to resolve this problem. As per my previous post, I even removed changes from that PR since it would have broken the "workaround" with defaultRuleSyntax: v2.

In the PR, only the default-router is explicitly set to v3, with the route hardcoded to PathPrefix("/"). If I didn't miss something these values are not relevant to other ingress routers. The ingress configuration version remains unchanged and can still be set to v2 using defaultRuleSyntax: v2, as @corybolar mentioned. Therefore I don't think there is a conflict with any new solution or config option.

But it is entirely possible that there is something I didn't take into account. In any case I still vote against reverting #10689 since it does not break anything that previously worked at least as far as I know.

The other two suggestions seem reasonable. In addition, it might also be helpful if the proposed new annotation will be mentioned in the syntax remediation section of the migration guide.

Feel free correct me if I'm wrong any statement made or if I misunderstood something.