Use Credential from BSL for restic commands

This change introduces support for restic to make use of per-BSL credentials. It makes use of the `credentials.FileStore` introduced in PR vmware-tanzu#3442 to write the BSL credentials to disk. To support per-BSL credentials for restic, the environment for the restic commands needs to be modified for each provider to ensure that the credentials are provided via the correct provider specific environment variables. This change introduces a new function `restic.CmdEnv` to check the BSL provider and create the correct mapping of environment variables for each provider. Previously, AWS and GCP could rely on the environment variables in the Velero deployments to obtain the credentials file, but now these environment variables need to be set with the path to the serialized credentials file if a credential is set on the BSL. For Azure, the credentials file is loaded and parsed to set the environment variables for restic. Now, we check if the BSL has a credential, and if it does, load and parse that file instead. This change also introduces a few other small improvements. Now that we are fetching the BSL to check for the `Credential` field, we can use the BSL directly to get the `CACert` which means that we can remove the `GetCACert` function. Also, now that we have a way to serialize secrets to disk, we can use the `credentials.FileStore` to get a temp file for the restic repo password and remove the `restic.TempCredentialsFile` function. Signed-off-by: Bridget McErlean <[email protected]>
zubron · Feb 22, 2021 · 47fb8c2 · 47fb8c2
1 parent 5c9fe63
commit 47fb8c2
Show file tree

Hide file tree

Showing 14 changed files with 271 additions and 287 deletions.
diff --git a/changelogs/unreleased/3489-zubron b/changelogs/unreleased/3489-zubron
@@ -0,0 +1 @@
+Add support for restic to use per-BSL credentials. Velero will now serialize the secret referenced by the `Credential` field in the BSL and use this path when setting provider specific environment variables for restic commands. 
diff --git a/pkg/cmd/cli/restic/server.go b/pkg/cmd/cli/restic/server.go
@@ -47,6 +47,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/cmd"
 	"github.com/vmware-tanzu/velero/pkg/cmd/util/signals"
 	"github.com/vmware-tanzu/velero/pkg/controller"
+	"github.com/vmware-tanzu/velero/pkg/credentials"
 	clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned"
 	informers "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions"
 	"github.com/vmware-tanzu/velero/pkg/util/filesystem"
@@ -108,6 +109,7 @@ type resticServer struct {
 	mgr                   manager.Manager
 	metrics               *metrics.ServerMetrics
 	metricsAddress        string
+	namespace             string
 }
 
 func newResticServer(logger logrus.FieldLogger, factory client.Factory, metricAddress string) (*resticServer, error) {
@@ -164,6 +166,7 @@ func newResticServer(logger logrus.FieldLogger, factory client.Factory, metricAd
 		fileSystem:            filesystem.NewFileSystem(),
 		mgr:                   mgr,
 		metricsAddress:        metricAddress,
+		namespace:             factory.Namespace(),
 	}
 
 	if err := s.validatePodVolumesHostPath(); err != nil {
@@ -190,6 +193,11 @@ func (s *resticServer) run() {
 
 	s.logger.Info("Starting controllers")
 
+	credentialFileStore, err := credentials.NewNamespacedFileStore(s.mgr.GetClient(), s.namespace, "/tmp/credentials/", filesystem.NewFileSystem())
+	if err != nil {
+		s.logger.Fatalf("Failed to create credentials file store: %v", err)
+	}
+
 	backupController := controller.NewPodVolumeBackupController(
 		s.logger,
 		s.veleroInformerFactory.Velero().V1().PodVolumeBackups(),
@@ -200,6 +208,7 @@ func (s *resticServer) run() {
 		s.metrics,
 		s.mgr.GetClient(),
 		os.Getenv("NODE_NAME"),
+		credentialFileStore,
 	)
 
 	restoreController := controller.NewPodVolumeRestoreController(
@@ -211,6 +220,7 @@ func (s *resticServer) run() {
 		s.kubeInformerFactory.Core().V1().PersistentVolumes(),
 		s.mgr.GetClient(),
 		os.Getenv("NODE_NAME"),
+		credentialFileStore,
 	)
 
 	go s.veleroInformerFactory.Start(s.ctx.Done())

diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go
@@ -231,6 +231,7 @@ type server struct {
 	metrics                             *metrics.ServerMetrics
 	config                              serverConfig
 	mgr                                 manager.Manager
+	credentialFileStore                 credentials.FileStore
 }
 
 func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*server, error) {
@@ -297,6 +298,12 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s
 		return nil, err
 	}
 
+	credentialFileStore, err := credentials.NewNamespacedFileStore(mgr.GetClient(), f.Namespace(), "/tmp/credentials/", filesystem.NewFileSystem())
+	if err != nil {
+		cancelFunc()
+		return nil, err
+	}
+
 	s := &server{
 		namespace:                           f.Namespace(),
 		metricsAddress:                      config.metricsAddress,
@@ -315,6 +322,7 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s
 		pluginRegistry:                      pluginRegistry,
 		config:                              config,
 		mgr:                                 mgr,
+		credentialFileStore:                 credentialFileStore,
 	}
 
 	return s, nil
@@ -495,6 +503,7 @@ func (s *server) initRestic() error {
 		s.mgr.GetClient(),
 		s.kubeClient.CoreV1(),
 		s.kubeClient.CoreV1(),
+		s.credentialFileStore,
 		s.logger,
 	)
 	if err != nil {
@@ -555,10 +564,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 		return clientmgmt.NewManager(logger, s.logLevel, s.pluginRegistry)
 	}
 
-	credentialFileStore, err := credentials.NewNamespacedFileStore(s.mgr.GetClient(), s.namespace, "/tmp/credentials/", filesystem.NewFileSystem())
-	cmd.CheckError(err)
-
-	backupStoreGetter := persistence.NewObjectBackupStoreGetter(credentialFileStore)
+	backupStoreGetter := persistence.NewObjectBackupStoreGetter(s.credentialFileStore)
 
 	csiVSLister, csiVSCLister := s.getCSISnapshotListers()
 

diff --git a/pkg/controller/pod_volume_backup_controller.go b/pkg/controller/pod_volume_backup_controller.go
@@ -38,6 +38,7 @@ import (
 	"k8s.io/client-go/tools/cache"
 
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/credentials"
 	velerov1client "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1"
 	informers "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions/velero/v1"
 	listers "github.com/vmware-tanzu/velero/pkg/generated/listers/velero/v1"
@@ -60,6 +61,7 @@ type podVolumeBackupController struct {
 	kbClient              client.Client
 	nodeName              string
 	metrics               *metrics.ServerMetrics
+	credentialsFileStore  credentials.FileStore
 
 	processBackupFunc func(*velerov1api.PodVolumeBackup) error
 	fileSystem        filesystem.Interface
@@ -77,6 +79,7 @@ func NewPodVolumeBackupController(
 	metrics *metrics.ServerMetrics,
 	kbClient client.Client,
 	nodeName string,
+	credentialsFileStore credentials.FileStore,
 ) Interface {
 	c := &podVolumeBackupController{
 		genericController:     newGenericController(PodVolumeBackup, logger),
@@ -88,6 +91,7 @@ func NewPodVolumeBackupController(
 		kbClient:              kbClient,
 		nodeName:              nodeName,
 		metrics:               metrics,
+		credentialsFileStore:  credentialsFileStore,
 
 		fileSystem: filesystem.NewFileSystem(),
 		clock:      &clock.RealClock{},
@@ -221,7 +225,7 @@ func (c *podVolumeBackupController) processBackup(req *velerov1api.PodVolumeBack
 	log.WithField("path", path).Debugf("Found path matching glob")
 
 	// temp creds
-	credentialsFile, err := restic.TempCredentialsFile(c.kbClient, req.Namespace, c.fileSystem)
+	credentialsFile, err := c.credentialsFileStore.Get(restic.RepoKeySelector())
 	if err != nil {
 		log.WithError(err).Error("Error creating temp restic credentials file")
 		return c.fail(req, errors.Wrap(err, "error creating temp restic credentials file").Error(), log)
@@ -236,15 +240,18 @@ func (c *podVolumeBackupController) processBackup(req *velerov1api.PodVolumeBack
 		req.Spec.Tags,
 	)
 
-	// if there's a caCert on the ObjectStorage, write it to disk so that it can be passed to restic
-	caCert, err := restic.GetCACert(c.kbClient, req.Namespace, req.Spec.BackupStorageLocation)
-	if err != nil {
-		log.WithError(err).Error("Error getting caCert")
+	backupLocation := &velerov1api.BackupStorageLocation{}
+	if err := c.kbClient.Get(context.Background(), client.ObjectKey{
+		Namespace: req.Namespace,
+		Name:      req.Spec.BackupStorageLocation,
+	}, backupLocation); err != nil {
+		return c.fail(req, errors.Wrap(err, "error getting backup storage location").Error(), log)
 	}
 
+	// if there's a caCert on the ObjectStorage, write it to disk so that it can be passed to restic
 	var caCertFile string
-	if caCert != nil {
-		caCertFile, err = restic.TempCACertFile(caCert, req.Spec.BackupStorageLocation, c.fileSystem)
+	if backupLocation.Spec.ObjectStorage != nil && backupLocation.Spec.ObjectStorage.CACert != nil {
+		caCertFile, err = restic.TempCACertFile(backupLocation.Spec.ObjectStorage.CACert, req.Spec.BackupStorageLocation, c.fileSystem)
 		if err != nil {
 			log.WithError(err).Error("Error creating temp cacert file")
 		}
@@ -253,20 +260,11 @@ func (c *podVolumeBackupController) processBackup(req *velerov1api.PodVolumeBack
 	}
 	resticCmd.CACertFile = caCertFile
 
-	// Running restic command might need additional provider specific environment variables. Based on the provider, we
-	// set resticCmd.Env appropriately (currently for Azure and S3 based backuplocations)
-	var env []string
-	if strings.HasPrefix(req.Spec.RepoIdentifier, "azure") {
-		if env, err = restic.AzureCmdEnv(c.kbClient, req.Namespace, req.Spec.BackupStorageLocation); err != nil {
-			return c.fail(req, errors.Wrap(err, "error setting restic cmd env").Error(), log)
-		}
-		resticCmd.Env = env
-	} else if strings.HasPrefix(req.Spec.RepoIdentifier, "s3") {
-		if env, err = restic.S3CmdEnv(c.kbClient, req.Namespace, req.Spec.BackupStorageLocation); err != nil {
-			return c.fail(req, errors.Wrap(err, "error setting restic cmd env").Error(), log)
-		}
-		resticCmd.Env = env
+	env, err := restic.CmdEnv(backupLocation, c.credentialsFileStore)
+	if err != nil {
+		return c.fail(req, errors.Wrap(err, "error setting restic cmd env").Error(), log)
 	}
+	resticCmd.Env = env
 
 	// If this is a PVC, look for the most recent completed pod volume backup for it and get
 	// its restic snapshot ID to use as the value of the `--parent` flag. Without this,
@@ -298,7 +296,11 @@ func (c *podVolumeBackupController) processBackup(req *velerov1api.PodVolumeBack
 
 	var snapshotID string
 	if !emptySnapshot {
-		snapshotID, err = restic.GetSnapshotID(req.Spec.RepoIdentifier, credentialsFile, req.Spec.Tags, env, caCertFile)
+		cmd := restic.GetSnapshotCommand(req.Spec.RepoIdentifier, credentialsFile, req.Spec.Tags)
+		cmd.Env = env
+		cmd.CACertFile = caCertFile
+
+		snapshotID, err = restic.GetSnapshotID(cmd)
 		if err != nil {
 			log.WithError(err).Error("Error getting SnapshotID")
 			return c.fail(req, errors.Wrap(err, "error getting snapshot id").Error(), log)

diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go
@@ -23,7 +23,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/pkg/errors"
@@ -41,6 +40,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/credentials"
 	velerov1client "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1"
 	informers "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions/velero/v1"
 	listers "github.com/vmware-tanzu/velero/pkg/generated/listers/velero/v1"
@@ -61,6 +61,7 @@ type podVolumeRestoreController struct {
 	backupLocationInformer k8scache.Informer
 	kbClient               client.Client
 	nodeName               string
+	credentialsFileStore   credentials.FileStore
 
 	processRestoreFunc func(*velerov1api.PodVolumeRestore) error
 	fileSystem         filesystem.Interface
@@ -77,6 +78,7 @@ func NewPodVolumeRestoreController(
 	pvInformer corev1informers.PersistentVolumeInformer,
 	kbClient client.Client,
 	nodeName string,
+	credentialsFileStore credentials.FileStore,
 ) Interface {
 	c := &podVolumeRestoreController{
 		genericController:      newGenericController(PodVolumeRestore, logger),
@@ -87,6 +89,7 @@ func NewPodVolumeRestoreController(
 		pvLister:               pvInformer.Lister(),
 		kbClient:               kbClient,
 		nodeName:               nodeName,
+		credentialsFileStore:   credentialsFileStore,
 
 		fileSystem: filesystem.NewFileSystem(),
 		clock:      &clock.RealClock{},
@@ -300,32 +303,8 @@ func (c *podVolumeRestoreController) processRestore(req *velerov1api.PodVolumeRe
 		return c.failRestore(req, errors.Wrap(err, "error getting volume directory name").Error(), log)
 	}
 
-	credsFile, err := restic.TempCredentialsFile(c.kbClient, req.Namespace, c.fileSystem)
-	if err != nil {
-		log.WithError(err).Error("Error creating temp restic credentials file")
-		return c.failRestore(req, errors.Wrap(err, "error creating temp restic credentials file").Error(), log)
-	}
-	// ignore error since there's nothing we can do and it's a temp file.
-	defer os.Remove(credsFile)
-
-	// if there's a caCert on the ObjectStorage, write it to disk so that it can be passed to restic
-	caCert, err := restic.GetCACert(c.kbClient, req.Namespace, req.Spec.BackupStorageLocation)
-	if err != nil {
-		log.WithError(err).Error("Error getting caCert")
-	}
-
-	var caCertFile string
-	if caCert != nil {
-		caCertFile, err = restic.TempCACertFile(caCert, req.Spec.BackupStorageLocation, c.fileSystem)
-		if err != nil {
-			log.WithError(err).Error("Error creating temp cacert file")
-		}
-		// ignore error since there's nothing we can do and it's a temp file.
-		defer os.Remove(caCertFile)
-	}
-
 	// execute the restore process
-	if err := c.restorePodVolume(req, credsFile, caCertFile, volumeDir, log); err != nil {
+	if err := c.restorePodVolume(req, volumeDir, log); err != nil {
 		log.WithError(err).Error("Error restoring volume")
 		return c.failRestore(req, errors.Wrap(err, "error restoring volume").Error(), log)
 	}
@@ -344,37 +323,54 @@ func (c *podVolumeRestoreController) processRestore(req *velerov1api.PodVolumeRe
 	return nil
 }
 
-func (c *podVolumeRestoreController) restorePodVolume(req *velerov1api.PodVolumeRestore, credsFile, caCertFile, volumeDir string, log logrus.FieldLogger) error {
+func (c *podVolumeRestoreController) restorePodVolume(req *velerov1api.PodVolumeRestore, volumeDir string, log logrus.FieldLogger) error {
 	// Get the full path of the new volume's directory as mounted in the daemonset pod, which
 	// will look like: /host_pods/<new-pod-uid>/volumes/<volume-plugin-name>/<volume-dir>
 	volumePath, err := singlePathMatch(fmt.Sprintf("/host_pods/%s/volumes/*/%s", string(req.Spec.Pod.UID), volumeDir))
 	if err != nil {
 		return errors.Wrap(err, "error identifying path of volume")
 	}
 
+	credsFile, err := c.credentialsFileStore.Get(restic.RepoKeySelector())
+	if err != nil {
+		log.WithError(err).Error("Error creating temp restic credentials file")
+		return c.failRestore(req, errors.Wrap(err, "error creating temp restic credentials file").Error(), log)
+	}
+	// ignore error since there's nothing we can do and it's a temp file.
+	defer os.Remove(credsFile)
+
 	resticCmd := restic.RestoreCommand(
 		req.Spec.RepoIdentifier,
 		credsFile,
 		req.Spec.SnapshotID,
 		volumePath,
 	)
-	resticCmd.CACertFile = caCertFile
 
-	// Running restic command might need additional provider specific environment variables. Based on the provider, we
-	// set resticCmd.Env appropriately (currently for Azure and S3 based backuplocations)
-	if strings.HasPrefix(req.Spec.RepoIdentifier, "azure") {
-		env, err := restic.AzureCmdEnv(c.kbClient, req.Namespace, req.Spec.BackupStorageLocation)
-		if err != nil {
-			return c.failRestore(req, errors.Wrap(err, "error setting restic cmd env").Error(), log)
-		}
-		resticCmd.Env = env
-	} else if strings.HasPrefix(req.Spec.RepoIdentifier, "s3") {
-		env, err := restic.S3CmdEnv(c.kbClient, req.Namespace, req.Spec.BackupStorageLocation)
+	backupLocation := &velerov1api.BackupStorageLocation{}
+	if err := c.kbClient.Get(context.Background(), client.ObjectKey{
+		Namespace: req.Namespace,
+		Name:      req.Spec.BackupStorageLocation,
+	}, backupLocation); err != nil {
+		return c.failRestore(req, errors.Wrap(err, "error getting backup storage location").Error(), log)
+	}
+
+	// if there's a caCert on the ObjectStorage, write it to disk so that it can be passed to restic
+	var caCertFile string
+	if backupLocation.Spec.ObjectStorage != nil && backupLocation.Spec.ObjectStorage.CACert != nil {
+		caCertFile, err = restic.TempCACertFile(backupLocation.Spec.ObjectStorage.CACert, req.Spec.BackupStorageLocation, c.fileSystem)
 		if err != nil {
-			return c.failRestore(req, errors.Wrap(err, "error setting restic cmd env").Error(), log)
+			log.WithError(err).Error("Error creating temp cacert file")
 		}
-		resticCmd.Env = env
+		// ignore error since there's nothing we can do and it's a temp file.
+		defer os.Remove(caCertFile)
+	}
+	resticCmd.CACertFile = caCertFile
+
+	env, err := restic.CmdEnv(backupLocation, c.credentialsFileStore)
+	if err != nil {
+		return c.failRestore(req, errors.Wrap(err, "error setting restic cmd env").Error(), log)
 	}
+	resticCmd.Env = env
 
 	var stdout, stderr string
 

diff --git a/pkg/restic/aws.go b/pkg/restic/aws.go
@@ -1,5 +1,5 @@
 /*
-Copyright 2019 the Velero contributors.
+Copyright the Velero contributors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -18,8 +18,9 @@ package restic
 
 const (
 	// AWS specific environment variable
-	awsProfileEnvVar = "AWS_PROFILE"
-	awsProfileKey    = "profile"
+	awsProfileEnvVar         = "AWS_PROFILE"
+	awsProfileKey            = "profile"
+	awsCredentialsFileEnvVar = "AWS_SHARED_CREDENTIALS_FILE"
 )
 
 // getS3ResticEnvVars gets the environment variables that restic
@@ -28,6 +29,10 @@ const (
 func getS3ResticEnvVars(config map[string]string) (map[string]string, error) {
 	result := make(map[string]string)
 
+	if credentialsFile, ok := config[credentialsFileKey]; ok {
+		result[awsCredentialsFileEnvVar] = credentialsFile
+	}
+
 	if profile, ok := config[awsProfileKey]; ok {
 		result[awsProfileEnvVar] = profile
 	}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Add support for restic to use per-BSL credentials. Velero will now serialize the secret referenced by the `Credential` field in the BSL and use this path when setting provider specific environment variables for restic commands.