From c15cf084e366044065117080afba32b20e1608e1 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Wed, 8 Jul 2026 11:51:06 -0700 Subject: [PATCH] Add configmap copy support and move secret copy after accept - Add ConfigMapNames field to BackupPVC config for copying tenant configmaps (e.g., ceph-csi-kms-config with Vault connection overrides) - Add CopyConfigMap, DeleteConfigMapIfAny, DeleteConfigMapsWithLabel utilities mirroring the secret copy functions - Move secret/configmap copy after acceptDataUpload() so only the accepting node handles it, avoiding multi-node contest - Clean up copied configmaps in CleanUp() alongside secrets Signed-off-by: Shubham Pampattiwar --- pkg/controller/data_upload_controller.go | 38 +++++------ pkg/exposer/csi_snapshot.go | 2 + pkg/types/node_agent.go | 7 +++ pkg/util/kube/secrets.go | 80 ++++++++++++++++++++++-- 4 files changed, 105 insertions(+), 22 deletions(-) diff --git a/pkg/controller/data_upload_controller.go b/pkg/controller/data_upload_controller.go index c01918daa..357bc943d 100644 --- a/pkg/controller/data_upload_controller.go +++ b/pkg/controller/data_upload_controller.go @@ -270,24 +270,6 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil } - // Copy secrets required for backup PVC provisioning (e.g., encrypted volumes with KMS). - // This must happen before Expose() since Expose() errors are non-retryable. - // On collision (same secret name, different data from another DataUpload), requeue. - if du.Spec.CSISnapshot != nil { - if bpvcConfig, exists := r.backupPVCConfig[du.Spec.CSISnapshot.StorageClass]; exists { - for _, secretName := range bpvcConfig.SecretNames { - if copyErr := kube.CopySecret(ctx, r.kubeClient.CoreV1(), secretName, - du.Spec.SourceNamespace, du.Namespace, du.Name, log); copyErr != nil { - if errors.Is(copyErr, kube.ErrSecretCollision) { - log.Infof("Secret %s collision detected, requeue later", secretName) - return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil - } - return r.errorOut(ctx, du, copyErr, "error copying secret for backup PVC", log) - } - } - } - } - log.Info("Data upload starting") accepted, err := r.acceptDataUpload(ctx, du) @@ -302,6 +284,26 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request) log.Info("Data upload is accepted") + // Copy secrets and configmaps required for backup PVC provisioning + // (e.g., encrypted volumes with KMS). Done after accept so only the + // accepting node handles it, avoiding multi-node contest. + if du.Spec.CSISnapshot != nil { + if bpvcConfig, exists := r.backupPVCConfig[du.Spec.CSISnapshot.StorageClass]; exists { + for _, secretName := range bpvcConfig.SecretNames { + if copyErr := kube.CopySecret(ctx, r.kubeClient.CoreV1(), secretName, + du.Spec.SourceNamespace, du.Namespace, du.Name, log); copyErr != nil { + return r.errorOut(ctx, du, copyErr, "error copying secret for backup PVC", log) + } + } + for _, cmName := range bpvcConfig.ConfigMapNames { + if copyErr := kube.CopyConfigMap(ctx, r.kubeClient.CoreV1(), cmName, + du.Spec.SourceNamespace, du.Namespace, du.Name, log); copyErr != nil { + return r.errorOut(ctx, du, copyErr, "error copying configmap for backup PVC", log) + } + } + } + } + exposeParam, err := r.setupExposeParam(du) if err != nil { return r.errorOut(ctx, du, err, "failed to set exposer parameters", log) diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index 907fdc885..ed19aeef1 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -516,6 +516,8 @@ func (e *csiSnapshotExposer) CleanUp(ctx context.Context, ownerObject corev1api. kube.DeleteSecretsWithLabel(ctx, e.kubeClient.CoreV1(), ownerObject.Namespace, kube.BackupPVCSecretLabel, ownerObject.Name, e.log) + kube.DeleteConfigMapsWithLabel(ctx, e.kubeClient.CoreV1(), ownerObject.Namespace, + kube.BackupPVCSecretLabel, ownerObject.Name, e.log) csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, backupVSName, ownerObject.Namespace, e.log) csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, vsName, sourceNamespace, e.log) diff --git a/pkg/types/node_agent.go b/pkg/types/node_agent.go index 88e7fe336..08899e8c4 100644 --- a/pkg/types/node_agent.go +++ b/pkg/types/node_agent.go @@ -65,6 +65,13 @@ type BackupPVC struct { // after the DataUpload completes. This is needed for CSI drivers that require // namespace-scoped secrets for volume provisioning (e.g., encrypted volumes). SecretNames []string `json:"secretNames,omitempty"` + + // ConfigMapNames is a list of configmap names to copy from the source PVC namespace + // to the Velero namespace before creating the backupPVC. The configmaps are deleted + // after the DataUpload completes. This is needed for CSI drivers that require + // namespace-scoped configmaps for volume provisioning (e.g., tenant-specific + // Vault connection overrides for encrypted volumes). + ConfigMapNames []string `json:"configMapNames,omitempty"` } type RestorePVC struct { diff --git a/pkg/util/kube/secrets.go b/pkg/util/kube/secrets.go index b72959d26..3e7f88610 100644 --- a/pkg/util/kube/secrets.go +++ b/pkg/util/kube/secrets.go @@ -56,13 +56,13 @@ func GetSecretKey(client kbclient.Client, namespace string, selector *corev1api. } 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 is the label applied to secrets and configmaps copied to the + // Velero namespace for backup PVC provisioning. The value is the owning DataUpload name. BackupPVCSecretLabel = "velero.io/backup-pvc-secret" //nolint:gosec // not a credential ) -// 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. +// ErrSecretCollision is returned when a secret or configmap 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. @@ -137,3 +137,75 @@ func DeleteSecretsWithLabel(ctx context.Context, client corev1client.CoreV1Inter DeleteSecretIfAny(ctx, client, secrets.Items[i].Name, namespace, log) } } + +// CopyConfigMap copies a configmap from sourceNamespace to targetNamespace. +// If a configmap with the same name already exists in the target with identical data, it is a no-op. +// If a configmap with the same name exists with different data (collision from another DataUpload), +// it returns ErrSecretCollision so the caller can requeue. +func CopyConfigMap(ctx context.Context, client corev1client.CoreV1Interface, cmName, sourceNamespace, targetNamespace string, ownerName string, log logrus.FieldLogger) error { + srcCM, err := client.ConfigMaps(sourceNamespace).Get(ctx, cmName, metav1.GetOptions{}) + if err != nil { + return errors.Wrapf(err, "error getting configmap %s/%s", sourceNamespace, cmName) + } + + newCM := &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: targetNamespace, + Labels: map[string]string{ + BackupPVCSecretLabel: ownerName, + }, + }, + Data: srcCM.Data, + } + + _, err = client.ConfigMaps(targetNamespace).Create(ctx, newCM, metav1.CreateOptions{}) + if err == nil { + log.Infof("Copied configmap %s from %s to %s", cmName, sourceNamespace, targetNamespace) + return nil + } + + if !apierrors.IsAlreadyExists(err) { + return errors.Wrapf(err, "error creating configmap %s in %s", cmName, targetNamespace) + } + + existing, err := client.ConfigMaps(targetNamespace).Get(ctx, cmName, metav1.GetOptions{}) + if err != nil { + return errors.Wrapf(err, "error getting existing configmap %s/%s", targetNamespace, cmName) + } + + if reflect.DeepEqual(existing.Data, srcCM.Data) { + log.Infof("ConfigMap %s already exists in %s with same data, skipping copy", cmName, targetNamespace) + return nil + } + + log.Infof("ConfigMap %s already exists in %s with different data, collision detected", cmName, targetNamespace) + return ErrSecretCollision +} + +// DeleteConfigMapIfAny deletes a configmap if it exists, logging but not returning errors. +func DeleteConfigMapIfAny(ctx context.Context, client corev1client.CoreV1Interface, cmName, namespace string, log logrus.FieldLogger) { + err := client.ConfigMaps(namespace).Delete(ctx, cmName, metav1.DeleteOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + log.Debugf("ConfigMap %s/%s not found, skipping delete", namespace, cmName) + } else { + log.WithError(err).Errorf("Failed to delete configmap %s/%s", namespace, cmName) + } + } +} + +// DeleteConfigMapsWithLabel deletes all configmaps in a namespace matching a label key=value pair. +func DeleteConfigMapsWithLabel(ctx context.Context, client corev1client.CoreV1Interface, namespace, labelKey, labelValue string, log logrus.FieldLogger) { + cms, err := client.ConfigMaps(namespace).List(ctx, metav1.ListOptions{ + LabelSelector: labelKey + "=" + labelValue, + }) + if err != nil { + log.WithError(err).Errorf("Failed to list configmaps with label %s=%s in %s", labelKey, labelValue, namespace) + return + } + + for i := range cms.Items { + DeleteConfigMapIfAny(ctx, client, cms.Items[i].Name, namespace, log) + } +}