[OADP-460] Cherry-pick #662 into 1.0

controllers/validator.go

@@ -18,8 +18,14 @@ func (r *DPAReconciler) ValidateDataProtectionCR(log logr.Logger) (bool, error)
 		return false, errors.New("DPA CR Velero configuration cannot be nil")
 	}
 
-	if len(dpa.Spec.BackupLocations) == 0 && !dpa.Spec.Configuration.Velero.NoDefaultBackupLocation {
-		return false, errors.New("no backupstoragelocations configured, ensure a backupstoragelocation has been configured or use the noDefaultLocationBackupLocation flag")
+	if dpa.Spec.Configuration.Velero.NoDefaultBackupLocation {
+		if len(dpa.Spec.BackupLocations) != 0 {
+			return false, errors.New("DPA CR Velero configuration cannot have backup locations if noDefaultBackupLocation is set")
+		}
+	} else {
+		if len(dpa.Spec.BackupLocations) == 0 {
+			return false, errors.New("no backupstoragelocations configured, ensure a backupstoragelocation has been configured or use the noDefaultBackupLocation flag")
+		}
 	}
 
 	if len(dpa.Spec.BackupLocations) > 0 {

controllers/velero.go

@@ -711,32 +711,39 @@ func (r *DPAReconciler) getResticResourceReqs(dpa *oadpv1alpha1.DataProtectionAp
 func (r DPAReconciler) noDefaultCredentials(dpa oadpv1alpha1.DataProtectionApplication) (map[string]bool, bool, error) {
 	providerNeedsDefaultCreds := map[string]bool{}
 	hasCloudStorage := false
-
-	for _, bsl := range dpa.Spec.BackupLocations {
-		if bsl.Velero != nil && bsl.Velero.Credential == nil {
-			bslProvider := strings.TrimPrefix(bsl.Velero.Provider, "velero.io/")
-			providerNeedsDefaultCreds[bslProvider] = true
-		}
-		if bsl.Velero != nil && bsl.Velero.Credential != nil {
-			bslProvider := strings.TrimPrefix(bsl.Velero.Provider, "velero.io/")
-			if found := providerNeedsDefaultCreds[bslProvider]; !found {
-				providerNeedsDefaultCreds[bslProvider] = false
+	if dpa.Spec.Configuration.Velero.NoDefaultBackupLocation {
+		// go through cloudprovider plugins and mark providerNeedsDefaultCreds to false
+		for _, provider := range dpa.Spec.Configuration.Velero.DefaultPlugins {
+			if psf, ok := credentials.PluginSpecificFields[provider]; ok && psf.IsCloudProvider {
+				providerNeedsDefaultCreds[psf.PluginName] = false
 			}
 		}
-		if bsl.CloudStorage != nil {
-			if bsl.CloudStorage.Credential == nil {
-				cloudStorage := oadpv1alpha1.CloudStorage{}
-				err := r.Get(r.Context, types.NamespacedName{Name: bsl.CloudStorage.CloudStorageRef.Name, Namespace: dpa.Namespace}, &cloudStorage)
-				if err != nil {
-					return nil, false, err
+	} else {
+		for _, bsl := range dpa.Spec.BackupLocations {
+			if bsl.Velero != nil && bsl.Velero.Credential == nil {
+				bslProvider := strings.TrimPrefix(bsl.Velero.Provider, "velero.io/")
+				providerNeedsDefaultCreds[bslProvider] = true
+			}
+			if bsl.Velero != nil && bsl.Velero.Credential != nil {
+				bslProvider := strings.TrimPrefix(bsl.Velero.Provider, "velero.io/")
+				if found := providerNeedsDefaultCreds[bslProvider]; !found {
+					providerNeedsDefaultCreds[bslProvider] = false
+				}
+			}
+			if bsl.CloudStorage != nil {
+				if bsl.CloudStorage.Credential == nil {
+					cloudStorage := oadpv1alpha1.CloudStorage{}
+					err := r.Get(r.Context, types.NamespacedName{Name: bsl.CloudStorage.CloudStorageRef.Name, Namespace: dpa.Namespace}, &cloudStorage)
+					if err != nil {
+						return nil, false, err
+					}
+					providerNeedsDefaultCreds[string(cloudStorage.Spec.Provider)] = true
+				} else {
+					hasCloudStorage = true
 				}
-				providerNeedsDefaultCreds[string(cloudStorage.Spec.Provider)] = true
-			} else {
-				hasCloudStorage = true
 			}
 		}
 	}
-
 	for _, vsl := range dpa.Spec.SnapshotLocations {
 		if vsl.Velero != nil {
 			// To handle the case where we want to manually hand the credentials for a cloud storage created

controllers/velero_test.go

@@ -2816,3 +2816,96 @@ func Test_validateVeleroPlugins(t *testing.T) {
 		})
 	}
 }
+
+var allDefaultPluginsList = []oadpv1alpha1.DefaultPlugin{
+	oadpv1alpha1.DefaultPluginAWS,
+	oadpv1alpha1.DefaultPluginGCP,
+	oadpv1alpha1.DefaultPluginMicrosoftAzure,
+	oadpv1alpha1.DefaultPluginKubeVirt,
+	oadpv1alpha1.DefaultPluginOpenShift,
+	oadpv1alpha1.DefaultPluginCSI,
+}
+
+func TestDPAReconciler_noDefaultCredentials(t *testing.T) {
+	type args struct {
+		dpa oadpv1alpha1.DataProtectionApplication
+	}
+	tests := []struct {
+		name                string
+		args                args
+		want                map[string]bool
+		wantHasCloudStorage bool
+		wantErr             bool
+	}{
+		{
+			name: "dpa with all plugins but with noDefualtBackupLocation should not require default credentials",
+			args: args{
+				dpa: oadpv1alpha1.DataProtectionApplication{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-Velero-CR",
+						Namespace: "test-ns",
+					},
+					Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+						Configuration: &oadpv1alpha1.ApplicationConfig{
+							Velero: &oadpv1alpha1.VeleroConfig{
+								DefaultPlugins:          allDefaultPluginsList,
+								NoDefaultBackupLocation: true,
+							},
+						},
+					},
+				},
+			},
+			want: map[string]bool{
+				"velero-plugin-for-aws":             false,
+				"velero-plugin-for-gcp":             false,
+				"velero-plugin-for-microsoft-azure": false,
+			},
+			wantHasCloudStorage: false,
+			wantErr:             false,
+		},
+		{
+			name: "dpa no default cloudprovider plugins should not require default credentials",
+			args: args{
+				dpa: oadpv1alpha1.DataProtectionApplication{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-Velero-CR",
+						Namespace: "test-ns",
+					},
+					Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+						Configuration: &oadpv1alpha1.ApplicationConfig{
+							Velero: &oadpv1alpha1.VeleroConfig{
+								DefaultPlugins:          []oadpv1alpha1.DefaultPlugin{oadpv1alpha1.DefaultPluginOpenShift},
+								NoDefaultBackupLocation: true,
+							},
+						},
+					},
+				},
+			},
+			want:                map[string]bool{},
+			wantHasCloudStorage: false,
+			wantErr:             false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeClient, err := getFakeClientFromObjects(&tt.args.dpa)
+			if err != nil {
+				t.Errorf("error in creating fake client, likely programmer error")
+			}
+			r := DPAReconciler{
+				Client: fakeClient,
+			}
+			got, got1, err := r.noDefaultCredentials(tt.args.dpa)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("DPAReconciler.noDefaultCredentials() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("DPAReconciler.noDefaultCredentials() got = \n%v, \nwant \n%v", got, tt.want)
+			}
+			if got1 != tt.wantHasCloudStorage {
+				t.Errorf("DPAReconciler.noDefaultCredentials() got1 = %v, want %v", got1, tt.wantHasCloudStorage)
+			}
+		})
+	}
+}

docs/credentials.md

@@ -171,7 +171,25 @@ spec:
 
 <hr style="height:1px;border:none;color:#333;">
 
-3. #### No `BackupStorageLocation` or `VolumeSnapshotLocation` specified, but the plugin for the provider exists:
+1. #### No `BackupStorageLocation` specified, but the plugin for the provider exists:
+Specify .spec.configuration.noDefaultBackupLocation like so:
+```yaml
+apiVersion: oadp.openshift.io/v1alpha1
+kind: DataProtectionApplication
+metadata:
+  name: velero-sample
+spec:
+  configuration:
+    velero:
+      defaultPlugins:
+      - openshift
+      - aws
+      noDefaultBackupLocation: true
+    restic:
+      enable: true
+```
+If you don't need volumesnapshotlocation, you will not need to create a VSL credentials.
+
+If you need `VolumeSnapshotLocation`, regardless of the `noDefaultBackupLocation` setting, you will need a to create VSL credentials.
 
-    - The default secret name still needs to exist in order for the Velero installation
-    to be successful, but the secret can be empty.
+VSL credentials **must** be the [default secret name](#defaultsecrets)

pkg/credentials/credentials.go

@@ -190,14 +190,12 @@ func AppendCloudProviderVolumes(dpa *oadpv1alpha1.DataProtectionApplication, ds
 		// ok is boolean that will be true if `plugin` is a valid key in `PluginSpecificFields` map
 		// pattern from https://golang.org/doc/effective_go#maps
 		// this replaces the need to iterate through the `pluginSpecificFields` O(n) -> O(1)
-		if cloudProviderMap, ok := PluginSpecificFields[plugin]; ok {
-			if !cloudProviderMap.IsCloudProvider {
-				continue
-			}
-
-			pluginNeedsCheck, foundInBSLorVSL := providerNeedsDefaultCreds[string(plugin)]
+		if cloudProviderMap, ok := PluginSpecificFields[plugin]; ok &&
+			cloudProviderMap.IsCloudProvider &&
+			!dpa.Spec.Configuration.Velero.NoDefaultBackupLocation {
 
-			if !foundInBSLorVSL && !hasCloudStorage {
+			pluginNeedsCheck, foundProviderPlugin := providerNeedsDefaultCreds[string(plugin)]
+			if !foundProviderPlugin && !hasCloudStorage {
 				pluginNeedsCheck = true
 			}
 
@@ -288,6 +286,9 @@ func AppendPluginSpecificSpecs(dpa *oadpv1alpha1.DataProtectionApplication, vele
 			if !pluginSpecificMap.IsCloudProvider || !pluginNeedsCheck {
 				continue
 			}
+			if dpa.Spec.Configuration.Velero.NoDefaultBackupLocation && pluginSpecificMap.IsCloudProvider {
+				continue
+			}
 			// set default secret name to use
 			secretName := pluginSpecificMap.SecretName
 			// append plugin specific volume mounts