Add webhook certificate handling for k8s #114
Conversation
|
/cc @martinkennelly |
| For example, given `cacert.pem`, `key.pem` and `cert.pem`: | ||
| ```bash | ||
| kubectl -n sriov-network-operator create secret tls operator-webhook-service --cert=cert.pem --key=key.pem | ||
| kubectl -n sriov-network-operator create secret tls network-resources-injector-secret --cert=cert.pem --key=key.pem |
There was a problem hiding this comment.
To be able to inject the secret, do we need this annotation in the secret?:
cert-manager.io/allow-direct-injection: "true"
There was a problem hiding this comment.
So this option is assuming the users are creating the certificates manually and cert-manager is not being used.
| service.beta.openshift.io/inject-cabundle: "true" | ||
| {{else if and (not .CaBundle) (eq .ClusterType "kubernetes") }} | ||
| cert-manager.io/inject-ca-from: {{.Namespace}}/operator-webhook-service |
There was a problem hiding this comment.
I read the docs on cert manager and I am wondering, why not use this?
cert-manager.io/inject-ca-from-secret
There was a problem hiding this comment.
That requires creating the secrets manually with a specific annotation which would be an actual extra step on all the procedure. It's is primarily used when bootstrapping cert-manager itself. After that, cert-manager creates the secrets from certificates for you so all you can directly annotate the certificate and save that extra step.
| And then deploy the operator: | ||
| ```bash | ||
| export ENABLE_ADMISSION_CONTROLLER=true | ||
| make deploy-setup-k8s |
There was a problem hiding this comment.
Warning FailedMount 40s kubelet Unable to attach or mount volumes: unmounted volumes=[tls], unattached volumes=[tls network-resources-injector-sa-token-zwls5]: timed out waiting for the condition
Warning FailedMount 35s (x9 over 2m43s) kubelet MountVolume.SetUp failed for volume "tls" : secret "network-resources-injector-secret" not found
Got the following error when tried this method on a fresh k8 cluster. I made sure WEBHOOK_CA_BUNDLEenv var wasnt exported.
There was a problem hiding this comment.
Strange, this is working for me.
The secret network-resources-injector-secret should be created in namespace sriov-network-operator from the Certificate network-resources-injector-service if cert-manager is functioning correctly. Can you describe the Certificate and check what's its status? It is also strange that it happens for one of the webhooks and not the other. I wiull try it again on my side.
There was a problem hiding this comment.
Tried again and working for me.
There was a problem hiding this comment.
It's working for me too on a fresh k8s install
There was a problem hiding this comment.
worked on a fresh install. Thanks!
jcaamano
force-pushed
the
webhook-cert-k8s
branch
from
ca5ee80 to
ac25254
Compare
| And then deploy the operator: | ||
| ```bash | ||
| export ENABLE_ADMISSION_CONTROLLER=true | ||
| make deploy-setup-k8s |
There was a problem hiding this comment.
It's working for me too on a fresh k8s install
jcaamano
force-pushed
the
webhook-cert-k8s
branch
from
ac25254 to
e823c26
Compare
| @@ -14,6 +18,9 @@ webhooks: | |||
| name: operator-webhook-service | |||
| namespace: {{.Namespace}} | |||
| path: "/mutating-custom-resource" | |||
| {{if .CaBundle}} | |||
There was a problem hiding this comment.
Could we check the "ClusterType" to be equals to "kubernetes" as well? Just in case WEBHOOK_CA_BUNDLE is accidentally set in "openshift" environment.
There was a problem hiding this comment.
This would apply to other manifests in bindata.
There was a problem hiding this comment.
The idea I had here is that passing the CaBundle would also be an option in Openshift. Thinking on situations where one would have a quick test or dev cluster where the ca operator is not deployed. This might not make sense though as I am not familiar with Openshift. If it does not, I can add the check for cluster type there as you mention. Let me know.
There was a problem hiding this comment.
@jcaamano thanks for all the consideration!
We didn't do ca injection manually in openshift afaik, I think we could add it only for k8s at this moment.
If anything changes, we could remove the conditional check.
There was a problem hiding this comment.
@zshi-redhat I rewrote the conditions so that CaBundle is only considered for ClusterType=kubernetes. With ClusterType=openshift, CaBundle is ignored.
|
/cc @pliurh |
jcaamano
force-pushed
the
webhook-cert-k8s
branch
from
e823c26 to
b91700b
Compare
Adds support for webhooks in k8s deployments through options to handle webhoook certificate configuration, either via providing the CA bundle through environment variable or cert-manager annotations. Signed-off-by: Jaime Caamaño Ruiz <[email protected]>
jcaamano
force-pushed
the
webhook-cert-k8s
branch
from
b91700b to
9ef5552
Compare
Adds support for webhooks in k8s deployments through options to handle
webhoook certificate configuration, either via providing the CA bundle
through environment variable or cert-manager annotations.