traefik-helm-chart icon indicating copy to clipboard operation
traefik-helm-chart copied to clipboard

fix(rbac): do not create clusterrole for namespace deployment

Open ChandonPierre opened this issue 1 year ago • 2 comments

A namespace scoped deployment should not create cluster scoped rbac.

ChandonPierre avatar Feb 27 '24 16:02 ChandonPierre

@ChandonPierre Thanks. You found an interesting way for rbac namespaced.

Would you please add test and some documentation in values.yaml ?

mloiseleur avatar Feb 28 '24 12:02 mloiseleur

@ChandonPierre Thanks. You found an interesting way for rbac namespaced.

Would you please add test and some documentation in values.yaml ?

Updated!

ChandonPierre avatar Feb 28 '24 14:02 ChandonPierre

This PR only works with traefik v3.0.0. Please update your if statements and your tests accordingly by adding something like: {{- if semverCompare "<3.0.0-0" (include "imageVersion" $) }}

Yes, you are correct. Updated to use the previous behavior on Traefik V2, and disable ingressclass lookup/ClusterRole when Traefik V3 and namespaced

ChandonPierre avatar Mar 08 '24 13:03 ChandonPierre

@ChandonPierre In its current form, this PR will be breaking for v3 users, using spec.ingressClassname with rbac.namespaced enabled. They will be forced to add annotations.

Wdyt about:

  1. Introducing this new provider.kubernetesIngress.disableIngressClassLookup in values for v3 users
  2. When both provider.kubernetesIngress.disableIngressClassLookup and rbac.namespaced are set, then it won't generate ClusterRole

mloiseleur avatar Mar 08 '24 14:03 mloiseleur

@ChandonPierre In its current form, this PR will be breaking for v3 users, using spec.ingressClassname with rbac.namespaced enabled. They will be forced to add annotations.

Wdyt about:

  1. Introducing this new provider.kubernetesIngress.disableIngressClassLookup in values for v3 users
  2. When both provider.kubernetesIngress.disableIngressClassLookup and rbac.namespaced are set, then it won't generate ClusterRole

I would argue the previous behvaior was a bug, or at the very least, a misnomer.

But I agree not regressing existing functionality. Updated PR with the suggested behavior.

ChandonPierre avatar Mar 09 '24 18:03 ChandonPierre