Replace NewObjectBackupStore with interface (#3329)

In preparation for modifying the instantiation of `BackupStores` to be
able to load credentials, change the function `NewObjectBackupStore` to
be an interface that is passed in to all controllers.

Previously, the function to get a new backup store was configurable but
for many controllers was fixed to use `NewObjectBackupStore`. This
change introduces an interface for getting the backup store and wraps
the functionality from `NewObjectBackupStore` in a type which implements
this interface. This will allow more flexibility when introducing
credentials for a specific backup store as it will allow us to create a
new `ObjectBackupStoreGetter` type which can be configured to add
credentials config when creating the ObjectBackupStore without needing
to change the API used by the controllers.

Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>

pkg/cmd/server/server.go

@@ -578,6 +578,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			s.kubeClient,
 			s.config.defaultBackupLocation,
 			newPluginManager,
+			persistence.NewObjectBackupStoreGetter(),
 			s.logger,
 		)
 
@@ -620,6 +621,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			s.config.formatFlag.Parse(),
 			csiVSLister,
 			csiVSCLister,
+			persistence.NewObjectBackupStoreGetter(),
 		)
 
 		return controllerRunInfo{
@@ -676,6 +678,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			csiVSCLister,
 			s.csiSnapshotClient,
 			newPluginManager,
+			persistence.NewObjectBackupStoreGetter(),
 			s.metrics,
 			s.discoveryHelper,
 		)
@@ -713,6 +716,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			s.logger,
 			s.logLevel,
 			newPluginManager,
+			persistence.NewObjectBackupStoreGetter(),
 			s.metrics,
 			s.config.formatFlag.Parse(),
 		)
@@ -747,6 +751,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			s.mgr.GetClient(),
 			s.sharedInformerFactory.Velero().V1().Backups().Lister(),
 			newPluginManager,
+			persistence.NewObjectBackupStoreGetter(),
 			s.logger,
 		)
 
@@ -824,9 +829,9 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			StorageLocation:           s.config.defaultBackupLocation,
 			ServerValidationFrequency: s.config.storeValidationFrequency,
 		},
-		NewPluginManager: newPluginManager,
-		NewBackupStore:   persistence.NewObjectBackupStore,
-		Log:              s.logger,
+		NewPluginManager:  newPluginManager,
+		BackupStoreGetter: persistence.NewObjectBackupStoreGetter(),
+		Log:               s.logger,
 	}
 	if err := bslr.SetupWithManager(s.mgr); err != nil {
 		s.logger.Fatal(err, "unable to create controller", "controller", controller.BackupStorageLocation)

pkg/controller/backup_controller.go

@@ -80,7 +80,7 @@ type backupController struct {
 	snapshotLocationLister      velerov1listers.VolumeSnapshotLocationLister
 	defaultSnapshotLocations    map[string]string
 	metrics                     *metrics.ServerMetrics
-	newBackupStore              func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error)
+	backupStoreGetter           persistence.ObjectBackupStoreGetter
 	formatFlag                  logging.Format
 	volumeSnapshotLister        snapshotv1beta1listers.VolumeSnapshotLister
 	volumeSnapshotContentLister snapshotv1beta1listers.VolumeSnapshotContentLister
@@ -105,6 +105,7 @@ func NewBackupController(
 	formatFlag logging.Format,
 	volumeSnapshotLister snapshotv1beta1listers.VolumeSnapshotLister,
 	volumeSnapshotContentLister snapshotv1beta1listers.VolumeSnapshotContentLister,
+	backupStoreGetter persistence.ObjectBackupStoreGetter,
 ) Interface {
 	c := &backupController{
 		genericController:           newGenericController(Backup, logger),
@@ -126,7 +127,7 @@ func NewBackupController(
 		formatFlag:                  formatFlag,
 		volumeSnapshotLister:        volumeSnapshotLister,
 		volumeSnapshotContentLister: volumeSnapshotContentLister,
-		newBackupStore:              persistence.NewObjectBackupStore,
+		backupStoreGetter:           backupStoreGetter,
 	}
 
 	c.syncHandler = c.processBackup
@@ -570,7 +571,7 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 	}
 
 	backupLog.Info("Setting up backup store to check for backup existence")
-	backupStore, err := c.newBackupStore(backup.StorageLocation, pluginManager, backupLog)
+	backupStore, err := c.backupStoreGetter.Get(backup.StorageLocation, pluginManager, backupLog)
 	if err != nil {
 		return err
 	}
@@ -651,7 +652,7 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 	// re-instantiate the backup store because credentials could have changed since the original
 	// instantiation, if this was a long-running backup
 	backupLog.Info("Setting up backup store to persist the backup")
-	backupStore, err = c.newBackupStore(backup.StorageLocation, pluginManager, backupLog)
+	backupStore, err = c.backupStoreGetter.Get(backup.StorageLocation, pluginManager, backupLog)
 	if err != nil {
 		return err
 	}

pkg/controller/backup_controller_test.go

@@ -818,11 +818,9 @@ func TestProcessBackupCompletions(t *testing.T) {
 				metrics:                metrics.NewServerMetrics(),
 				clock:                  clock.NewFakeClock(now),
 				newPluginManager:       func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
-				newBackupStore: func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) {
-					return backupStore, nil
-				},
-				backupper:  backupper,
-				formatFlag: formatFlag,
+				backupStoreGetter:      NewFakeSingleObjectBackupStoreGetter(backupStore),
+				backupper:              backupper,
+				formatFlag:             formatFlag,
 			}
 
 			pluginManager.On("GetBackupItemActions").Return(nil, nil)

pkg/controller/backup_deletion_controller.go

@@ -77,7 +77,7 @@ type backupDeletionController struct {
 	processRequestFunc        func(*velerov1api.DeleteBackupRequest) error
 	clock                     clock.Clock
 	newPluginManager          func(logrus.FieldLogger) clientmgmt.Manager
-	newBackupStore            func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error)
+	backupStoreGetter         persistence.ObjectBackupStoreGetter
 	metrics                   *metrics.ServerMetrics
 	helper                    discovery.Helper
 }
@@ -99,6 +99,7 @@ func NewBackupDeletionController(
 	csiSnapshotContentLister snapshotv1beta1listers.VolumeSnapshotContentLister,
 	csiSnapshotClient *snapshotterClientSet.Clientset,
 	newPluginManager func(logrus.FieldLogger) clientmgmt.Manager,
+	backupStoreGetter persistence.ObjectBackupStoreGetter,
 	metrics *metrics.ServerMetrics,
 	helper discovery.Helper,
 ) Interface {
@@ -121,8 +122,8 @@ func NewBackupDeletionController(
 		helper:                    helper,
 		// use variables to refer to these functions so they can be
 		// replaced with fakes for testing.
-		newPluginManager: newPluginManager,
-		newBackupStore:   persistence.NewObjectBackupStore,
+		newPluginManager:  newPluginManager,
+		backupStoreGetter: backupStoreGetter,
 
 		clock: &clock.RealClock{},
 	}
@@ -290,7 +291,7 @@ func (c *backupDeletionController) processRequest(req *velerov1api.DeleteBackupR
 	pluginManager := c.newPluginManager(log)
 	defer pluginManager.CleanupClients()
 
-	backupStore, err := c.newBackupStore(location, pluginManager, log)
+	backupStore, err := c.backupStoreGetter.Get(location, pluginManager, log)
 	if err != nil {
 		errs = append(errs, err.Error())
 	}

pkg/controller/backup_deletion_controller_test.go

@@ -75,6 +75,7 @@ func TestBackupDeletionControllerProcessQueueItem(t *testing.T) {
 		nil, // csiSnapshotContentLister
 		nil, // csiSnapshotClient
 		nil, // new plugin manager func
+		persistence.NewObjectBackupStoreGetter(),
 		metrics.NewServerMetrics(),
 		nil, // discovery helper
 	).(*backupDeletionController)
@@ -172,6 +173,7 @@ func setupBackupDeletionControllerTest(t *testing.T, objects ...runtime.Object)
 			nil, // csiSnapshotContentLister
 			nil, // csiSnapshotClient
 			func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+			NewFakeSingleObjectBackupStoreGetter(backupStore),
 			metrics.NewServerMetrics(),
 			nil, // discovery helper
 		).(*backupDeletionController),
@@ -179,10 +181,6 @@ func setupBackupDeletionControllerTest(t *testing.T, objects ...runtime.Object)
 		req: req,
 	}
 
-	data.controller.newBackupStore = func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) {
-		return backupStore, nil
-	}
-
 	pluginManager.On("CleanupClients").Return(nil)
 
 	return data
@@ -1133,6 +1131,7 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) {
 				nil, // csiSnapshotContentLister
 				nil, // csiSnapshotClient
 				nil, // new plugin manager func
+				persistence.NewObjectBackupStoreGetter(),
 				metrics.NewServerMetrics(),
 				nil, // discovery helper,
 			).(*backupDeletionController)

pkg/controller/backup_storage_location_controller.go

@@ -44,8 +44,8 @@ type BackupStorageLocationReconciler struct {
 	DefaultBackupLocationInfo storage.DefaultBackupLocationInfo
 	// use variables to refer to these functions so they can be
 	// replaced with fakes for testing.
-	NewPluginManager func(logrus.FieldLogger) clientmgmt.Manager
-	NewBackupStore   func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error)
+	NewPluginManager  func(logrus.FieldLogger) clientmgmt.Manager
+	BackupStoreGetter persistence.ObjectBackupStoreGetter
 
 	Log logrus.FieldLogger
 }
@@ -95,7 +95,7 @@ func (r *BackupStorageLocationReconciler) Reconcile(ctx context.Context, req ctr
 			continue
 		}
 
-		backupStore, err := r.NewBackupStore(location, pluginManager, log)
+		backupStore, err := r.BackupStoreGetter.Get(location, pluginManager, log)
 		if err != nil {
 			log.WithError(err).Error("Error getting a backup store")
 			continue

pkg/controller/backup_storage_location_controller_test.go

@@ -33,7 +33,6 @@ import (
 	"github.com/vmware-tanzu/velero/internal/storage"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/builder"
-	"github.com/vmware-tanzu/velero/pkg/persistence"
 	persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
 	pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks"
@@ -87,12 +86,9 @@ var _ = Describe("Backup Storage Location Reconciler", func() {
 				StorageLocation:           "location-1",
 				ServerValidationFrequency: 0,
 			},
-			NewPluginManager: func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
-			NewBackupStore: func(loc *velerov1api.BackupStorageLocation, _ persistence.ObjectStoreGetter, _ logrus.FieldLogger) (persistence.BackupStore, error) {
-				// this gets populated just below, prior to exercising the method under test
-				return backupStores[loc.Name], nil
-			},
-			Log: velerotest.NewLogger(),
+			NewPluginManager:  func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+			BackupStoreGetter: NewFakeObjectBackupStoreGetter(backupStores),
+			Log:               velerotest.NewLogger(),
 		}
 
 		actualResult, err := r.Reconcile(ctx, ctrl.Request{
@@ -156,12 +152,9 @@ var _ = Describe("Backup Storage Location Reconciler", func() {
 				StorageLocation:           "default",
 				ServerValidationFrequency: 0,
 			},
-			NewPluginManager: func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
-			NewBackupStore: func(loc *velerov1api.BackupStorageLocation, _ persistence.ObjectStoreGetter, _ logrus.FieldLogger) (persistence.BackupStore, error) {
-				// this gets populated just below, prior to exercising the method under test
-				return backupStores[loc.Name], nil
-			},
-			Log: velerotest.NewLogger(),
+			NewPluginManager:  func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+			BackupStoreGetter: NewFakeObjectBackupStoreGetter(backupStores),
+			Log:               velerotest.NewLogger(),
 		}
 
 		actualResult, err := r.Reconcile(ctx, ctrl.Request{
@@ -229,12 +222,9 @@ var _ = Describe("Backup Storage Location Reconciler", func() {
 				StorageLocation:           "default",
 				ServerValidationFrequency: 0,
 			},
-			NewPluginManager: func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
-			NewBackupStore: func(loc *velerov1api.BackupStorageLocation, _ persistence.ObjectStoreGetter, _ logrus.FieldLogger) (persistence.BackupStore, error) {
-				// this gets populated just below, prior to exercising the method under test
-				return backupStores[loc.Name], nil
-			},
-			Log: velerotest.NewLogger(),
+			NewPluginManager:  func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+			BackupStoreGetter: NewFakeObjectBackupStoreGetter(backupStores),
+			Log:               velerotest.NewLogger(),
 		}
 
 		actualResult, err := r.Reconcile(ctx, ctrl.Request{

pkg/controller/backup_sync_controller.go

@@ -54,7 +54,7 @@ type backupSyncController struct {
 	defaultBackupLocation   string
 	defaultBackupSyncPeriod time.Duration
 	newPluginManager        func(logrus.FieldLogger) clientmgmt.Manager
-	newBackupStore          func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error)
+	backupStoreGetter       persistence.ObjectBackupStoreGetter
 }
 
 func NewBackupSyncController(
@@ -68,6 +68,7 @@ func NewBackupSyncController(
 	kubeClient kubernetes.Interface,
 	defaultBackupLocation string,
 	newPluginManager func(logrus.FieldLogger) clientmgmt.Manager,
+	backupStoreGetter persistence.ObjectBackupStoreGetter,
 	logger logrus.FieldLogger,
 ) Interface {
 	if syncPeriod <= 0 {
@@ -89,8 +90,8 @@ func NewBackupSyncController(
 
 		// use variables to refer to these functions so they can be
 		// replaced with fakes for testing.
-		newPluginManager: newPluginManager,
-		newBackupStore:   persistence.NewObjectBackupStore,
+		newPluginManager:  newPluginManager,
+		backupStoreGetter: backupStoreGetter,
 	}
 
 	c.resyncFunc = c.run
@@ -169,7 +170,7 @@ func (c *backupSyncController) run() {
 
 		log.Debug("Checking backup location for backups to sync into cluster")
 
-		backupStore, err := c.newBackupStore(&location, pluginManager, log)
+		backupStore, err := c.backupStoreGetter.Get(&location, pluginManager, log)
 		if err != nil {
 			log.WithError(err).Error("Error getting backup store for this location")
 			continue

pkg/controller/backup_sync_controller_test.go

@@ -21,6 +21,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/vmware-tanzu/velero/pkg/persistence"
+
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -34,7 +36,6 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/fake"
 	informers "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions"
 	"github.com/vmware-tanzu/velero/pkg/label"
-	"github.com/vmware-tanzu/velero/pkg/persistence"
 	persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
 	pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks"
@@ -351,14 +352,10 @@ func TestBackupSyncControllerRun(t *testing.T) {
 				nil, // kubeClient
 				"",
 				func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+				NewFakeObjectBackupStoreGetter(backupStores),
 				velerotest.NewLogger(),
 			).(*backupSyncController)
 
-			c.newBackupStore = func(loc *velerov1api.BackupStorageLocation, _ persistence.ObjectStoreGetter, _ logrus.FieldLogger) (persistence.BackupStore, error) {
-				// this gets populated just below, prior to exercising the method under test
-				return backupStores[loc.Name], nil
-			}
-
 			pluginManager.On("CleanupClients").Return(nil)
 
 			for _, location := range test.locations {
@@ -576,6 +573,7 @@ func TestDeleteOrphanedBackups(t *testing.T) {
 				nil, // kubeClient
 				"",
 				nil, // new plugin manager func
+				persistence.NewObjectBackupStoreGetter(),
 				velerotest.NewLogger(),
 			).(*backupSyncController)
 
@@ -669,6 +667,7 @@ func TestStorageLabelsInDeleteOrphanedBackups(t *testing.T) {
 				nil, // kubeClient
 				"",
 				nil, // new plugin manager func
+				persistence.NewObjectBackupStoreGetter(),
 				velerotest.NewLogger(),
 			).(*backupSyncController)
 

pkg/controller/download_request_controller.go

@@ -51,7 +51,7 @@ type downloadRequestController struct {
 	kbClient              client.Client
 	backupLister          velerov1listers.BackupLister
 	newPluginManager      func(logrus.FieldLogger) clientmgmt.Manager
-	newBackupStore        func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error)
+	backupStoreGetter     persistence.ObjectBackupStoreGetter
 }
 
 // NewDownloadRequestController creates a new DownloadRequestController.
@@ -62,6 +62,7 @@ func NewDownloadRequestController(
 	kbClient client.Client,
 	backupLister velerov1listers.BackupLister,
 	newPluginManager func(logrus.FieldLogger) clientmgmt.Manager,
+	backupStoreGetter persistence.ObjectBackupStoreGetter,
 	logger logrus.FieldLogger,
 ) Interface {
 	c := &downloadRequestController{
@@ -74,8 +75,8 @@ func NewDownloadRequestController(
 
 		// use variables to refer to these functions so they can be
 		// replaced with fakes for testing.
-		newPluginManager: newPluginManager,
-		newBackupStore:   persistence.NewObjectBackupStore,
+		newPluginManager:  newPluginManager,
+		backupStoreGetter: backupStoreGetter,
 
 		clock: &clock.RealClock{},
 	}
@@ -172,7 +173,7 @@ func (c *downloadRequestController) generatePreSignedURL(downloadRequest *velero
 	pluginManager := c.newPluginManager(log)
 	defer pluginManager.CleanupClients()
 
-	backupStore, err := c.newBackupStore(backupLocation, pluginManager, log)
+	backupStore, err := c.backupStoreGetter.Get(backupLocation, pluginManager, log)
 	if err != nil {
 		return errors.WithStack(err)
 	}

pkg/controller/download_request_controller_test.go

@@ -34,7 +34,6 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/builder"
 	"github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/fake"
 	informers "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions"
-	"github.com/vmware-tanzu/velero/pkg/persistence"
 	persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
 	pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks"
@@ -64,6 +63,7 @@ func newDownloadRequestTestHarness(t *testing.T, fakeClient client.Client) *down
 			fakeClient,
 			informerFactory.Velero().V1().Backups().Lister(),
 			func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+			NewFakeSingleObjectBackupStoreGetter(backupStore),
 			velerotest.NewLogger(),
 		).(*downloadRequestController)
 	)
@@ -72,10 +72,6 @@ func newDownloadRequestTestHarness(t *testing.T, fakeClient client.Client) *down
 	require.NoError(t, err)
 	controller.clock = clock.NewFakeClock(clockTime)
 
-	controller.newBackupStore = func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) {
-		return backupStore, nil
-	}
-
 	pluginManager.On("CleanupClients").Return()
 
 	return &downloadRequestTestHarness{

pkg/controller/restore_controller.go

@@ -92,8 +92,8 @@ type restoreController struct {
 	logFormat              logging.Format
 	clock                  clock.Clock
 
-	newPluginManager func(logger logrus.FieldLogger) clientmgmt.Manager
-	newBackupStore   func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error)
+	newPluginManager  func(logger logrus.FieldLogger) clientmgmt.Manager
+	backupStoreGetter persistence.ObjectBackupStoreGetter
 }
 
 func NewRestoreController(
@@ -108,6 +108,7 @@ func NewRestoreController(
 	logger logrus.FieldLogger,
 	restoreLogLevel logrus.Level,
 	newPluginManager func(logrus.FieldLogger) clientmgmt.Manager,
+	backupStoreGetter persistence.ObjectBackupStoreGetter,
 	metrics *metrics.ServerMetrics,
 	logFormat logging.Format,
 ) Interface {
@@ -128,8 +129,8 @@ func NewRestoreController(
 
 		// use variables to refer to these functions so they can be
 		// replaced with fakes for testing.
-		newPluginManager: newPluginManager,
-		newBackupStore:   persistence.NewObjectBackupStore,
+		newPluginManager:  newPluginManager,
+		backupStoreGetter: backupStoreGetter,
 	}
 
 	c.syncHandler = c.processQueueItem
@@ -410,7 +411,7 @@ func (c *restoreController) fetchBackupInfo(backupName string, pluginManager cli
 		return backupInfo{}, errors.WithStack(err)
 	}
 
-	backupStore, err := c.newBackupStore(location, pluginManager, c.logger)
+	backupStore, err := c.backupStoreGetter.Get(location, pluginManager, c.logger)
 	if err != nil {
 		return backupInfo{}, err
 	}
@@ -480,7 +481,7 @@ func (c *restoreController) runValidatedRestore(restore *api.Restore, info backu
 
 	// re-instantiate the backup store because credentials could have changed since the original
 	// instantiation, if this was a long-running restore
-	info.backupStore, err = c.newBackupStore(info.location, pluginManager, c.logger)
+	info.backupStore, err = c.backupStoreGetter.Get(info.location, pluginManager, c.logger)
 	if err != nil {
 		return errors.Wrap(err, "error setting up backup store to persist log and results files")
 	}

pkg/controller/restore_controller_test.go

@@ -117,14 +117,11 @@ func TestFetchBackupInfo(t *testing.T) {
 				logger,
 				logrus.InfoLevel,
 				func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+				NewFakeSingleObjectBackupStoreGetter(backupStore),
 				metrics.NewServerMetrics(),
 				formatFlag,
 			).(*restoreController)
 
-			c.newBackupStore = func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) {
-				return backupStore, nil
-			}
-
 			if test.backupStoreError == nil {
 				for _, itm := range test.informerLocations {
 					require.NoError(t, fakeClient.Create(context.Background(), itm))
@@ -212,6 +209,7 @@ func TestProcessQueueItemSkips(t *testing.T) {
 				logger,
 				logrus.InfoLevel,
 				nil,
+				persistence.NewObjectBackupStoreGetter(),
 				metrics.NewServerMetrics(),
 				formatFlag,
 			).(*restoreController)
@@ -438,13 +436,11 @@ func TestProcessQueueItem(t *testing.T) {
 				logger,
 				logrus.InfoLevel,
 				func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+				NewFakeSingleObjectBackupStoreGetter(backupStore),
 				metrics.NewServerMetrics(),
 				formatFlag,
 			).(*restoreController)
 
-			c.newBackupStore = func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) {
-				return backupStore, nil
-			}
 			c.clock = clock.NewFakeClock(now)
 			if test.location != nil {
 				require.NoError(t, fakeClient.Create(context.Background(), test.location))
@@ -670,6 +666,7 @@ func TestvalidateAndCompleteWhenScheduleNameSpecified(t *testing.T) {
 		logger,
 		logrus.DebugLevel,
 		nil,
+		persistence.NewObjectBackupStoreGetter(),
 		nil,
 		formatFlag,
 	).(*restoreController)

pkg/controller/suite_test.go

@@ -22,6 +22,11 @@ import (
 	"testing"
 	"time"
 
+	"github.com/sirupsen/logrus"
+
+	"github.com/vmware-tanzu/velero/pkg/persistence"
+	persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -133,3 +138,31 @@ func newFakeClient(t *testing.T, initObjs ...runtime.Object) client.Client {
 	require.NoError(t, err)
 	return fake.NewFakeClientWithScheme(scheme.Scheme, initObjs...)
 }
+
+type fakeSingleObjectBackupStoreGetter struct {
+	store persistence.BackupStore
+}
+
+func (f *fakeSingleObjectBackupStoreGetter) Get(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) {
+	return f.store, nil
+}
+
+// NewFakeSingleObjectBackupStoreGetter returns an ObjectBackupStoreGetter
+// that will return only the given BackupStore.
+func NewFakeSingleObjectBackupStoreGetter(store persistence.BackupStore) persistence.ObjectBackupStoreGetter {
+	return &fakeSingleObjectBackupStoreGetter{store: store}
+}
+
+type fakeObjectBackupStoreGetter struct {
+	stores map[string]*persistencemocks.BackupStore
+}
+
+func (f *fakeObjectBackupStoreGetter) Get(loc *velerov1api.BackupStorageLocation, _ persistence.ObjectStoreGetter, _ logrus.FieldLogger) (persistence.BackupStore, error) {
+	return f.stores[loc.Name], nil
+}
+
+// NewFakeObjectBackupStoreGetter returns an ObjectBackupStoreGetter that will
+// return the BackupStore for a given BackupStorageLocation name.
+func NewFakeObjectBackupStoreGetter(stores map[string]*persistencemocks.BackupStore) persistence.ObjectBackupStoreGetter {
+	return &fakeObjectBackupStoreGetter{stores: stores}
+}

pkg/persistence/object_store.go

@@ -90,7 +90,21 @@ type ObjectStoreGetter interface {
 	GetObjectStore(provider string) (velero.ObjectStore, error)
 }
 
-func NewObjectBackupStore(location *velerov1api.BackupStorageLocation, objectStoreGetter ObjectStoreGetter, logger logrus.FieldLogger) (BackupStore, error) {
+// ObjectBackupStoreGetter is a type that can get a velero.BackupStore for a
+// given BackupStorageLocation and ObjectStore.
+type ObjectBackupStoreGetter interface {
+	Get(location *velerov1api.BackupStorageLocation, objectStoreGetter ObjectStoreGetter, logger logrus.FieldLogger) (BackupStore, error)
+}
+
+type objectBackupStoreGetter struct{}
+
+// NewObjectBackupStoreGetter returns a ObjectBackupStoreGetter that can get a
+// default velero.BackupStore.
+func NewObjectBackupStoreGetter() ObjectBackupStoreGetter {
+	return &objectBackupStoreGetter{}
+}
+
+func (b *objectBackupStoreGetter) Get(location *velerov1api.BackupStorageLocation, objectStoreGetter ObjectStoreGetter, logger logrus.FieldLogger) (BackupStore, error) {
 	if location.Spec.ObjectStorage == nil {
 		return nil, errors.New("backup storage location does not use object storage")
 	}

pkg/persistence/object_store_test.go

@@ -604,10 +604,10 @@ func (osg objectStoreGetter) GetObjectStore(provider string) (velero.ObjectStore
 	return res, nil
 }
 
-// TestNewObjectBackupStore runs the NewObjectBackupStore constructor and ensures
-// that an ObjectBackupStore is constructed correctly or an appropriate error is
-// returned.
-func TestNewObjectBackupStore(t *testing.T) {
+// TestNewObjectBackupStore runs the NewObjectBackupStoreGetter constructor and ensures
+// that it provides a BackupStore with a correctly constructed ObjectBackupStore or
+// that an appropriate error is returned.
+func TestNewObjectBackupStoreGetter(t *testing.T) {
 	tests := []struct {
 		name              string
 		location          *velerov1api.BackupStorageLocation
@@ -661,7 +661,8 @@ func TestNewObjectBackupStore(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			res, err := NewObjectBackupStore(tc.location, tc.objectStoreGetter, velerotest.NewLogger())
+			getter := NewObjectBackupStoreGetter()
+			res, err := getter.Get(tc.location, tc.objectStoreGetter, velerotest.NewLogger())
 			if tc.wantErr != "" {
 				require.Equal(t, tc.wantErr, err.Error())
 			} else {