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

#298: Add support to TLS domains for dashboards #299

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eyal-lantzman
Copy link

See related issue: #298

@jcrist
Copy link
Member

jcrist commented Aug 5, 2020

IIUC the goal here is to have the dashboard traffic served to the user over TLS, is that correct?

If so, I think an easier way would be to add TLS configuration support for both of the existing entrypoints (web and tcp). Then both the api and dashboard traffic could be served over TLS without the need for additional logic in the controller.

I'm not sure how best to offer TLS configuration via the helm chart. Ideally we could support both self-provided certs as well as letsencrypt generated ones. I'd probably look at what the zero-to-jupyterhub-k8s helm chart offered for inspiration. Thoughts?

@eyal-lantzman
Copy link
Author

Good point, I need to provide more context as to why I went for this implementation:
For organizations that can leverage helm charts, that might be a good solution, but in other organizations things like RBAC, restricted access to K8s secrets and special resource-type specific annotations prevent leveraging helm charts from being useful (assuming the organization allows using of helm charts in the first place).
If you take all those things into consideration, then hard-coded traefik resources in the controller are the real pain point here, the other K8s resource in there have extraXXXConfig which is very useful!

I'm trying to bring dask gateway into the company I'm working for, and helm chart was useful to generate the initial yaml files, which then had to be tweaked quite a bit. Perhaps, once I get it all working I can contribute the additional extension points to helm charts to make it usable in highly controlled K8s environments.

I managed to solve many issues (inc. securing the API) and this PR is one of the few remaining outstanding issues (still investigating some of the other ones). I chose the minimal change to reduce likely hood for breaking changes and nasty side effects.

If there's a way to configure traefik endpoint with the settings in the PR (not certs/secrets!) and is external to the controller, please let me know, as that would reduce the need for this PR altogether.

@eyal-lantzman
Copy link
Author

@jcrist , any push back or concerns about this targeted fix?

Base automatically changed from master to main March 9, 2021 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants