diff --git a/controllers/validator.go b/controllers/validator.go index 7630b68fca0..b620933a68a 100644 --- a/controllers/validator.go +++ b/controllers/validator.go @@ -18,12 +18,18 @@ 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 dpa.Spec.Configuration.Velero.NoDefaultBackupLocation && dpa.BackupImages() { - return false, errors.New("backupImages needs to be set to false when noDefaultLocationBackupLocation is set") + return false, errors.New("backupImages needs to be set to false when noDefaultBackupLocation is set") } if len(dpa.Spec.BackupLocations) > 0 { diff --git a/controllers/velero.go b/controllers/velero.go index d5d64dd8ea9..4ca68c26ba3 100644 --- a/controllers/velero.go +++ b/controllers/velero.go @@ -676,32 +676,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 diff --git a/controllers/velero_test.go b/controllers/velero_test.go index cccd829569c..adf8e5d38ef 100644 --- a/controllers/velero_test.go +++ b/controllers/velero_test.go @@ -1992,3 +1992,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) + } + }) + } +} diff --git a/docs/credentials.md b/docs/credentials.md index 5e1979b1f8a..853b519da44 100644 --- a/docs/credentials.md +++ b/docs/credentials.md @@ -171,7 +171,25 @@ spec:
-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. \ No newline at end of file +VSL credentials **must** be the [default secret name](#defaultsecrets) diff --git a/pkg/credentials/credentials.go b/pkg/credentials/credentials.go index 0ef4664d958..c745b4cdc26 100644 --- a/pkg/credentials/credentials.go +++ b/pkg/credentials/credentials.go @@ -175,14 +175,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 } @@ -273,6 +271,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