Allow limiting adding/removing finalizers

[liggitt]

/kind feature
@kubernetes/sig-auth-feature-requests
@kubernetes/sig-api-machinery-feature-requests

What happened:

As a namespace-constrained user, I am able to manually add/remove finalizers added by system components:

• garbage collection finalizers
• pv/pvc protection finalizers
• service catalog deprovisioning finalizers
• etc...

What you expected to happen:

As a cluster admin, I expected to be able to control what finalizers can be added/removed by end users, so they can be relied on by system components and controllers for gating deletion

[mikedanese]

@ihmccreery

[hzxuzhonghu]

/cc

[ikehz]

+1

I've noticed this deficiency as well. More specifics:

The finalize command as it exists today takes a complete namespace object and, as far as I can tell, removes any finalizers from the API object that don't appear in the request object.

This makes fine-grained authorization impossible, which is especially important for namespace-constrained users. I want to write an authorization rules like: "only service account Y can modify finalizer X (on all namespaces)" (it might also be nice to specify "user Z can modify all finalizers on their namespace except X or W" but that seems like a nice-to-have for which I'm not sure there are many use cases).

Additionally to the multi-tenancy concerns above, bugs are just easy to write and hard to protect against right now. If I write a namespace controller that accidentally deletes all finalizers, instead of just the one I intend to modify, that's a large blast radius.

My first pass at this would be to define finalizers as first-class subresources on a namespace?

Xref https://github.com/kubernetes/community/blob/master/contributors/design-proposals/architecture/namespaces.md#finalizers.

[ikehz]

I think we could do this in a backward-compatible way, even, by making the current finalize call an alias for the subresource-based, correctly authorized call, where the apiserver computes what finalizers are being deleted and delegates to the subresource endpoint.

[mbohlool]

/cc @jennybuckley

[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]

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

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[smarterclayton]

I’d like to escalate this. Abuse of finalizer deletion is widespread, and people are doing terrible things because we aren’t adequately communicating the risks of these.

In particular the PVC finalizer abuse has reached new heights - on that one there are other things to investigate (like whether the controller is giving feedback to users appropriately).

I’d like to (as part of this) make sure finalizer owners are living up to their responsibility to explain to humans why they aren’t allowing a deletion.

[smarterclayton]

I’d be happy with an admission control check.

Also we should note an explicit difference between namespace deletion and other types of finalizers.

[zhouhaibing089]

We've been thinking about this, too. Besides finalizers, we also want to take care of annotations and init containers(where those annotations and init containers are added by third-party systems).

Basically, we are trying to setup new RBAC clusterroles for finalizers, and checking authorization during admission(just like PodSecurityPolicy checks the use permission on podsecuritypolicies).

[zhouhaibing089]

Does this require a KEP? I am interested to work on this.

[lavalamp]

If you want to change core kubernetes for this, then yes, it needs a KEP.

If you want to write a general admission controller that solves the problem, I don't think you need a KEP. I'm not 100% sure off the top of my head that we pass enough information about the user to the webhook admission controllers; so you may end up asking for more features there.

[zhouhaibing089]

I think a general admission + RBAC should suffice:

Briefly, we can define a new verb finalize, cluster administrators can create proper RBAC rules for it, for example, we can give GC controller permissions to remove finalizers orphan:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: orphan-finalizer
rules:
- apiGroups:
  - "*"
  resources:
  - "*"
  resourceNames:
  - "orphan"
  verbs:
  - "finalize"
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: orphan-finalizer
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: orphan-finalizer
subjects:
- kind: ServiceAccount
  name: generic-garbage-collector
  namespace: kube-system

During admission time, we get the user info, and figure out:

1. All the new added finalizers.
2. Finalizers which are removed.

We send a call to authorizer to check whether that user can do the finalization of the finalizer.

Can I get some feedback here?

[zhouhaibing089]

Proposing this: #76213.

cc @liggitt @lavalamp