Allow more than one BSL of a provider (#887)

Author: kaovilai

controllers/bsl.go

@@ -27,8 +27,7 @@ func (r *DPAReconciler) ValidateBackupStorageLocations(log logr.Logger) (bool, e
 		return false, errors.New("no backupstoragelocations configured, ensure a backupstoragelocation has been configured")
 	}
 
-	// Ensure BSL:Provider has a 1:1 mapping
-	if err := r.ensureBSLProviderMapping(&dpa); err != nil {
+	if err := r.ensureBackupLocationHasVeleroOrCloudStorage(&dpa); err != nil {
 		return false, err
 	}
 
@@ -336,27 +335,14 @@ func (r *DPAReconciler) validateProviderPluginAndSecret(bslSpec velerov1.BackupS
 	return nil
 }
 
-func (r *DPAReconciler) ensureBSLProviderMapping(dpa *oadpv1alpha1.DataProtectionApplication) error {
-
-	providerBSLMap := map[string]int{}
+func (r *DPAReconciler) ensureBackupLocationHasVeleroOrCloudStorage(dpa *oadpv1alpha1.DataProtectionApplication) error {
 	for _, bsl := range dpa.Spec.BackupLocations {
 		if bsl.CloudStorage == nil && bsl.Velero == nil {
-			return fmt.Errorf("no bucket or BSL provided for backupstoragelocations")
+			return fmt.Errorf("no bucket CloudStorage or velero BackupStorageLocation provided for backupLocations")
 		}
-		if bsl.Velero != nil {
-			// Only check the default providers here, if there are extra credentials passed then we can have more than one.
-			if bsl.Velero.Credential == nil {
-				provider := bsl.Velero.Provider
-
-				providerBSLMap[provider]++
 
-				if providerBSLMap[provider] > 1 {
-					return fmt.Errorf("more than one backupstoragelocations configured for provider %s ", provider)
-				}
-			}
-		}
 		if bsl.CloudStorage != nil && bsl.Velero != nil {
-			return fmt.Errorf("more than one of backupstoragelocations and bucket provided for a single StorageLocation")
+			return fmt.Errorf("cannot have both backupstoragelocations and bucket provided for a single StorageLocation")
 		}
 	}
 	return nil

controllers/bsl_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 
 	"github.com/go-logr/logr"
 	oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
@@ -1305,7 +1306,7 @@ func TestDPAReconciler_updateBSLFromSpec(t *testing.T) {
 	}
 }
 
-func TestDPAReconciler_ensureBSLProviderMapping(t *testing.T) {
+func TestDPAReconciler_ensureBackupLocationHasVeleroOrCloudStorage(t *testing.T) {
 	tests := []struct {
 		name    string
 		dpa     *oadpv1alpha1.DataProtectionApplication
@@ -1341,7 +1342,7 @@ func TestDPAReconciler_ensureBSLProviderMapping(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "two bsl configured for aws provider",
+			name: "wantErr: a bsl has both velero and cloudstorage configured",
 			dpa: &oadpv1alpha1.DataProtectionApplication{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "foo",
@@ -1353,12 +1354,41 @@ func TestDPAReconciler_ensureBSLProviderMapping(t *testing.T) {
 							Velero: &velerov1.BackupStorageLocationSpec{
 								Provider: "aws",
 							},
+							CloudStorage: &oadpv1alpha1.CloudStorageLocation{
+								CloudStorageRef: v1.LocalObjectReference{
+									Name: "foo",
+								},
+							},
 						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "two bsl configured per provider",
+			dpa: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "test-ns",
+				},
+				Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+					BackupLocations: []oadpv1alpha1.BackupLocation{
 						{
 							Velero: &velerov1.BackupStorageLocationSpec{
 								Provider: "aws",
 							},
 						},
+						{
+							Velero: &velerov1.BackupStorageLocationSpec{
+								Provider: "aws",
+							},
+						},
+						{
+							Velero: &velerov1.BackupStorageLocationSpec{
+								Provider: "azure",
+							},
+						},
 						{
 							Velero: &velerov1.BackupStorageLocationSpec{
 								Provider: "azure",
@@ -1371,13 +1401,13 @@ func TestDPAReconciler_ensureBSLProviderMapping(t *testing.T) {
 						},
 						{
 							Velero: &velerov1.BackupStorageLocationSpec{
-								Provider: "thirdpary-objectstorage-provider",
+								Provider: "gcp",
 							},
 						},
 					},
 				},
 			},
-			wantErr: true,
+			wantErr: false,
 		},
 	}
 	for _, tt := range tests {
@@ -1389,7 +1419,7 @@ func TestDPAReconciler_ensureBSLProviderMapping(t *testing.T) {
 			r := &DPAReconciler{
 				Scheme: scheme,
 			}
-			if err := r.ensureBSLProviderMapping(tt.dpa); (err != nil) != tt.wantErr {
+			if err := r.ensureBackupLocationHasVeleroOrCloudStorage(tt.dpa); (err != nil) != tt.wantErr {
 				t.Errorf("ensureBSLProviderMapping() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})