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

Make workspace controller generating TLS certs for its webhook server #16251

Closed
sleshchenko opened this issue Mar 6, 2020 · 5 comments
Closed
Assignees
Labels
engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.

Comments

@sleshchenko
Copy link
Member

Is your enhancement related to a problem? Please describe.

Initial implementation of AdmissionWebhooks for workspace controller was implemented in way, where an administrator generates TLS certs during deploy, but it has cons:

  • it won't work with OLM installation;
  • an administrator must care about TLS and regenerate them when they expire;

Instead, we must make workspace controller generating TLS certs for its webhook server.
So, on each start, it will be able to check TLS certs and generate them if they are missing[1] or regenerate if they are expired[2].
The second part can be moved to scope of a separate issue if it will be clear that it requires quite a lot of time... It makes sense to deliver [1] part where certificates have like a couple of mouths expiration time and deliver [2] later.

Some non-working POC is already implemented in devfile/devworkspace-operator#24
P.S. It would be also cool to investigate if we're able to reuse default cluster CA (probably we won't be able to get CA private part, but it's needed to analyze it)

@sleshchenko sleshchenko added kind/enhancement A feature request - must adhere to the feature request template. engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. team/controller labels Mar 6, 2020
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Mar 6, 2020
@l0rd l0rd mentioned this issue Mar 6, 2020
38 tasks
@tolusha tolusha added severity/P1 Has a major impact to usage or development of the system. and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Mar 6, 2020
@sleshchenko
Copy link
Member Author

sleshchenko commented Mar 17, 2020

Interesting option to go, not sure that it's the right one - since we then would depend on cert manager operator sleshchenko/devworkspace-operator@15f3940

@JPinkney
Copy link
Contributor

After doing some research I think there's only 3 ways we can go about this:

  1. Make Workspace Controller generate TLS certs for webhook server devfile/devworkspace-operator#24
  2. using cert manager
  3. Using Jobs (POC still in progress, but I've at least gotten it far enough where I can see that it will work)

To me I think we can remove 1 was a viable option because the package it relies on is not supported anymore, just early stage experiment. However, I've borrowed some of the ideas and put them into 3.

Pros of 2.

  • Cert manager seems like the "default" way of dealing with certs in kubernetes, or at least there are a lot of articles on it
  • Once it's set up the operator will essentially take care of everything itself AFAIK (renewing certs, etc)

Cons of 2.

  • Need another operator installed which can remove some of the lightweightness of the che-workspace-operator
  • Would need to work with operator versions in case something we use is supported in one version of the operator but not another

Pros of 3

  • Pretty lightweight
  • Everything is done using k8s standard resources
  • Minimal complexity
  • All the code lives in this repo so we can control everything about it

Cons of 3.

  • We need to maintain all the code that will do cert creation/renewal
  • In order to have everything sync up we need to start the operator -> start the job to create certs -> update the deployment of the operator and then mount the secrets -> restart deployment
  • More of a "non-standard" approach

@sleshchenko @amisevsk WDYT?

@amisevsk
Copy link
Contributor

I think it's a bit of a toss-up between 2 and 3. Some thoughts:

  • Option 3 seems to be the best way at the moment, especially since we're at the incubator stage. I think on balance the burden of maintaining the code ourselves would be offset by the overall simplicity.
  • Option 2 is definitely what is suggested for by e.g. the kubebuilder docs, but I agree with the cons listed.
  • An additional con for Option 3, IMO, is what happens for clusters that already have TLS set up, etc.? Would it be possible for an organization to reuse their already-provisioned certs for the webhooks? This seems like something that would be easier with option 2, since cert-manager supports e.g. getting certs from letsecrypt.

I think for now, especially since the work is mostly done, option 3 is the way to go. We can discuss more complicated solutions later, while option 3 allows us to keep working on the numerous actual issues we have already.

@davidfestal
Copy link
Contributor

I'm quite concerned that with option 3 the operator is not self-contained anymore, which might make the installation of the operator through OLM impossible.

@sleshchenko
Copy link
Member Author

The useful links David shared about OLM + Webhook Server:
Support validating admission webhooks operator-framework/operator-sdk#1217
Design proposal Add support for admission webhooks in OLM
https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/contributors/design-proposals/webhooks.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

6 participants