Merge pull request #4864 from sseago/vsl-creds-main

Add credentials to volume snapshot locations.

changelogs/unreleased/4864-sseago

@@ -0,0 +1 @@
+Add credentials to volume snapshot locations

config/crd/v1/bases/velero.io_volumesnapshotlocations.yaml

@@ -45,6 +45,24 @@ spec:
                   type: string
                 description: Config is for provider-specific configuration fields.
                 type: object
+              credential:
+                description: Credential contains the credential information intended
+                  to be used with this location
+                properties:
+                  key:
+                    description: The key of the secret to select from.  Must be a
+                      valid secret key.
+                    type: string
+                  name:
+                    description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                      TODO: Add other useful fields. apiVersion, kind, uid?'
+                    type: string
+                  optional:
+                    description: Specify whether the Secret or its key must be defined
+                    type: boolean
+                required:
+                - key
+                type: object
               provider:
                 description: Provider is the provider of the volume storage.
                 type: string

pkg/apis/velero/v1/volume_snapshot_location_type.go

@@ -16,7 +16,10 @@ limitations under the License.
 
 package v1
 
-import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+import (
+	corev1api "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
 
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -61,6 +64,10 @@ type VolumeSnapshotLocationSpec struct {
 	// Config is for provider-specific configuration fields.
 	// +optional
 	Config map[string]string `json:"config,omitempty"`
+
+	// Credential contains the credential information intended to be used with this location
+	// +optional
+	Credential *corev1api.SecretKeySelector `json:"credential,omitempty"`
 }
 
 // VolumeSnapshotLocationPhase is the lifecycle phase of a Velero VolumeSnapshotLocation.

pkg/builder/volume_snapshot_location_builder.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.
@@ -19,6 +19,8 @@ package builder
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	corev1api "k8s.io/api/core/v1"
+
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 )
 
@@ -62,3 +64,9 @@ func (b *VolumeSnapshotLocationBuilder) Provider(name string) *VolumeSnapshotLoc
 	b.object.Spec.Provider = name
 	return b
 }
+
+// Credential sets the VolumeSnapshotLocation's credential selector.
+func (b *VolumeSnapshotLocationBuilder) Credential(selector *corev1api.SecretKeySelector) *VolumeSnapshotLocationBuilder {
+	b.object.Spec.Credential = selector
+	return b
+}

pkg/cmd/cli/snapshotlocation/create.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2020 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.
@@ -26,6 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/builder"
 	"github.com/vmware-tanzu/velero/pkg/client"
 	"github.com/vmware-tanzu/velero/pkg/cmd"
 	"github.com/vmware-tanzu/velero/pkg/cmd/util/flag"
@@ -54,23 +55,26 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command {
 }
 
 type CreateOptions struct {
-	Name     string
-	Provider string
-	Config   flag.Map
-	Labels   flag.Map
+	Name       string
+	Provider   string
+	Config     flag.Map
+	Labels     flag.Map
+	Credential flag.Map
 }
 
 func NewCreateOptions() *CreateOptions {
 	return &CreateOptions{
-		Config: flag.NewMap(),
-		Labels: flag.NewMap(),
+		Config:     flag.NewMap(),
+		Labels:     flag.NewMap(),
+		Credential: flag.NewMap(),
 	}
 }
 
 func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) {
 	flags.StringVar(&o.Provider, "provider", o.Provider, "Name of the volume snapshot provider (e.g. aws, azure, gcp).")
 	flags.Var(&o.Config, "config", "Configuration key-value pairs.")
 	flags.Var(&o.Labels, "labels", "Labels to apply to the volume snapshot location.")
+	flags.Var(&o.Credential, "credential", "The credential to be used by this location as a key-value pair, where the key is the Kubernetes Secret name, and the value is the data key name within the Secret. Optional, one value only.")
 }
 
 func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Factory) error {
@@ -82,6 +86,10 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto
 		return errors.New("--provider is required")
 	}
 
+	if len(o.Credential.Data()) > 1 {
+		return errors.New("--credential can only contain 1 key/value pair")
+	}
+
 	return nil
 }
 
@@ -90,10 +98,10 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error {
 	return nil
 }
 
-func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
+func (o *CreateOptions) BuildVolumeSnapshotLocation(namespace string) *api.VolumeSnapshotLocation {
 	volumeSnapshotLocation := &api.VolumeSnapshotLocation{
 		ObjectMeta: metav1.ObjectMeta{
-			Namespace: f.Namespace(),
+			Namespace: namespace,
 			Name:      o.Name,
 			Labels:    o.Labels.Data(),
 		},
@@ -102,6 +110,15 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
 			Config:   o.Config.Data(),
 		},
 	}
+	for secretName, secretKey := range o.Credential.Data() {
+		volumeSnapshotLocation.Spec.Credential = builder.ForSecretKeySelector(secretName, secretKey).Result()
+		break
+	}
+	return volumeSnapshotLocation
+}
+
+func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
+	volumeSnapshotLocation := o.BuildVolumeSnapshotLocation(f.Namespace())
 
 	if printed, err := output.PrintWithFormat(c, volumeSnapshotLocation); printed || err != nil {
 		return err

pkg/cmd/cli/snapshotlocation/set.go

@@ -27,8 +27,10 @@ import (
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/builder"
 	"github.com/vmware-tanzu/velero/pkg/client"
 	"github.com/vmware-tanzu/velero/pkg/cmd"
+	"github.com/vmware-tanzu/velero/pkg/cmd/util/flag"
 	"github.com/vmware-tanzu/velero/pkg/cmd/util/output"
 )
 
@@ -39,9 +41,6 @@ func NewSetCommand(f client.Factory, use string) *cobra.Command {
 		Use:   use + " NAME",
 		Short: "Set specific features for a snapshot location",
 		Args:  cobra.ExactArgs(1),
-		// Mark this command as hidden until more functionality is added
-		// as part of https://github.com/vmware-tanzu/velero/issues/2426
-		Hidden: true,
 		Run: func(c *cobra.Command, args []string) {
 			cmd.CheckError(o.Complete(args, f))
 			cmd.CheckError(o.Validate(c, args, f))
@@ -54,21 +53,29 @@ func NewSetCommand(f client.Factory, use string) *cobra.Command {
 }
 
 type SetOptions struct {
-	Name string
+	Name       string
+	Credential flag.Map
 }
 
 func NewSetOptions() *SetOptions {
-	return &SetOptions{}
+	return &SetOptions{
+		Credential: flag.NewMap(),
+	}
 }
 
-func (o *SetOptions) BindFlags(*pflag.FlagSet) {
+func (o *SetOptions) BindFlags(flags *pflag.FlagSet) {
+	flags.Var(&o.Credential, "credential", "Sets the credential to be used by this location as a key-value pair, where the key is the Kubernetes Secret name, and the value is the data key name within the Secret. Optional, one value only.")
 }
 
 func (o *SetOptions) Validate(c *cobra.Command, args []string, f client.Factory) error {
 	if err := output.ValidateFlags(c); err != nil {
 		return err
 	}
 
+	if len(o.Credential.Data()) > 1 {
+		return errors.New("--credential can only contain 1 key/value pair")
+	}
+
 	return nil
 }
 
@@ -92,6 +99,11 @@ func (o *SetOptions) Run(c *cobra.Command, f client.Factory) error {
 		return errors.WithStack(err)
 	}
 
+	for name, key := range o.Credential.Data() {
+		location.Spec.Credential = builder.ForSecretKeySelector(name, key).Result()
+		break
+	}
+
 	if err := kbClient.Update(context.Background(), location, &kbclient.UpdateOptions{}); err != nil {
 		return errors.WithStack(err)
 	}

pkg/cmd/server/server.go

@@ -650,6 +650,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			csiVSCLister,
 			csiVSClassLister,
 			backupStoreGetter,
+			s.credentialFileStore,
 		)
 
 		return controllerRunInfo{
@@ -672,6 +673,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 			s.logger,
 			podexec.NewPodCommandExecutor(s.kubeClientConfig, s.kubeClient.CoreV1().RESTClient()),
 			s.kubeClient.CoreV1().RESTClient(),
+			s.credentialFileStore,
 		)
 		cmd.CheckError(err)
 

pkg/controller/backup_controller.go

@@ -50,6 +50,7 @@ import (
 	snapshotterClientSet "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned"
 	snapshotv1listers "github.com/kubernetes-csi/external-snapshotter/client/v4/listers/volumesnapshot/v1"
 
+	"github.com/vmware-tanzu/velero/internal/credentials"
 	"github.com/vmware-tanzu/velero/internal/storage"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	pkgbackup "github.com/vmware-tanzu/velero/pkg/backup"
@@ -98,6 +99,7 @@ type backupController struct {
 	volumeSnapshotClient        *snapshotterClientSet.Clientset
 	volumeSnapshotContentLister snapshotv1listers.VolumeSnapshotContentLister
 	volumeSnapshotClassLister   snapshotv1listers.VolumeSnapshotClassLister
+	credentialFileStore         credentials.FileStore
 }
 
 func NewBackupController(
@@ -123,6 +125,7 @@ func NewBackupController(
 	volumeSnapshotContentLister snapshotv1listers.VolumeSnapshotContentLister,
 	volumesnapshotClassLister snapshotv1listers.VolumeSnapshotClassLister,
 	backupStoreGetter persistence.ObjectBackupStoreGetter,
+	credentialStore credentials.FileStore,
 ) Interface {
 	c := &backupController{
 		genericController:           newGenericController(Backup, logger),
@@ -148,6 +151,7 @@ func NewBackupController(
 		volumeSnapshotContentLister: volumeSnapshotContentLister,
 		volumeSnapshotClassLister:   volumesnapshotClassLister,
 		backupStoreGetter:           backupStoreGetter,
+		credentialFileStore:         credentialStore,
 	}
 
 	c.syncHandler = c.processBackup
@@ -566,6 +570,15 @@ func (c *backupController) validateAndGetSnapshotLocations(backup *velerov1api.B
 		return nil, errors
 	}
 
+	// add credential to config for each location
+	for _, location := range providerLocations {
+		err = volume.UpdateVolumeSnapshotLocationWithCredentialConfig(location, c.credentialFileStore, c.logger)
+		if err != nil {
+			errors = append(errors, fmt.Sprintf("error adding credentials to volume snapshot location named %s: %v", location.Name, err))
+			continue
+		}
+	}
+
 	return providerLocations, nil
 }
 

pkg/controller/backup_controller_test.go

@@ -1007,12 +1007,15 @@ func TestValidateAndGetSnapshotLocations(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			formatFlag := logging.FormatText
 			var (
 				client          = fake.NewSimpleClientset()
 				sharedInformers = informers.NewSharedInformerFactory(client, 0)
+				logger          = logging.DefaultLogger(logrus.DebugLevel, formatFlag)
 			)
 
 			c := &backupController{
+				genericController:        newGenericController("backup-test", logger),
 				snapshotLocationLister:   sharedInformers.Velero().V1().VolumeSnapshotLocations().Lister(),
 				defaultSnapshotLocations: test.defaultLocations,
 			}

pkg/restore/pv_restorer.go

@@ -21,6 +21,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
+	"github.com/vmware-tanzu/velero/internal/credentials"
 	api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	listers "github.com/vmware-tanzu/velero/pkg/generated/listers/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
@@ -39,6 +40,7 @@ type pvRestorer struct {
 	volumeSnapshots         []*volume.Snapshot
 	volumeSnapshotterGetter VolumeSnapshotterGetter
 	snapshotLocationLister  listers.VolumeSnapshotLocationLister
+	credentialFileStore     credentials.FileStore
 }
 
 func (r *pvRestorer) executePVAction(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) {
@@ -59,7 +61,7 @@ func (r *pvRestorer) executePVAction(obj *unstructured.Unstructured) (*unstructu
 
 	log := r.logger.WithFields(logrus.Fields{"persistentVolume": pvName})
 
-	snapshotInfo, err := getSnapshotInfo(pvName, r.backup, r.volumeSnapshots, r.snapshotLocationLister)
+	snapshotInfo, err := getSnapshotInfo(pvName, r.backup, r.volumeSnapshots, r.snapshotLocationLister, r.credentialFileStore, r.logger)
 	if err != nil {
 		return nil, err
 	}
@@ -103,7 +105,7 @@ type snapshotInfo struct {
 	location           *api.VolumeSnapshotLocation
 }
 
-func getSnapshotInfo(pvName string, backup *api.Backup, volumeSnapshots []*volume.Snapshot, snapshotLocationLister listers.VolumeSnapshotLocationLister) (*snapshotInfo, error) {
+func getSnapshotInfo(pvName string, backup *api.Backup, volumeSnapshots []*volume.Snapshot, snapshotLocationLister listers.VolumeSnapshotLocationLister, credentialStore credentials.FileStore, logger logrus.FieldLogger) (*snapshotInfo, error) {
 	var pvSnapshot *volume.Snapshot
 	for _, snapshot := range volumeSnapshots {
 		if snapshot.Spec.PersistentVolumeName == pvName {
@@ -120,6 +122,11 @@ func getSnapshotInfo(pvName string, backup *api.Backup, volumeSnapshots []*volum
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
+	// add credential to config
+	err = volume.UpdateVolumeSnapshotLocationWithCredentialConfig(loc, credentialStore, logger)
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
 
 	return &snapshotInfo{
 		providerSnapshotID: pvSnapshot.Status.ProviderSnapshotID,