Allow Mutating|ValidatingWebhookConfiguration to use secret for CABundle

[mengqiy]

What would you like to be added:
Allow Mutating|ValidatingWebhookConfiguration to reference content of a secret as CABundle.

The following shows the idea. We can use better names than below.

type WebhookClientConfig struct {
	URL *string `json:"url,omitempty" protobuf:"bytes,3,opt,name=url"`
	Service *ServiceReference `json:"service,omitempty" protobuf:"bytes,1,opt,name=service"`
	CABundle []byte `json:"caBundle,omitempty" protobuf:"bytes,2,opt,name=caBundle"`

	// Optional field that reference a secret.
	CAFromSecret *CASecret
}

type CASecret struct {
	// the namespace of the secret
	Namespace string
	// the name of the secret
	Name string
	// the key in the secret. e.g. ca.key
	KeyName string
}

This can also be applied to CRD conversion webhook

Why is this needed:
It will be very useful if the CA is rotated.
Before: An admin or a controller (with permission to update WebhookConfiguration) need to update CABundle field if the CA has changed.
After: The Webhook configuration will automatically pickup the new CA.

EDIT:
From the security PoV, another advantage is that no need to update the CA means we don't need to grant permissions to some controller for Updating the CABundle field in Mutating|ValidatingWebhookConfig. Current k8s authn doesn't support field-wise fine-grain access control. IOW, a compromised controller can modify the service field of MutatingWebhookConfig to point a random service that may inject an arbitrary container to each pod.

[mengqiy]

/sig api-machinery

[mengqiy]

cc: @liggitt @caesarxuchao

[yue9944882]

isn't configmap a better place to store CA? @deads2k

ref openshift ca rotation: openshift/cluster-kube-apiserver-operator#164

[krmayankk]

putting the caBundle inline in the configuration yaml makes it unreadable
caBundle: <pem encoded ca cert that signs the server cert used by the webhook> . it would be much better to read it from a configmaps

[fedebongio]

/assign @caesarxuchao

[caesarxuchao]

cc @mbohlool @deads2k

[mbohlool]

I think it is a good idea. Ad if we want to do it, it has API implications that I prefer it to be included in v1 API for Admission Webhook GA. @sttts @deads2k @caesarxuchao what do you think?

[liggitt]

I'm on the fence about whether this is desirable or not.

Things I see as pros:

• Update of a CA key in a configmap or secret is way more scoped than giving webhook update permission
• Cert management integrations for kube are more likely to support targeting a configmap or secret
• A CA bundle is more readable in a dedicated configmap
• Pointing several webhook configs at a single configmap/secret object allows for easier rotation

Things I see as cons:

• denormalizing means joining watches from two resource types to control the configured admission plugins
• it means webhook config is no longer self-contained
• granting read permission to the secret/configmap to all the subjects that will be setting up webhook admission calls is not straightforward in the case of extension API servers... the subjects used by extension apiservers to look up the referenced objects are not knowable in advance as the webhook config author, and the secrets/configmaps containing CAs are not knowable in advance as the extension API server deployer.
• introduces additional failure modes (referenced object doesn't exist, cannot be read, etc)

[deads2k]

isn't configmap a better place to store CA? @deads2k

ref openshift ca rotation: openshift/cluster-kube-apiserver-operator#164

When rotating a bundle that is going to be mounted into a pod, it makes a lot of sense to coordinate in configmaps. However, here we're talking about trying to manage a resource that is designed to be watched live for reaction and never mounted. While it is possible to do this with a configmap, you end up having to write an effective join between the two.

In a case where you're reading live object data, I think it's easier to take an approach similar to how the openshift/service-ca injects cabundles into annotated apiservices. The update can take place directly in the resource you're already watching.

[Benjamintf1]

When referencing certificates in the context of a service, a secret is used. Why should this be any different?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[krmayankk]

/remove-lifecycle rotten

[mgoltzsche]

FYI: cert-manager's cainjector solves this by injecting a Certificate into the caBundle field of APIServices or Mutating/ValidatingWebhooks annotated with certmanager.k8s.io/inject-ca-from: <NAMESPACE>/<CERTIFICATE>.

UPDATE: With cert-manager 0.11 it changed to:
cert-manager.io/inject-ca-from: <NAMESPACE>/<CERTIFICATE>

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[pierluigilenoci]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[grosser]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[grosser]

/lifecycle frozen

[grosser]

@liggitt can priority/awaiting-more-evidence be removed ... to me there seems to be no evidence missing ?

[dobesv]

You can workaround this by installing cert-manager's cainjector and setting the appropriate annotation, it will set the caBundle for you from a TLS secret.

[mikhailadvani]

doesn't the configmapRef become more relevant post the 1.20 release of RootCAConfigMap https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.20.md#introducing-rootcaconfigmap