From 635dd27e1a7dd179f7b22f0e72eb3ae95dbdfea4 Mon Sep 17 00:00:00 2001
From: Nolan Brubaker <brubakern@vmware.com>
Date: Thu, 1 Aug 2019 18:57:36 -0400
Subject: [PATCH] Make secret file optional on install (#1699)

* Make secret file optional on install

Fixes #1689

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>
---
 changelogs/unreleased/1699-nrb       |  1 +
 pkg/cmd/cli/install/install.go       | 33 +++++++++++------
 pkg/install/daemonset.go             | 29 ++++++++-------
 pkg/install/daemonset_test.go        |  7 ++--
 pkg/install/deployment.go            | 53 +++++++++++++++-------------
 pkg/install/deployment_test.go       |  9 ++---
 pkg/install/resources.go             | 12 +++++--
 site/docs/master/aws-config.md       |  6 ++--
 site/docs/master/install-overview.md |  5 ++-
 9 files changed, 92 insertions(+), 63 deletions(-)
 create mode 100644 changelogs/unreleased/1699-nrb

diff --git a/changelogs/unreleased/1699-nrb b/changelogs/unreleased/1699-nrb
new file mode 100644
index 000000000..d5f28c035
--- /dev/null
+++ b/changelogs/unreleased/1699-nrb
@@ -0,0 +1 @@
+Make --secret-file argument on `velero install` optional, add --no-secret flag for explicit confirmation
diff --git a/pkg/cmd/cli/install/install.go b/pkg/cmd/cli/install/install.go
index ca52cbe2f..617993366 100644
--- a/pkg/cmd/cli/install/install.go
+++ b/pkg/cmd/cli/install/install.go
@@ -55,6 +55,7 @@ type InstallOptions struct {
 	ResticPodMemLimit    string
 	RestoreOnly          bool
 	SecretFile           string
+	NoSecret             bool
 	DryRun               bool
 	BackupStorageConfig  flag.Map
 	VolumeSnapshotConfig flag.Map
@@ -67,7 +68,8 @@ type InstallOptions struct {
 func (o *InstallOptions) BindFlags(flags *pflag.FlagSet) {
 	flags.StringVar(&o.ProviderName, "provider", o.ProviderName, "provider name for backup and volume storage")
 	flags.StringVar(&o.BucketName, "bucket", o.BucketName, "name of the object storage bucket where backups should be stored")
-	flags.StringVar(&o.SecretFile, "secret-file", o.SecretFile, "file containing credentials for backup and volume provider")
+	flags.StringVar(&o.SecretFile, "secret-file", o.SecretFile, "file containing credentials for backup and volume provider. If not specified, --no-secret must be used for confirmation. Optional.")
+	flags.BoolVar(&o.NoSecret, "no-secret", o.NoSecret, "flag indicating if a secret should be created. Must be used as confirmation if --secret-file is not provided. Optional.")
 	flags.StringVar(&o.Image, "image", o.Image, "image to use for the Velero and restic server pods. Optional.")
 	flags.StringVar(&o.Prefix, "prefix", o.Prefix, "prefix under which all Velero data should be stored within the bucket. Optional.")
 	flags.Var(&o.PodAnnotations, "pod-annotations", "annotations to add to the Velero and restic pods. Optional. Format is key1=value1,key2=value2")
@@ -112,13 +114,16 @@ func NewInstallOptions() *InstallOptions {
 
 // AsVeleroOptions translates the values provided at the command line into values used to instantiate Kubernetes resources
 func (o *InstallOptions) AsVeleroOptions() (*install.VeleroOptions, error) {
-	realPath, err := filepath.Abs(o.SecretFile)
-	if err != nil {
-		return nil, err
-	}
-	secretData, err := ioutil.ReadFile(realPath)
-	if err != nil {
-		return nil, err
+	var secretData []byte
+	if o.SecretFile != "" && !o.NoSecret {
+		realPath, err := filepath.Abs(o.SecretFile)
+		if err != nil {
+			return nil, err
+		}
+		secretData, err = ioutil.ReadFile(realPath)
+		if err != nil {
+			return nil, err
+		}
 	}
 	veleroPodResources, err := parseResourceRequests(o.VeleroPodCPURequest, o.VeleroPodMemRequest, o.VeleroPodCPULimit, o.VeleroPodMemLimit)
 	if err != nil {
@@ -179,7 +184,7 @@ This is useful as a starting point for more customized installations.
 
 	# velero install --bucket gcp-backups --provider gcp --secret-file ./gcp-creds.json --wait
 
-	# velero install --bucket backups --provider aws --backup-location-config region=us-west-2 --secret-file ./an-empty-file --snapshot-location-config region=us-west-2 --pod-annotations iam.amazonaws.com/role=arn:aws:iam::<AWS_ACCOUNT_ID>:role/<VELERO_ROLE_NAME>
+	# velero install --bucket backups --provider aws --backup-location-config region=us-west-2 --snapshot-location-config region=us-west-2 --no-secret --pod-annotations iam.amazonaws.com/role=arn:aws:iam::<AWS_ACCOUNT_ID>:role/<VELERO_ROLE_NAME>
 
 	# velero install --bucket gcp-backups --provider gcp --secret-file ./gcp-creds.json --velero-pod-cpu-request=1000m --velero-pod-cpu-limit=5000m --velero-pod-mem-request=512Mi --velero-pod-mem-limit=1024Mi
 
@@ -238,6 +243,9 @@ func (o *InstallOptions) Run(c *cobra.Command, f client.Factory) error {
 			return errors.Wrap(err, errorMsg)
 		}
 	}
+	if o.SecretFile == "" {
+		fmt.Printf("\nNo secret file was specified, no Secret created.\n\n")
+	}
 	fmt.Printf("Velero is installed! ⛵ Use 'kubectl logs deployment/velero -n %s' to view the status.\n", o.Namespace)
 	return nil
 }
@@ -268,8 +276,11 @@ func (o *InstallOptions) Validate(c *cobra.Command, args []string, f client.Fact
 		return errors.New("--provider is required")
 	}
 
-	if o.SecretFile == "" {
-		return errors.New("--secret-file is required")
+	switch {
+	case o.SecretFile == "" && !o.NoSecret:
+		return errors.New("One of --secret-file or --no-secret is required")
+	case o.SecretFile != "" && o.NoSecret:
+		return errors.New("Cannot use both --secret-file and --no-secret")
 	}
 
 	return nil
diff --git a/pkg/install/daemonset.go b/pkg/install/daemonset.go
index 2b543aae1..075fea6ba 100644
--- a/pkg/install/daemonset.go
+++ b/pkg/install/daemonset.go
@@ -129,18 +129,6 @@ func DaemonSet(namespace string, opts ...podTemplateOption) *appsv1.DaemonSet {
 									Name:  "VELERO_SCRATCH_DIR",
 									Value: "/scratch",
 								},
-								{
-									Name:  "AZURE_CREDENTIALS_FILE",
-									Value: "/credentials/cloud",
-								},
-								{
-									Name:  "GOOGLE_APPLICATION_CREDENTIALS",
-									Value: "/credentials/cloud",
-								},
-								{
-									Name:  "AWS_SHARED_CREDENTIALS_FILE",
-									Value: "/credentials/cloud",
-								},
 							},
 							Resources: c.resources,
 						},
@@ -150,7 +138,7 @@ func DaemonSet(namespace string, opts ...podTemplateOption) *appsv1.DaemonSet {
 		},
 	}
 
-	if !c.withoutCredentialsVolume {
+	if c.withSecret {
 		daemonSet.Spec.Template.Spec.Volumes = append(
 			daemonSet.Spec.Template.Spec.Volumes,
 			corev1.Volume{
@@ -170,6 +158,21 @@ func DaemonSet(namespace string, opts ...podTemplateOption) *appsv1.DaemonSet {
 				MountPath: "/credentials",
 			},
 		)
+
+		daemonSet.Spec.Template.Spec.Containers[0].Env = append(daemonSet.Spec.Template.Spec.Containers[0].Env, []corev1.EnvVar{
+			{
+				Name:  "GOOGLE_APPLICATION_CREDENTIALS",
+				Value: "/credentials/cloud",
+			},
+			{
+				Name:  "AWS_SHARED_CREDENTIALS_FILE",
+				Value: "/credentials/cloud",
+			},
+			{
+				Name:  "AZURE_CREDENTIALS_FILE",
+				Value: "/credentials/cloud",
+			},
+		}...)
 	}
 
 	daemonSet.Spec.Template.Spec.Containers[0].Env = append(daemonSet.Spec.Template.Spec.Containers[0].Env, c.envVars...)
diff --git a/pkg/install/daemonset_test.go b/pkg/install/daemonset_test.go
index efb3ae04a..8954bc9b2 100644
--- a/pkg/install/daemonset_test.go
+++ b/pkg/install/daemonset_test.go
@@ -29,10 +29,11 @@ func TestDaemonSet(t *testing.T) {
 	assert.Equal(t, "restic", ds.Spec.Template.Spec.Containers[0].Name)
 	assert.Equal(t, "velero", ds.ObjectMeta.Namespace)
 
-	ds = DaemonSet("velero", WithoutCredentialsVolume())
-	assert.Equal(t, 2, len(ds.Spec.Template.Spec.Volumes))
-
 	ds = DaemonSet("velero", WithImage("gcr.io/heptio-images/velero:v0.11"))
 	assert.Equal(t, "gcr.io/heptio-images/velero:v0.11", ds.Spec.Template.Spec.Containers[0].Image)
 	assert.Equal(t, corev1.PullIfNotPresent, ds.Spec.Template.Spec.Containers[0].ImagePullPolicy)
+
+	ds = DaemonSet("velero", WithSecret(true))
+	assert.Equal(t, 6, len(ds.Spec.Template.Spec.Containers[0].Env))
+	assert.Equal(t, 3, len(ds.Spec.Template.Spec.Volumes))
 }
diff --git a/pkg/install/deployment.go b/pkg/install/deployment.go
index 4c8b48941..857860da2 100644
--- a/pkg/install/deployment.go
+++ b/pkg/install/deployment.go
@@ -27,12 +27,12 @@ import (
 type podTemplateOption func(*podTemplateConfig)
 
 type podTemplateConfig struct {
-	image                    string
-	withoutCredentialsVolume bool
-	envVars                  []corev1.EnvVar
-	restoreOnly              bool
-	annotations              map[string]string
-	resources                corev1.ResourceRequirements
+	image       string
+	envVars     []corev1.EnvVar
+	restoreOnly bool
+	annotations map[string]string
+	resources   corev1.ResourceRequirements
+	withSecret  bool
 }
 
 func WithImage(image string) podTemplateOption {
@@ -47,12 +47,6 @@ func WithAnnotations(annotations map[string]string) podTemplateOption {
 	}
 }
 
-func WithoutCredentialsVolume() podTemplateOption {
-	return func(c *podTemplateConfig) {
-		c.withoutCredentialsVolume = true
-	}
-}
-
 func WithEnvFromSecretKey(varName, secret, key string) podTemplateOption {
 	return func(c *podTemplateConfig) {
 		c.envVars = append(c.envVars, corev1.EnvVar{
@@ -69,6 +63,12 @@ func WithEnvFromSecretKey(varName, secret, key string) podTemplateOption {
 	}
 }
 
+func WithSecret(secretPresent bool) podTemplateOption {
+	return func(c *podTemplateConfig) {
+		c.withSecret = secretPresent
+	}
+}
+
 func WithRestoreOnly() podTemplateOption {
 	return func(c *podTemplateConfig) {
 		c.restoreOnly = true
@@ -144,18 +144,6 @@ func Deployment(namespace string, opts ...podTemplateOption) *appsv1.Deployment
 									Name:  "VELERO_SCRATCH_DIR",
 									Value: "/scratch",
 								},
-								{
-									Name:  "GOOGLE_APPLICATION_CREDENTIALS",
-									Value: "/credentials/cloud",
-								},
-								{
-									Name:  "AWS_SHARED_CREDENTIALS_FILE",
-									Value: "/credentials/cloud",
-								},
-								{
-									Name:  "AZURE_CREDENTIALS_FILE",
-									Value: "/credentials/cloud",
-								},
 							},
 							Resources: c.resources,
 						},
@@ -179,7 +167,7 @@ func Deployment(namespace string, opts ...podTemplateOption) *appsv1.Deployment
 		},
 	}
 
-	if !c.withoutCredentialsVolume {
+	if c.withSecret {
 		deployment.Spec.Template.Spec.Volumes = append(
 			deployment.Spec.Template.Spec.Volumes,
 			corev1.Volume{
@@ -199,6 +187,21 @@ func Deployment(namespace string, opts ...podTemplateOption) *appsv1.Deployment
 				MountPath: "/credentials",
 			},
 		)
+
+		deployment.Spec.Template.Spec.Containers[0].Env = append(deployment.Spec.Template.Spec.Containers[0].Env, []corev1.EnvVar{
+			{
+				Name:  "GOOGLE_APPLICATION_CREDENTIALS",
+				Value: "/credentials/cloud",
+			},
+			{
+				Name:  "AWS_SHARED_CREDENTIALS_FILE",
+				Value: "/credentials/cloud",
+			},
+			{
+				Name:  "AZURE_CREDENTIALS_FILE",
+				Value: "/credentials/cloud",
+			},
+		}...)
 	}
 
 	deployment.Spec.Template.Spec.Containers[0].Env = append(deployment.Spec.Template.Spec.Containers[0].Env, c.envVars...)
diff --git a/pkg/install/deployment_test.go b/pkg/install/deployment_test.go
index c87c8a318..939d95fe3 100644
--- a/pkg/install/deployment_test.go
+++ b/pkg/install/deployment_test.go
@@ -32,15 +32,16 @@ func TestDeployment(t *testing.T) {
 	assert.Equal(t, "--restore-only", deploy.Spec.Template.Spec.Containers[0].Args[1])
 
 	deploy = Deployment("velero", WithEnvFromSecretKey("my-var", "my-secret", "my-key"))
-	envSecret := deploy.Spec.Template.Spec.Containers[0].Env[4]
+	envSecret := deploy.Spec.Template.Spec.Containers[0].Env[1]
 	assert.Equal(t, "my-var", envSecret.Name)
 	assert.Equal(t, "my-secret", envSecret.ValueFrom.SecretKeyRef.LocalObjectReference.Name)
 	assert.Equal(t, "my-key", envSecret.ValueFrom.SecretKeyRef.Key)
 
-	deploy = Deployment("velero", WithoutCredentialsVolume())
-	assert.Equal(t, 2, len(deploy.Spec.Template.Spec.Volumes))
-
 	deploy = Deployment("velero", WithImage("gcr.io/heptio-images/velero:v0.11"))
 	assert.Equal(t, "gcr.io/heptio-images/velero:v0.11", deploy.Spec.Template.Spec.Containers[0].Image)
 	assert.Equal(t, corev1.PullIfNotPresent, deploy.Spec.Template.Spec.Containers[0].ImagePullPolicy)
+
+	deploy = Deployment("velero", WithSecret(true))
+	assert.Equal(t, 4, len(deploy.Spec.Template.Spec.Containers[0].Env))
+	assert.Equal(t, 3, len(deploy.Spec.Template.Spec.Volumes))
 }
diff --git a/pkg/install/resources.go b/pkg/install/resources.go
index f470bdda1..f272dfa69 100644
--- a/pkg/install/resources.go
+++ b/pkg/install/resources.go
@@ -229,8 +229,10 @@ func AllResources(o *VeleroOptions) (*unstructured.UnstructuredList, error) {
 	sa := ServiceAccount(o.Namespace)
 	appendUnstructured(resources, sa)
 
-	sec := Secret(o.Namespace, o.SecretData)
-	appendUnstructured(resources, sec)
+	if o.SecretData != nil {
+		sec := Secret(o.Namespace, o.SecretData)
+		appendUnstructured(resources, sec)
+	}
 
 	bsl := BackupStorageLocation(o.Namespace, o.ProviderName, o.Bucket, o.Prefix, o.BSLConfig)
 	appendUnstructured(resources, bsl)
@@ -241,15 +243,19 @@ func AllResources(o *VeleroOptions) (*unstructured.UnstructuredList, error) {
 		appendUnstructured(resources, vsl)
 	}
 
+	secretPresent := o.SecretData != nil
+
 	deploy := Deployment(o.Namespace,
 		WithAnnotations(o.PodAnnotations),
 		WithImage(o.Image),
 		WithResources(o.VeleroPodResources),
+		WithSecret(secretPresent),
 	)
 	if o.RestoreOnly {
 		deploy = Deployment(o.Namespace,
 			WithAnnotations(o.PodAnnotations),
 			WithImage(o.Image),
+			WithSecret(secretPresent),
 			WithRestoreOnly(),
 		)
 	}
@@ -257,9 +263,11 @@ func AllResources(o *VeleroOptions) (*unstructured.UnstructuredList, error) {
 
 	if o.UseRestic {
 		ds := DaemonSet(o.Namespace,
+
 			WithAnnotations(o.PodAnnotations),
 			WithImage(o.Image),
 			WithResources(o.ResticPodResources),
+			WithSecret(secretPresent),
 		)
 		appendUnstructured(resources, ds)
 	}
diff --git a/site/docs/master/aws-config.md b/site/docs/master/aws-config.md
index 1515894bd..67b6c82dd 100644
--- a/site/docs/master/aws-config.md
+++ b/site/docs/master/aws-config.md
@@ -292,13 +292,11 @@ velero install \
     --pod-annotations iam.amazonaws.com/role=arn:aws:iam::<AWS_ACCOUNT_ID>:role/<VELERO_ROLE_NAME> \
     --provider aws \
     --bucket $BUCKET \
-    --secret-file ./credentials-velero \
     --backup-location-config region=$REGION \
-    --snapshot-location-config region=$REGION
+    --snapshot-location-config region=$REGION \
+    --no-secret
 ```
 
-Note that the `--secret-file` argument is required, but it can be an empty file.
-
 [0]: namespace.md
 [5]: https://docs.aws.amazon.com/cli/latest/userguide/cli-chap-welcome.html
 [6]: api-types/volumesnapshotlocation.md#aws
diff --git a/site/docs/master/install-overview.md b/site/docs/master/install-overview.md
index 9e26a08cd..6755ba603 100644
--- a/site/docs/master/install-overview.md
+++ b/site/docs/master/install-overview.md
@@ -14,7 +14,8 @@ The Velero client includes an `install` command to specify the settings for each
 velero install \
     --provider <YOUR_PROVIDER> \
     --bucket <YOUR_BUCKET> \
-    --secret-file <PATH_TO_FILE> \
+    [--secret-file <PATH_TO_FILE>] \
+    [--no-secret] \
     [--backup-location-config] \
     [--snapshot-location-config] \
     [--namespace] \
@@ -23,6 +24,8 @@ velero install \
     [--pod-annotations] \
 ```
 
+When using node-based IAM policies, `--secret-file` is not required, but `--no-secret` is required for confirmation.
+
 For provider-specific instructions, see:
 
 * [Run Velero on AWS][0]