Namespace deletion hangs when containing vault-config-operator custom resources

[davoustp]

Hi,

After #126 was fixed, attempting to clear an entire namespace containing custom resources owned by vault-config-operator is still hanging, leaving the namespace into the Terminatingstate.

The obvious cause is again that finalizers are not removed from the Custom Resources for a different reason: the vault-config-operatorexhibits failures to handle the removal and then fails to clear the finalizers from the Custom Resources.

Removing the Custom Resource's finalizer manually unblocks the garbage collection and the namespace is cleared - leaving relevant content into Vault in the process.

Environment

Kubernetes cluster 1.24.8 (GKE)
vault 1.12.1 (deployed using helm) + fix for #126
vault-config-operator version 0.8.10 (deployed using Helm)

How to reproduce

• Create a namespace containing a RandomSecret

apiVersion: v1
kind: Namespace
metadata:
  name: deletion-test

---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: sa-vault-kv-creator
  namespace: deletion-test

---
apiVersion: redhatcop.redhat.io/v1alpha1
kind: RandomSecret
metadata:
  name: test-secret
  namespace: deletion-test
spec:
  authentication: 
    role: kv-creator
    serviceAccount:
      name: sa-vault-kv-creator
  isKVSecretsEngineV2: true
  path: kv/data/deletion-test
  secretKey: password
  secretFormat:
    passwordPolicyName: alphanum-with-special-chars

After the resources have been created, attempt to remove the entire namespace:

kubectl delete ns deletion-test

=> the operator immediately exhibits errors as shown below (backtraces stripped to ease reading):

1.678099454820571e+09	ERROR	unable to create service account token request	{"controller": "randomsecret", "controllerGroup": "redhatcop.redhat.io", "controllerKind": "RandomSecret", "RandomSecret": {"name":"test-secret","namespace":"deletion-test"}, "namespace": "deletion-test", "name": "test-secret", "reconcileID": "29c428b7-d02f-4cc9-bd65-de19c8fb832b", "in namespace": "deletion-test", "for service account": "sa-vault-kv-creator", "error": "serviceaccounts \"sa-vault-kv-creator\" is forbidden: unable to create new content in namespace deletion-test because it is being terminated"}
github.com/redhat-cop/vault-config-operator/api/v1alpha1/utils.GetJWTTokenWithDuration
	/workspace/api/v1alpha1/utils/commons.go:191
...
1.6780994548207064e+09	ERROR	unable to retrieve jwt token for	{"controller": "randomsecret", "controllerGroup": "redhatcop.redhat.io", "controllerKind": "RandomSecret", "RandomSecret": {"name":"test-secret","namespace":"deletion-test"}, "namespace": "deletion-test", "name": "test-secret", "reconcileID": "29c428b7-d02f-4cc9-bd65-de19c8fb832b", "namespace": "deletion-test", "serviceaccount": "sa-vault-kv-creator", "error": "serviceaccounts \"sa-vault-kv-creator\" is forbidden: unable to create new content in namespace deletion-test because it is being terminated"}
github.com/redhat-cop/vault-config-operator/api/v1alpha1/utils.(*KubeAuthConfiguration).GetVaultClient
	/workspace/api/v1alpha1/utils/commons.go:160
...
1.6780994548207371e+09	ERROR	unable to create vault client	{"controller": "randomsecret", "controllerGroup": "redhatcop.redhat.io", "controllerKind": "RandomSecret", "RandomSecret": {"name":"test-secret","namespace":"deletion-test"}, "namespace": "deletion-test", "name": "test-secret", "reconcileID": "29c428b7-d02f-4cc9-bd65-de19c8fb832b", "KubeAuthConfiguration": {"serviceAccount":{"name":"sa-vault-kv-creator"},"path":"kubernetes","role":"kv-creator"}, "namespace": "deletion-test", "error": "serviceaccounts \"sa-vault-kv-creator\" is forbidden: unable to create new content in namespace deletion-test because it is being terminated"}
github.com/redhat-cop/vault-config-operator/controllers.prepareContext
	/workspace/controllers/commons.go:24
...
1.6780994548207846e+09	ERROR	controllers.RandomSecret	unable to prepare context	{"instance": {"kind":"RandomSecret","apiVersion":...}}, "error": "serviceaccounts \"sa-vault-kv-creator\" is forbidden: unable to create new content in namespace deletion-test because it is being terminated"}
github.com/redhat-cop/vault-config-operator/controllers.(*RandomSecretReconciler).Reconcile
	/workspace/controllers/randomsecret_controller.go:81

Full log attached available on demand if needed.

Analysis

It seems that it's not possible to acquire a Kubernetes JWT token using the service account when the namespace is being deleted (which happens into the prepareContext function, see below), which is needed to run the proper cleanup logic (potentially cleaning content from Vault) and clearing the finalizer.

See

vault-config-operator/api/v1alpha1/utils/commons.go

Line 171 in 18c909d

func GetJWTTokenWithDuration(context context.Context, serviceAccountName string, kubeNamespace string, duration int64) (string, error) {

Unless I'm mistaken, the ServiceAccount resource is most probably already removed from the namespace at this moment, as this is a resource that is not managed by the operator. As a result, when the operator attempts to generate a token using the service account, the API Server ServiceAccount admission controller attempts to create it automatically into the namespace, which cannot happen because of the Terminating state, hence the error.

[raffaelespazzoli]

thanks for the in-depth analysis. This is a race condition that from the point of view of the operator is un-distinguishable from a mis-configuration (i.e. let's say the wrong service account was configured).
So, as far as I can tell, the operator is working as intended. How would you handle this situation?
Maybe we could add a check such that if the namespace is being deleted, then the finalizers should be removed. however that way still leaves some garbage in vault.

[davoustp]

So, as far as I can tell, the operator is working as intended. How would you handle this situation?

I would have the controller indicate to Kubernetes that the ServiceAccount referred to in the Custom Resource is "owned" (but not controlled) by the Custom Resource object.
Theoretically, Kube will handle the dependency graph and remove the Custom Resource(s) before the ServiceAccount, which should give the controller the opportunity to perform the appropriate cleanup in Vault before the ServiceAccount is removed.

See https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/#ownership-and-finalizers for more details.

I'm attempting to prototype this right now. Will keep you posted.

[davoustp]

Update - not working.

First, the ownership stanza does not prevent the service account from being deleted for some reason. Maybe I failed to do it properly, even if verified multiple times.

However, this would not solve the issue anyway: once the namespace enters the Terminating state, there is no way to generate a new JWT token to perform the authentication against Vault (it looks like the token is created / stored into the namespace).
In other words: once the namespace is being deleted, there is no way the operator can interact with Vault for items related to said namespace.

This situation would actually require an alternate, not Kubernetes-based means of authenticating to Vault.

So all that's left is the last resort outcome you that mentioned: since there is no way the cleanup can be performed on Vault for CR objects, there is no point stalling the CRs and namespace removal.
So a Kube-only cleanup (clearing the finalizers, basically) and a WARN message is probably about all that can be done here...

Thoughts?

[eye0fra]

Probably a webook on DELETE operation for namespace resource would help

[davoustp]

@eye0fra how would you see this working (assuming that generating the JWT still works during the hook processing)?
The namespace deletion hook would then look for all operator-managed objects and perform their cleanup logic (which is currently within their own controllers)?

[raffaelespazzoli]

A webhook on the DELETE operation would not work. But a webhook on the UPDATE operation that sets the deletion time on the namespace might allow you to execute some logic while the namespace is still healthy. But what exactly? One might delete all of the vault config resources in the namespace and then return.
This still does not give you the certainty that by the time the operator receives the event the a token can be still created...
Any other ideas?

[davoustp]

I don't have any smarter option here since obtaining a JWT from Kube within a namespace being deleted is not allowed (which makes sense, right?).
So what's the best option to avoid hanging? Ensure that we take the namespace-being-deleted condition into account into each controller and skip the Vault cleanup in this case?

[raffaelespazzoli]

I think so. It would not be a huge change as we centralize that logic.

…

On Wed, Mar 15, 2023 at 10:59 AM Pascal Davoust ***@***.***> wrote: I don't have any smarter option here since obtaining a JWT from Kube within a namespace being delete is not allowed (which makes sense, right?). So what's the best option to avoid hanging? Ensure that we take the namespace-being-delete condition into account into each controller and skip the Vault cleanup in this case? — Reply to this email directly, view it on GitHub <#133 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPERXFPCCQICRE3EHWNYYTW4HKN7ANCNFSM6AAAAAAVQ7UEOE> . You are receiving this because you commented.Message ID: ***@***.***>

-- ciao/bye Raffaele