Create Goroutine to Cleanup Orphaned Shadow Secrets #980
Conversation
… the secret to the hvsapp instead of the other way around in the original
…et if app is not found or if it's owner ref id does not match the apps uid
Co-authored-by: Theron Voran <[email protected]>
…e-cleanup-shadow-secrets' into jaireddjawed-feature-cleanup-shadow-secrets
…t be deleted if error occurs
| } | ||
|
|
||
| // if the HCPVaultSecretsApp has been deleted, and the shadow secret belongs to it, delete both | ||
| if o.GetDeletionTimestamp() != nil && o.GetUID() == types.UID(secret.Labels[helpers.LabelOwnerRefUID]) { |
There was a problem hiding this comment.
What if secret labels is missing the UID key?
There was a problem hiding this comment.
Assumed that it always would be included. Will update the if statement to protect from a nil pointer error.
There was a problem hiding this comment.
I don't think that would cause a nil pointer panic, I think it would just be like comparing something to an empty string
There was a problem hiding this comment.
I checked and the definition of labelOwnerRefID states that it cannot be blank on a secret. I think we are good to leave this as is?
labelOwnerRefUID is used as the primary key when listing the Secrets owned by a specific VSO object. It should be included in every Secret that is created by VSO.
| logger.Info("Deleted orphaned resources associated with HCPVaultSecretsApp", "app", o.Name) | ||
| } else if apierrors.IsNotFound(err) || secret.GetDeletionTimestamp() != nil { | ||
| // otherwise, delete the single shadow secret if it has a deletion timestamp | ||
| if err := helpers.DeleteSecret(ctx, r.Client, objKey); err != nil { |
There was a problem hiding this comment.
I think objKey is going to be the HCPVaultSecretsApp here; should be the secret instead?
| Labels: map[string]string{ | ||
| "app.kubernetes.io/component": "hvs-dynamic-secret-cache", | ||
| }, | ||
| DeletionTimestamp: &deletionTimestamp, |
There was a problem hiding this comment.
We should also add a similar test case where DeletionTimestamp is not set.
|
|
||
| if tt.isShadowSecretDeletionExpected { | ||
| deletedSecret := &corev1.Secret{} | ||
| err := r.Get(ctx, makeShadowObjKey(tt.secret), deletedSecret) |
There was a problem hiding this comment.
Seems like this should be ctrlclient.ObjectKeyFromObject(tt.secret)? Otherwise it's not going to fetch the secret in the test case. And could probably just do this Get() once then do the if/else.
Description
When an
HCPVaultSecretsAppis deleted, thehandleDeletion()method is called to remove the app's shadow secrets from k8s. However, ifhandleDeletion()fails to remove the secrets for some reason, the orphaned shadow secrets remain in k8s indefinitely because we don't have a mechanism that attempts to remove these shadow secrets again later.This PR addresses this issue by creating a goroutine that periodically checks for deleted HVS apps and removes the app's shadow secrets.
Local Testing
Reconcile()This is to mock the scenario where the firsthandleDeletion()call fails (mentioned here).kubectl get secrets -o yaml)kubectl delete hcpvaultsecretsapps.secrets.hashicorp.com web-application)kubectl get secrets -o yaml)Jira Ticket
https://hashicorp.atlassian.net/browse/VAULT-31820