/lgtm

/assign @lavalamp @deads2k
for aggregator package approval

/lgtm
/approve

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavalamp, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Thanks for fixing the bug @liggitt . Does this pass the bar for cherry picking in previous versions?

Does this pass the bar for cherry picking in previous versions?

I wouldn't think so, since the issue can be worked around by including this in your manifest:

caBundle: ""

@liggitt Do I understand correctly that with this PR merged in, an empty caBundle value should be equivalent to a caBundle that contains $(kubectl config view --raw --minify --flatten -o jsonpath='{.clusters[].cluster.certificate-authority-data}') (as seen here)?

If so, how to explain that I end up with errors like x509: certificate signed by unknown authority without caBundle, whereas it works correctly when specified?

@liggitt Do I understand correctly that with this PR merged in, an empty caBundle value should be equivalent to a caBundle that contains $(kubectl config view --raw --minify --flatten -o jsonpath='{.clusters[].cluster.certificate-authority-data}') (as seen here)?

no. an empty caBundle means the apiserver should use the system trust roots to verify the webhook's serving certificate.

From https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.15/#webhookclientconfig-v1beta1-admissionregistration-k8s-io:

caBundle is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. If unspecified, system trust roots on the apiserver are used.

Oh! You mean certificates in /etc/ssl/certs basically, right? Then would you consider it a good practice to add a cluster CA there?

It seems pretty inconvenient to me that a webhook without caBundle can't be called even if it uses certificates that have been signed by the cluster's CSR mechanism itself.

Oh! You mean certificates in /etc/ssl/certs basically, right?

Yeah, or your platform's equivalent

Then would you consider it a good practice to add a cluster CA there?

It could, depending on your deployment. If an API server is running in an image and is the only thing in the image, injecting a cluster CA could make sense.

It seems pretty inconvenient to me that a webhook without caBundle can't be called even if it uses certificates that have been signed by the cluster's CSR mechanism itself.

The CSR API does not guarantee a single signer, and does not guarantee what audiences accept certs obtained via it. See #69836 for a list of items that need further clarification before graduating that API.

Thanks a bunch for the quick answers, it's much more clear now.

One last question: would it make sense to modify the API server so that it internally check webhooks certificates against its own CA, on top of its system trusted roots?

would it make sense to modify the API server so that it internally check webhooks certificates against its own CA

the API server doesn't have a CA... controllers running outside the API server as clients sign the CSR API objects

the API server is given CA bundles for the following things:

• verifying client certificates presented in requests to the API server (--client-ca-file)
• verifying connections to an OIDC server for fetching discovery info (--oidc-ca-file)
• verifying authenticating proxy client certificates presented in requests to the API server (--requestheader-client-ca-file)
• verifying kubelet serving certificates (--kubelet-certificate-authority)

falling back to system roots for verification of webhook serving certificates is more reasonable than using any of those