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

Implement support for init containers and extra volumes/mounts for ssh #75

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sgaist
Copy link

@sgaist sgaist commented Oct 4, 2022

While the service itself does not require them, there are situations that may.

The example at hand is connecting to a JupyterHub instance that uses a custom certificate authority.

To the best of my knowledge there are two solutions:

  • Create a custom image that contains all the required certificates
  • Use an init container that will update the system's certs storage with additional certificates

This patch implements the latter as it avoids creating new images just for that purpose and the additional certificates can be easily changed using a central storage.

An example for it is using the service to access a JupyterHub instance that is
behind a PKI based certificate.
Copy link
Collaborator

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

Thanks!

I think the indentation needed some tweaks, and that the schema needs to be updated. See https://github.com/yuvipanda/jupyterhub-ssh/blob/main/helm-chart/jupyterhub-ssh/schema.yaml.

helm-chart/jupyterhub-ssh/templates/ssh/deployment.yaml Outdated Show resolved Hide resolved
helm-chart/jupyterhub-ssh/templates/ssh/deployment.yaml Outdated Show resolved Hide resolved
@sgaist
Copy link
Author

sgaist commented Oct 4, 2022

You're welcome !

I added the new fields to the schema but there are several others that are not documented. Since they are unrelated to this change, should they be added in a separate MR ?

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