Create Goroutine to Cleanup Orphaned Shadow Secrets

controllers/hcpvaultsecretsapp_controller.go

@@ -12,6 +12,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	httptransport "github.com/go-openapi/runtime/client"
@@ -24,6 +25,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -32,6 +34,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	"github.com/hashicorp/vault-secrets-operator/credentials"
 	"github.com/hashicorp/vault-secrets-operator/credentials/hcp"
@@ -80,14 +83,16 @@ var (
 // HCPVaultSecretsAppReconciler reconciles a HCPVaultSecretsApp object
 type HCPVaultSecretsAppReconciler struct {
 	client.Client
-	Scheme                      *runtime.Scheme
-	Recorder                    record.EventRecorder
-	SecretDataBuilder           *helpers.SecretDataBuilder
-	HMACValidator               helpers.HMACValidator
-	MinRefreshAfter             time.Duration
-	referenceCache              ResourceReferenceCache
-	GlobalTransformationOptions *helpers.GlobalTransformationOptions
-	BackOffRegistry             *BackOffRegistry
+	Scheme                              *runtime.Scheme
+	Recorder                            record.EventRecorder
+	SecretDataBuilder                   *helpers.SecretDataBuilder
+	HMACValidator                       helpers.HMACValidator
+	MinRefreshAfter                     time.Duration
+	referenceCache                      ResourceReferenceCache
+	GlobalTransformationOptions         *helpers.GlobalTransformationOptions
+	BackOffRegistry                     *BackOffRegistry
+	CleanupOrphanedShadowSecretInterval time.Duration
+	once                                sync.Once
 }
 
 // +kubebuilder:rbac:groups=secrets.hashicorp.com,resources=hcpvaultsecretsapps,verbs=get;list;watch;create;update;patch;delete
@@ -288,6 +293,74 @@ func (r *HCPVaultSecretsAppReconciler) Reconcile(ctx context.Context, req ctrl.R
 	}, nil
 }
 
+func (r *HCPVaultSecretsAppReconciler) startOrphanedShadowSecretCleanup(ctx context.Context) error {
+	var err error
+
+	r.once.Do(func() {
+		for {
+			select {
+			case <-ctx.Done():
+				if ctx.Err() != nil {
+					err = ctx.Err()
+				}
+			// runs the cleanup process once every hour or as specified by the user
+			case <-time.After(r.CleanupOrphanedShadowSecretInterval):
+				err = r.cleanupOrphanedShadowSecrets(ctx)
+			}
+		}
+	})
+
+	return fmt.Errorf("shadow secret cleanup error err=%s", err)
+}
+
+func (r *HCPVaultSecretsAppReconciler) cleanupOrphanedShadowSecrets(ctx context.Context) error {
+	logger := log.FromContext(ctx).WithName("cleanupOrphanedShadowSecrets")
+
+	// filtering only for dynamic secrets
+	dynamicSecretLabelSelector := client.MatchingLabels{"app.kubernetes.io/component": "hvs-dynamic-secret-cache"}
+
+	secrets := corev1.SecretList{}
+	if err := r.List(ctx, &secrets, client.InNamespace(common.OperatorNamespace), dynamicSecretLabelSelector); err != nil {
+		logger.Error(err, "Failed to list shadow secrets")
+		return err
+	}
+
+	for _, secret := range secrets.Items {
+		o := &secretsv1beta1.HCPVaultSecretsApp{}
+
+		namespace := secret.Labels[hvsaLabelPrefix+"/namespace"]
+		name := secret.Labels[hvsaLabelPrefix+"/name"]
+		namespacedName := types.NamespacedName{Namespace: namespace, Name: name}
+
+		// get the HCPVaultSecretsApp instance that that the shadow secret belongs to (if applicable)
+		// no errors are returned in the loop because this could block the deletion of other
+		// orphaned shadow secrets that are further along in the list
+		err := r.Get(ctx, namespacedName, o)
+		if err != nil && !apierrors.IsNotFound(err) {
+			logger.Error(err, "Error getting resource from k8s", "secret", secret.Name)
+			continue
+		}
+
+		// 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]) {
+			if err := r.handleDeletion(ctx, o); err != nil {
+				logger.Error(err, "Failed to handle deletion of HCPVaultSecretsApp", "app", o.Name)
+			}
+
+			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, namespacedName); err != nil {
+				logger.Error(err, "Failed to delete shadow secret", "secret", secret.Name)
+			}
+
+			logger.Info("Deleted orphaned shadow secret", "secret", secret.Name)
+		}
+	}
+
+	return nil
+}
+
 func (r *HCPVaultSecretsAppReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.HCPVaultSecretsApp) error {
 	o.Status.LastGeneration = o.GetGeneration()
 	if err := r.Status().Update(ctx, o); err != nil {
@@ -306,6 +379,11 @@ func (r *HCPVaultSecretsAppReconciler) SetupWithManager(mgr ctrl.Manager, opts c
 		r.BackOffRegistry = NewBackOffRegistry()
 	}
 
+	mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {
+		err := r.startOrphanedShadowSecretCleanup(ctx)
+		return err
+	}))
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&secretsv1beta1.HCPVaultSecretsApp{}).
 		WithEventFilter(syncableSecretPredicate(nil)).
@@ -371,7 +449,7 @@ func (r *HCPVaultSecretsAppReconciler) hvsClient(ctx context.Context, o *secrets
 	return hvsclient.New(cl, nil), nil
 }
 
-func (r *HCPVaultSecretsAppReconciler) handleDeletion(ctx context.Context, o client.Object) error {
+func (r *HCPVaultSecretsAppReconciler) handleDeletion(ctx context.Context, o *secretsv1beta1.HCPVaultSecretsApp) error {
 	logger := log.FromContext(ctx)
 	objKey := client.ObjectKeyFromObject(o)
 	r.referenceCache.Remove(SecretTransformation, objKey)

controllers/hcpvaultsecretsapp_controller_test.go

@@ -18,12 +18,15 @@ import (
 	"github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/preview/2023-11-28/models"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1"
 	"github.com/hashicorp/vault-secrets-operator/common"
 	"github.com/hashicorp/vault-secrets-operator/helpers"
+	"github.com/hashicorp/vault-secrets-operator/internal/testutils"
 )
 
 var _ runtime.ClientTransport = (*fakeHVSTransport)(nil)
@@ -1224,3 +1227,159 @@ func Test_makeShadowObjKey(t *testing.T) {
 		})
 	}
 }
+
+func Test_CleanupOrphanedShadowSecrets(t *testing.T) {
+	deletionTimestamp := metav1.Now()
+
+	tests := map[string]struct {
+		o                                    *secretsv1beta1.HCPVaultSecretsApp
+		secret                               *corev1.Secret
+		isHCPVaultSecretsAppDeletionExpected bool
+		isShadowSecretDeletionExpected       bool
+	}{
+		"deleted-secret-hvsapp-owner": {
+			o: &secretsv1beta1.HCPVaultSecretsApp{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       HCPVaultSecretsApp.String(),
+					APIVersion: secretsv1beta1.GroupVersion.Version,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					UID:        "hvsApp1UID",
+					Namespace:  "hvsApp1Namespace",
+					Name:       "hvsApp1",
+					Finalizers: []string{hcpVaultSecretsAppFinalizer},
+				},
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:         common.OperatorNamespace,
+					Name:              "shadowSecret1",
+					DeletionTimestamp: &deletionTimestamp,
+					Finalizers:        []string{vaultDynamicSecretFinalizer},
+					Labels: map[string]string{
+						hvsaLabelPrefix + "/namespace": "hvsApp1Namespace",
+						hvsaLabelPrefix + "/name":      "hvsApp1",
+						"app.kubernetes.io/component":  "hvs-dynamic-secret-cache",
+						helpers.LabelOwnerRefUID:       "hvsApp1UID",
+					},
+				},
+			},
+			isHCPVaultSecretsAppDeletionExpected: true,
+			isShadowSecretDeletionExpected:       true,
+		},
+		"deleted-secret-hvsapp-not-owner": {
+			o: &secretsv1beta1.HCPVaultSecretsApp{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       HCPVaultSecretsApp.String(),
+					APIVersion: secretsv1beta1.GroupVersion.Version,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					UID:        "hvsApp2UID",
+					Namespace:  "hvsApp2Namespace",
+					Name:       "hvsApp2",
+					Finalizers: []string{hcpVaultSecretsAppFinalizer},
+				},
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:         common.OperatorNamespace,
+					Name:              "shadowSecret2",
+					DeletionTimestamp: &deletionTimestamp,
+					Finalizers:        []string{vaultDynamicSecretFinalizer},
+					Labels: map[string]string{
+						"app.kubernetes.io/component": "hvs-dynamic-secret-cache",
+					},
+				},
+			},
+			isShadowSecretDeletionExpected: true,
+		},
+		"deleted-secret-hvsapp-not-found": {
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: common.OperatorNamespace,
+					Name:      "shadowSecret3",
+					Labels: map[string]string{
+						"app.kubernetes.io/component": "hvs-dynamic-secret-cache",
+					},
+					DeletionTimestamp: &deletionTimestamp,
+					Finalizers:        []string{hcpVaultSecretsAppFinalizer},
+				},
+			},
+			isShadowSecretDeletionExpected: true,
+		},
+		"secret-not-dynamic": {
+			o: &secretsv1beta1.HCPVaultSecretsApp{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       HCPVaultSecretsApp.String(),
+					APIVersion: secretsv1beta1.GroupVersion.Version,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					UID:        "hvsApp4UID",
+					Namespace:  "hvsApp4Namespace",
+					Name:       "hvsApp4",
+					Finalizers: []string{hcpVaultSecretsAppFinalizer},
+				},
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:         common.OperatorNamespace,
+					Name:              "nonShadowSecret",
+					DeletionTimestamp: &deletionTimestamp,
+					Finalizers:        []string{vaultDynamicSecretFinalizer},
+					Labels: map[string]string{
+						hvsaLabelPrefix + "/namespace": "hvsApp4Namespace",
+						hvsaLabelPrefix + "/name":      "hvsApp4",
+						helpers.LabelOwnerRefUID:       "hvsApp4UID",
+					},
+				},
+			},
+			isShadowSecretDeletionExpected: false,
+		},
+	}
+
+	ctx := context.Background()
+	clientBuilder := testutils.NewFakeClientBuilder().Build()
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			r := &HCPVaultSecretsAppReconciler{
+				Client:          clientBuilder,
+				BackOffRegistry: NewBackOffRegistry(),
+				referenceCache:  newResourceReferenceCache(),
+			}
+
+			// create the HCPVaultSecretsApp if the test case has one
+			if tt.o != nil {
+				assert.NoError(t, clientBuilder.Create(ctx, tt.o))
+			}
+
+			// create the secret for the test case
+			assert.NoError(t, clientBuilder.Create(ctx, tt.secret))
+
+			// DeleteTimestamp is a read-only field, so Delete will need to be called to
+			// simulate deletion of the HCPVaultSecretsApp
+			if tt.isHCPVaultSecretsAppDeletionExpected {
+				assert.NoError(t, clientBuilder.Delete(ctx, tt.o))
+			}
+
+			err := r.cleanupOrphanedShadowSecrets(ctx)
+			assert.NoError(t, err)
+
+			if tt.isHCPVaultSecretsAppDeletionExpected {
+				deletedHVSApp := &secretsv1beta1.HCPVaultSecretsApp{}
+				err = r.Get(ctx, client.ObjectKeyFromObject(tt.o), deletedHVSApp)
+				assert.True(t, apierrors.IsNotFound(err))
+			}
+
+			if tt.isShadowSecretDeletionExpected {
+				deletedSecret := &corev1.Secret{}
+				err = r.Get(ctx, makeShadowObjKey(tt.secret), deletedSecret)
+				assert.True(t, apierrors.IsNotFound(err))
+			} else {
+				secret := &corev1.Secret{}
+				err := r.Get(ctx, client.ObjectKeyFromObject(tt.secret), secret)
+				assert.False(t, apierrors.IsNotFound(err))
+			}
+		})
+	}
+}

helpers/secrets.go

@@ -41,7 +41,7 @@ var SecretDataErrorContainsRaw = fmt.Errorf("key '%s' not permitted in Secret da
 // 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.
-var labelOwnerRefUID = fmt.Sprintf("%s/vso-ownerRefUID", secretsv1beta1.GroupVersion.Group)
+var LabelOwnerRefUID = fmt.Sprintf("%s/vso-ownerRefUID", secretsv1beta1.GroupVersion.Group)
 
 // OwnerLabels will be applied to any k8s secret we create. They are used in Secret ownership checks.
 // There are similar labels in the vault package. It's important that component secret's value never
@@ -66,7 +66,7 @@ func OwnerLabelsForObj(obj ctrlclient.Object) (map[string]string, error) {
 	for k, v := range OwnerLabels {
 		l[k] = v
 	}
-	l[labelOwnerRefUID] = uid
+	l[LabelOwnerRefUID] = uid
 
 	return l, nil
 }
@@ -84,7 +84,7 @@ func matchingLabelsForObj(obj ctrlclient.Object) (ctrlclient.MatchingLabels, err
 	for k, v := range l {
 		m[k] = v
 	}
-	m[labelOwnerRefUID] = string(obj.GetUID())
+	m[LabelOwnerRefUID] = string(obj.GetUID())
 
 	return m, nil
 }

helpers/secrets_test.go

@@ -52,7 +52,7 @@ func TestFindSecretsOwnedByObj(t *testing.T) {
 	for k, v := range OwnerLabels {
 		ownerLabels[k] = v
 	}
-	ownerLabels[labelOwnerRefUID] = string(owner.GetUID())
+	ownerLabels[LabelOwnerRefUID] = string(owner.GetUID())
 
 	notOwner := &secretsv1beta1.VaultDynamicSecret{
 		TypeMeta: metav1.TypeMeta{
@@ -489,7 +489,7 @@ func TestSyncSecret(t *testing.T) {
 			}
 
 			if tt.obj.GetUID() != "" {
-				ownerLabels[labelOwnerRefUID] = string(tt.obj.GetUID())
+				ownerLabels[LabelOwnerRefUID] = string(tt.obj.GetUID())
 			}
 
 			var orphans []ctrlclient.ObjectKey

internal/options/env.go

@@ -49,6 +49,9 @@ type VSOEnvOptions struct {
 
 	// ClientCacheNumLocks is VSO_CLIENT_CACHE_NUM_LOCKS environment variable option
 	ClientCacheNumLocks *int `split_words:"true"`
+
+	// CleanupOrphanedShadowSecretInterval is VSO_CLEANUP_ORPHANED_SHADOW_SECRETS_INTERVAL environment variable option
+	CleanupOrphanedShadowSecretInterval time.Duration `split_words:"true"`
 }
 
 // Parse environment variable options, prefixed with "VSO_"