Skip to content

Let Traefik's route traffic across namespaces via IngressRoute resources#569

Merged
consideRatio merged 1 commit intodask:mainfrom
olivier-lacroix:fix-traefik-crossnamespaces
Jun 6, 2022
Merged

Let Traefik's route traffic across namespaces via IngressRoute resources#569
consideRatio merged 1 commit intodask:mainfrom
olivier-lacroix:fix-traefik-crossnamespaces

Conversation

@olivier-lacroix
Copy link
Contributor

@olivier-lacroix olivier-lacroix commented Jun 6, 2022

Hello,

Setting allowCrossNamespace=true fixes #568

see https://doc.traefik.io/traefik/providers/kubernetes-crd/#allowcrossnamespace

Not 100% sure of the security implication of that setting - would it be preferable to gate it behind a chart parameter?

Cheers,

Olivier

@olivier-lacroix olivier-lacroix changed the title Set allowCrossNamespace=true to handle multi-domain Set allowCrossNamespace=true to handle multi-namespaces Jun 6, 2022
@consideRatio
Copy link
Collaborator

consideRatio commented Jun 6, 2022

Ah nice investigative work! Is this tested to work already?

Btw, lots of test failures are unrelated, not sure if all are though.

@olivier-lacroix
Copy link
Contributor Author

@consideRatio , yes, pulling the chart and patching it with that additional argument solves it for me.

@olivier-lacroix
Copy link
Contributor Author

Indeed, the tests don't look too good!

@consideRatio
Copy link
Collaborator

consideRatio commented Jun 6, 2022

@olivier-lacroix excellent okay!

Since users don't create IngressRoute objects themselves, but dask-gateway does it for them to specifically send traffic to the users created dask-cluster resources (scheduler/workers), I think we should be fine. If we didn't allow cross-namespaced work, I'd argue we should lower the permissions granted to traefik as well, but we have instead granted significant permissions.

kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: {{ include "dask-gateway.traefikName" . }}
rules:
- apiGroups:
- ""
resources:
- pods
- services
- endpoints
- secrets

I think the intention was to support this by default historically, but with modern versions of traefik, allowing this by default changed in v2.3.5: traefik/traefik#7595

@consideRatio consideRatio changed the title Set allowCrossNamespace=true to handle multi-namespaces Let Traefik's route traffic across namespaces via IngressRoute resources Jun 6, 2022
@olivier-lacroix
Copy link
Contributor Author

@olivier-lacroix excellent okay!

Since users don't create IngressRoute objects themselves, but dask-gateway does it for them to specifically send traffic to the users created dask-cluster resources (scheduler/workers), I think we should be fine. If we didn't allow cross-namespaced work, I'd argue we should lower the permissions granted to traefik as well, but we have instead granted significant permissions.

kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: {{ include "dask-gateway.traefikName" . }}
rules:
- apiGroups:
- ""
resources:
- pods
- services
- endpoints
- secrets

I think the intention was to support this by default historically, but with modern versions of traefik, allowing this by default changed in v2.3.5: traefik/traefik#7595

OK, great then!

@consideRatio consideRatio merged commit 5e63fa6 into dask:main Jun 6, 2022
Comment on lines +47 to 49
- "--providers.kubernetescrd.allowCrossNamespace=true"
- '--providers.kubernetescrd.labelselector=gateway.dask.org/instance={{ include "dask-gateway.fullname" . }}'
- "--providers.kubernetescrd.throttleduration=2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, one confusion point for me is case sensitivity.

Is this accepted? Looking at https://doc.traefik.io/traefik/providers/kubernetes-crd/#allowcrossnamespace, it could make more sense if this was --providers.kubernetesCRD.allowCrossNamespace=true or --providers.kubernetescrd.allowcrossnamespace=true in a way.

I wonder if there is a silent issue with this or other flags? I saw though in the logs of traefik that this was enabled, but with a warning asking if it was intentional or not, which seems weird if we explicitly try to set it.

I'm confused about how we pass these flags overall, hmmm...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think everything is case insensitive, nevermind.

@consideRatio consideRatio added the bug Something isn't working label Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working codebase:helm-chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard is unavailable for namespaces different from the gateway

2 participants