Funcionality : being able to give an already existing secret as SSL

[konfiot]

Hello,
I manage my certificates using an external tool (saltstack) that can put secrets into kubernetes, and I wanted for jupyterhub to be able to use an already-existing secret.
I've implemented this funcionality there, and tested it, it takes a secret name and the name of the certs in the secret
It's like one of my first PR ever, so pleas be kind, I'm open to any suggestion, like where to put the documentation, what to document
Thanks for all the work !

[choldgraf]

hey @konfiot, looks quite interesting! Thanks for the PR! To clarify, this is something people would use instead of the kube-lego letsencrypt stuff, yeah? I'm trying to think about where in the docs it'd exist, for example.

for a review, perhaps @minrk or @yuvipanda has thoughts re: the handling pre-existing SSL certificates? Thanks for being patient and letting us know it's your first PR :-)

[konfiot]

Hmm, it's more instead of the manual setting.
At the moment, if you want to setup a manual certificate, you have to put it in the yaml describing the helm deploy, which is not very convenient if you have an external tool supplying the certificate.
But yes, it also replaces the letsencrypt autogeneration

[minrk]

Can you fill out values.yaml with null values for these?

I'm guessing it should look like:

proxy:
  https:
    type: secret
    secret:
      name: 'my-tls-secret'
      key: 'mydomain.key'
      cert: 'mydomain.crt'

Does that look right?

[konfiot]

Yes, the only thing would be that type: manualsecret, but that's easily transformable to secret, I can do that if you want !

[konfiot]

Just added default values =)

[konfiot]

Any feedback ?

[minrk]

Can we restructure this a tiny bit so it looks like:

type: "secret"
secret:
  name: ""
  keyname: ""
  crtname: ""

so that the secret-related config is under https.secret instead of at the top level and the 'type' key matches the sub-dict ('secret')?

[minrk]

Sorry for the slow turnaround. I made a comment asking for a slight reshaping of the config, but 👍 to the implementation in general.

[konfiot]

Done =)

[konfiot]

Any update ?

[minrk]

@konfiot thanks! I think this is ready to go, I just need to iron out the tests, which shouldn't be related to your changes. Sorry turnaround is very slow in the summer, there are lots of conferences and vacations interrupting things. This should be all set from your end, though.

[konfiot]

Okay, thanks =), when I had published I think the tests passed
No problem, I understand X)

[minrk]

Testing seems to have recovered. Thanks for your patience!

[consideRatio]

This PR wanted to utilize a pre-existing secret containing a key/crt for TLS. This means that this PR should

1. Ensure that we can override the secret name.
2. Ensure we don't create a secret with TLS content any more.

This PR is now overriding the secret name, mounting it etc, but then letting the container utilize the key/crt through command line arguments anyhow. This means that the provided secret mounted is never utilized anyhow.

Note that the secret is made available on line 48-53, and it is mounted to the chp container on line 86.

---

If a secret is to be provided though, it must have the same keys that we have, which isn't obvious though. @konfiot, what is the structure of the pre-existing secret that you wanted to utilize?

Does it happen to be formatted exactly like this, having tls.crt and tls.key as keys for the secrets data dictionary?

data:
  tls.crt: <base-64-encoded cert>
  tls.key: <base-64-encoded key>

[consideRatio]

To summarize @konfiot, if you pass the crt / key file anyhow, you could utilize the manual proxy.https.type, and if you want to utilize a preexisting secret, the following lines should reference the files that the secret has mounted in /etc/chp/tls. You must know the files names though, and that depends on what you have named the keys within your pre-existing secrets data dictionary. Every key in the data dictionary will become a file.