From f65652bfc322f43477b4b6f325874ba7e7e66cbc Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Mon, 15 Jun 2026 11:32:45 -0700 Subject: [PATCH] Add secret copy utilities for backup PVC provisioning Add CopySecret, DeleteSecretIfAny, and DeleteSecretsWithLabel utilities for copying namespace-scoped secrets to the Velero namespace during datamover backup PVC creation. CopySecret handles three cases: - Secret does not exist in target: copies it with a tracking label - Secret exists with same data: no-op (same source namespace) - Secret exists with different data: returns ErrSecretCollision so the caller can requeue Signed-off-by: Shubham Pampattiwar --- pkg/util/kube/secrets.go | 88 ++++++++++++++ pkg/util/kube/secrets_copy_test.go | 186 +++++++++++++++++++++++++++++ 2 files changed, 274 insertions(+) create mode 100644 pkg/util/kube/secrets_copy_test.go diff --git a/pkg/util/kube/secrets.go b/pkg/util/kube/secrets.go index f1d19b84e..170f1c49d 100644 --- a/pkg/util/kube/secrets.go +++ b/pkg/util/kube/secrets.go @@ -18,9 +18,14 @@ package kube import ( "context" + "reflect" "github.com/cockroachdb/errors" + "github.com/sirupsen/logrus" corev1api "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" kbclient "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -49,3 +54,86 @@ func GetSecretKey(client kbclient.Client, namespace string, selector *corev1api. return key, nil } + +const ( + // BackupPVCSecretLabel is the label applied to secrets copied to the Velero namespace + // for backup PVC provisioning. The value is the owning DataUpload name. + BackupPVCSecretLabel = "velero.io/backup-pvc-secret" +) + +// ErrSecretCollision is returned when a secret with the same name but different data +// already exists in the target namespace, indicating another DataUpload is using it. +var ErrSecretCollision = errors.New("secret collision: same name exists with different data") + +// CopySecret copies a secret from sourceNamespace to targetNamespace. +// If a secret with the same name already exists in the target with identical data, it is a no-op. +// If a secret with the same name exists with different data (collision from another DataUpload), +// it returns ErrSecretCollision so the caller can requeue. +func CopySecret(ctx context.Context, client corev1client.CoreV1Interface, secretName, sourceNamespace, targetNamespace string, ownerName string, log logrus.FieldLogger) error { + srcSecret, err := client.Secrets(sourceNamespace).Get(ctx, secretName, metav1.GetOptions{}) + if err != nil { + return errors.Wrapf(err, "error getting secret %s/%s", sourceNamespace, secretName) + } + + newSecret := &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: targetNamespace, + Labels: map[string]string{ + BackupPVCSecretLabel: ownerName, + }, + }, + Type: srcSecret.Type, + Data: srcSecret.Data, + } + + _, err = client.Secrets(targetNamespace).Create(ctx, newSecret, metav1.CreateOptions{}) + if err == nil { + log.Infof("Copied secret %s from %s to %s", secretName, sourceNamespace, targetNamespace) + return nil + } + + if !apierrors.IsAlreadyExists(err) { + return errors.Wrapf(err, "error creating secret %s in %s", secretName, targetNamespace) + } + + existing, err := client.Secrets(targetNamespace).Get(ctx, secretName, metav1.GetOptions{}) + if err != nil { + return errors.Wrapf(err, "error getting existing secret %s/%s", targetNamespace, secretName) + } + + if reflect.DeepEqual(existing.Data, srcSecret.Data) { + log.Infof("Secret %s already exists in %s with same data, skipping copy", secretName, targetNamespace) + return nil + } + + log.Infof("Secret %s already exists in %s with different data, collision detected", secretName, targetNamespace) + return ErrSecretCollision +} + +// DeleteSecretIfAny deletes a secret if it exists, logging but not returning errors. +func DeleteSecretIfAny(ctx context.Context, client corev1client.CoreV1Interface, secretName, namespace string, log logrus.FieldLogger) { + err := client.Secrets(namespace).Delete(ctx, secretName, metav1.DeleteOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + log.Debugf("Secret %s/%s not found, skipping delete", namespace, secretName) + } else { + log.WithError(err).Errorf("Failed to delete secret %s/%s", namespace, secretName) + } + } +} + +// DeleteSecretsWithLabel deletes all secrets in a namespace matching a label key=value pair. +func DeleteSecretsWithLabel(ctx context.Context, client corev1client.CoreV1Interface, namespace, labelKey, labelValue string, log logrus.FieldLogger) { + secrets, err := client.Secrets(namespace).List(ctx, metav1.ListOptions{ + LabelSelector: labelKey + "=" + labelValue, + }) + if err != nil { + log.WithError(err).Errorf("Failed to list secrets with label %s=%s in %s", labelKey, labelValue, namespace) + return + } + + for i := range secrets.Items { + DeleteSecretIfAny(ctx, client, secrets.Items[i].Name, namespace, log) + } +} diff --git a/pkg/util/kube/secrets_copy_test.go b/pkg/util/kube/secrets_copy_test.go new file mode 100644 index 000000000..805fe1b59 --- /dev/null +++ b/pkg/util/kube/secrets_copy_test.go @@ -0,0 +1,186 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "context" + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1api "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + k8sruntime "k8s.io/apimachinery/pkg/runtime" +) + +func TestCopySecret(t *testing.T) { + log := logrus.New() + + tests := []struct { + name string + secretName string + sourceNS string + targetNS string + ownerName string + objects []k8sruntime.Object + expectErr bool + errContains string + }{ + { + name: "successfully copies secret to target namespace", + secretName: "ceph-csi-kms-token", + sourceNS: "app-ns", + targetNS: "velero", + ownerName: "du-123", + objects: []k8sruntime.Object{ + &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "ceph-csi-kms-token", Namespace: "app-ns"}, + Data: map[string][]byte{"token": []byte("vault-token-a")}, + Type: corev1api.SecretTypeOpaque, + }, + }, + }, + { + name: "returns error when source secret does not exist", + secretName: "missing-secret", + sourceNS: "app-ns", + targetNS: "velero", + ownerName: "du-123", + objects: []k8sruntime.Object{}, + expectErr: true, + errContains: "error getting secret", + }, + { + name: "no-op when target already has secret with same data", + secretName: "ceph-csi-kms-token", + sourceNS: "app-ns", + targetNS: "velero", + ownerName: "du-123", + objects: []k8sruntime.Object{ + &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "ceph-csi-kms-token", Namespace: "app-ns"}, + Data: map[string][]byte{"token": []byte("same-token")}, + Type: corev1api.SecretTypeOpaque, + }, + &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "ceph-csi-kms-token", Namespace: "velero"}, + Data: map[string][]byte{"token": []byte("same-token")}, + Type: corev1api.SecretTypeOpaque, + }, + }, + }, + { + name: "returns collision error when target has secret with different data", + secretName: "ceph-csi-kms-token", + sourceNS: "app-ns", + targetNS: "velero", + ownerName: "du-123", + objects: []k8sruntime.Object{ + &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "ceph-csi-kms-token", Namespace: "app-ns"}, + Data: map[string][]byte{"token": []byte("token-a")}, + Type: corev1api.SecretTypeOpaque, + }, + &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "ceph-csi-kms-token", Namespace: "velero"}, + Data: map[string][]byte{"token": []byte("token-b")}, + Type: corev1api.SecretTypeOpaque, + }, + }, + expectErr: true, + errContains: "secret collision", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewSimpleClientset(tt.objects...) + + err := CopySecret(context.Background(), fakeClient.CoreV1(), + tt.secretName, tt.sourceNS, tt.targetNS, tt.ownerName, log) + + if tt.expectErr { + require.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + return + } + + require.NoError(t, err) + + copied, getErr := fakeClient.CoreV1().Secrets(tt.targetNS).Get( + context.Background(), tt.secretName, metav1.GetOptions{}) + require.NoError(t, getErr) + assert.NotNil(t, copied) + }) + } +} + +func TestDeleteSecretIfAny(t *testing.T) { + log := logrus.New() + + t.Run("deletes existing secret", func(t *testing.T) { + secret := &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "test-secret", Namespace: "velero"}, + } + fakeClient := fake.NewSimpleClientset(secret) + + DeleteSecretIfAny(context.Background(), fakeClient.CoreV1(), "test-secret", "velero", log) + + _, err := fakeClient.CoreV1().Secrets("velero").Get( + context.Background(), "test-secret", metav1.GetOptions{}) + assert.True(t, err != nil) + }) + + t.Run("no error when secret does not exist", func(t *testing.T) { + fakeClient := fake.NewSimpleClientset() + DeleteSecretIfAny(context.Background(), fakeClient.CoreV1(), "missing", "velero", log) + }) +} + +func TestDeleteSecretsWithLabel(t *testing.T) { + log := logrus.New() + + fakeClient := fake.NewSimpleClientset( + &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-1", Namespace: "velero", + Labels: map[string]string{BackupPVCSecretLabel: "du-123"}, + }, + }, + &corev1api.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-2", Namespace: "velero", + Labels: map[string]string{BackupPVCSecretLabel: "du-456"}, + }, + }, + ) + + DeleteSecretsWithLabel(context.Background(), fakeClient.CoreV1(), "velero", + BackupPVCSecretLabel, "du-123", log) + + _, err := fakeClient.CoreV1().Secrets("velero").Get( + context.Background(), "secret-1", metav1.GetOptions{}) + assert.True(t, err != nil, "secret-1 should be deleted") + + _, err = fakeClient.CoreV1().Secrets("velero").Get( + context.Background(), "secret-2", metav1.GetOptions{}) + assert.NoError(t, err, "secret-2 should still exist") +}