open-policy-agent / kube-mgmt

Sidecar for managing OPA instances in Kubernetes.
Apache License 2.0
239 stars 106 forks source link

opa-kube-mgmt Helm Chart config can't use existing Cert-Manager Issuer or an existing Secret created from Cert-Manager #208

Closed joshuagrisham-karolinska closed 1 year ago

joshuagrisham-karolinska commented 1 year ago

When using the opa-kube-mgmt Helm Chart version 8.1.1 it is not possible to configure it to use an existing Issuer or ClusterIssuer (instead what I see is that the template is creating its own self-signed issuer if you turn on certManager.enabled).

We would rather either

  1. a Cert-Manager Certificate resource is created where we can specify the issuerRef using values (and that a new issuer is not created), or
  2. the ability to use an existing Secret which has the keys ca.crt, tls.crt, and tls.key (basically that was generated by Cert-Manager or has the same keys)

With # 1 there are still some limitations on the Certificate that is generated (for example if we wanted specific attributes in the certificate? etc), # 2 gives more flexibility but there is risk with the CN etc just like is written in the comments if you supply your own text-based certificate to .CA, .cert, etc values.

eshepelyuk commented 1 year ago

Hello @joshuagrisham-karolinska

I am asuming that you are describing admission webhook tls configuration, am I right ?

joshuagrisham-karolinska commented 1 year ago

Hi @eshepelyuk well, half yes and half no..

Much of what happens with the TLS certs is in the webhookconfiguration.yaml template of the Helm Chart, even though it is not strictly just webhook stuff.

What I am talking about here is the server certificate for the OPA Server service itself. Specifically what gets picked up here as tls.crt and tls.key: https://github.com/open-policy-agent/kube-mgmt/blob/f23b3b0e3d3bea25eee6dd2b107f864ad432dadd/charts/opa-kube-mgmt/templates/deployment.yaml#L115-L118

This is pointing to some files mounted in via a secret here: https://github.com/open-policy-agent/kube-mgmt/blob/f23b3b0e3d3bea25eee6dd2b107f864ad432dadd/charts/opa-kube-mgmt/templates/deployment.yaml#L229-L233

And this secret is generated in 3 different ways from what I can see (and this is where we get back to webhookconfiguration.yaml) ...

  1. "default" is self-generating a CA and a self-signed cert using Helm functions like genCA and then putting them into a static secret which is then used (so the certs are created are completely new, dynamic, and isolated, only created once at essentially "deploy" time), see: https://github.com/open-policy-agent/kube-mgmt/blob/f23b3b0e3d3bea25eee6dd2b107f864ad432dadd/charts/opa-kube-mgmt/templates/webhookconfiguration.yaml#L135-L141
  2. The user can also provide a full string of their certificate and key as Values properties (.Values.cert and .Values.key) and they are just inserted directly into the same secret without using the new self-signed generated certs
  3. The user opts to use Cert-Manager, but the only way this seems to happen is basically that it will create its own brand new Issuer and then a new Certificate using that new Issuer that writes its resulting certificate to the same aforementioned secret.

The problem with both # 1 and # 3 is that now it is difficult to have a trust relationship between this service and all of our other services -- this is a brand new certificate issuer we have never seen before and we will have to jump through hoops in order to try to automate some kind of flow that all of our other services can trust this? It feels a lot cleaner if we can actually control this part.

And # 2 is potentially a way out of this problem, but now we are dealing with a static string that we must write into the values file either by somehow setting or injecting it at deploy-time or checking our private certificates in full into Github, etc? Also not super easy to work with unfortunately.

So that leads me to this specifically issue report -- it would be amazing if we could instead either just point the config to our own existing Cert-Manager Issuer (via the issuerRef of the Certificate resource) or that we could just create our own Secret on the side, however we want (e.g. we can use Cert-Manager to generate the Secret, or something else entirely like HashiCorp Vault to manage certificates and then create them as Secrets which could be used here instead?)

Make more sense, I hope? 😸

eshepelyuk commented 1 year ago

Hi, yes i is much clearer now.

So my 5 cents. I came as a maintainer of this chart last year and I tried not to change much, but I consider that TLS interception by a pod itself is kind of of a antipattern because violation of separation of concern principle.

TLS interception should be handled

Although OPA has TLS interception posibility - its more for standalone cases when running outside k8s, in k8s it must be disabled imo.

SO my take on this issue is that TLS interception for OPA container should be removed from this chart.

eshepelyuk commented 1 year ago

@joshuagrisham-karolinska any feedback on this ?