pkg
pkg copied to clipboard
knative
Supporting reading TLS data from places other than Kubernetes Secrets
When deploying a webhook, it may be desirable to not make use of the certificates controller (for generating a TLS certificate for the webhook), nor reading the TLS data from a Kubernetes secret (e.g. in the case where some form of TLS certificate/identity document is already available within the webhook pod).
It is already possible to not instantiate the certificate controller (by excluding it from the call to sharedmain.WebhookMainWithConfig, however the webhook.Run method explicitly sets the GetCertificate field on the http.Server to one that requires a Secret lister to function: https://github.com/knative/pkg/blob/b0c121fb667f02e7536b09790b2507842b7300dd/webhook/webhook.go#L187-L206
Ideally, this would be one of a number of optional mechanisms for loading TLS certificate data. This would increase the flexibility of the knative.dev/pkg package.
I'm considering whether we should simply allow setting GetCertificate directly, however changes are needed to webhook.New which currently fetches the SecretInformer from the SharedInformerFactory (thereby causing the informer factory to require read permission on secrets to start): https://github.com/knative/pkg/blob/b0c121fb667f02e7536b09790b2507842b7300dd/webhook/webhook.go#L110-L115
I think we could introduce a new NewWithCertificateSource or something, which would allow a user to override this behaviour (and the existing behaviour of the New function could be retained without breaking Go API compatibility).
/kind feature
Ack! I need the very same thing, where my cluster has already mechanisms to come up with certificates that I'd like to mount for the webhook to "just" pick up.
I'd love to see those as commandline options for webhooks generically too, so existing webhooks (like in Knative Serving) can be altered to do things differently.
I think defining an interface to encapsulate the webhook's needs and having something like the certificate controller implement that as one option would be a welcome refactor (I was talking about this a while back with someone 🤔 ). With that, it should be straightforward to add new strategies.
cc @dprotaso for thoughts.
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.