cert-manager / openshift-routes

OpenShift Route support for cert-manager
Apache License 2.0
46 stars 20 forks source link

`openshift-routes` should create Certificates instead of CertificateRequests #34

Open poponealex opened 1 year ago

poponealex commented 1 year ago

openshift-routes doesn't work as expected and isn't suitable for a production environment with multiple routes using the same host. Indeed, it won't create a Certificate and let cert-manager properly handle the certificate renewal process cluster-wide. Hence, it might make multiple orders for the same hostname which will result in being rate limited by some CA (e.g. Let'sEncrypt).

All of the certificate renewal process should be entirely delegated to cert-manager. However, openshift-routes implements CSR functionalities, certificate validity checking and whatnot. These have nothing to do with populating OpenShift routes. Therefore, this is an issue not only because it's merely the role of the cert-manager operator but also because it induces unexpected behaviours.

Populating OpenShift routes doesn't require any addition to cert-manager. It is the role of cert-manager and cert-manager only to handle all of the certificate renewal process.


To address this issue, I developed a controller that does exactly that: populating OpenShift routes with cert-manager certificates: github.com/its4u/cert-manager-routes-controller.

ctrought commented 1 year ago

I too wondered about the reasoning openshift-routes went around the Certificate API, and instead opt to create CertificateRequests directly. It is a bit more limiting in some regards, like you mentioned sharing certificates is not possible. Perhaps there is an opportunity to refactor openshift-routes to delegate more functionality to cert-manager by using Certificate resources instead, and simply keep track of the secret attached to the Certificate to use in the route?

I see some benefits to the setup you've described colocating all certificates in a shared namespace but also some negatives. For shared certificates in a single namespace on Routes, using Ingress objects would probably be the simplest solution that wouldn't require a secondary controller (ie. just cert-manager) as it would be a single configuration for all path based routes and they'd share a certificate too. But that comes at a cost of fragmentation because users would likely be creating Routes both natively and using Ingress objects, and the whole point of controllers like openshift-routes or cert-manager-routes-controller is to have native support for Routes. Personally we don't have a need for sharing certificates across namespaces and the feature allowing internamespace ownership of route hostnames in openshift is disabled by default as you're probably aware.

Hopefully a maintainer here can chime in and give their thoughts/reasoning for openshift-routes to interact with CertificateRequests directly instead of using Certificates which comes with certain limitations you mentioned.

poponealex commented 1 year ago

Hey @ctrought !

Thank you for your insights.

Perhaps there is an opportunity to refactor openshift-routes to delegate more functionality to cert-manager by using Certificate resources instead, and simply keep track of the secret attached to the Certificate to use in the route?

Yes this is exactly what I think should be done. There should be a clear SOC in between openshift-routes and cert-manager. From my point of view, openshift-routes should create a Certificate, use the TLS from the cert-manager generated Secret and then keep track of changes. I personally did do that in the controller I developed.

I see some benefits to the setup you've described colocating all certificates in a shared namespace but also some negatives.

Since I developed cert-manager-routes-controller, I've also been questioning my implementation and I've opened an issue on the project to implement the use of an Issuer and hence to store the Certificate in the same namespace as the Route. Therefore, ClusterIssuer certificates would be saved in the cert-manager NS and Issuer certificates would be saved in the same NS.

maelvls commented 2 months ago

Thanks, @poponealex, for the detailed feedback. A couple of months ago, I opened https://github.com/cert-manager/openshift-routes/issues/49 to try to gather the issues that people are seeing with the existing CertificateRequest-based behavior, and whether it would make sense to rely on Certificate resources instead.

During one the last dev biweekly meetings, we decided that it would be a good idea to support Certificate resources, and @jacksgt started working on an implementation: https://github.com/cert-manager/openshift-routes/pull/55.

Note: I have renamed your issue to capture its content.