From 259903978b3a64f468df93569c767b58dc594043 Mon Sep 17 00:00:00 2001
From: Jason Hall <jasonhall@google.com>
Date: Wed, 9 Oct 2019 14:08:01 -0400
Subject: [PATCH] Generate volume and volume mount names

Before this, if a user wanted to attach a Volume named "workspace"
they'd get a confusing error message (#1402). #1404 improves the error
message, but it would be nice to not have an error at all and just allow
user-defined volumes named "workspace"
---
 pkg/reconciler/taskrun/resources/pod.go      | 49 ++++++++++++--------
 pkg/reconciler/taskrun/resources/pod_test.go |  2 +
 2 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/pkg/reconciler/taskrun/resources/pod.go b/pkg/reconciler/taskrun/resources/pod.go
index 91437b36751..4ac9688db8c 100644
--- a/pkg/reconciler/taskrun/resources/pod.go
+++ b/pkg/reconciler/taskrun/resources/pod.go
@@ -67,20 +67,6 @@ var (
 		Name:  "HOME",
 		Value: homeDir,
 	}}
-	implicitVolumeMounts = []corev1.VolumeMount{{
-		Name:      "workspace",
-		MountPath: workspaceDir,
-	}, {
-		Name:      "home",
-		MountPath: homeDir,
-	}}
-	implicitVolumes = []corev1.Volume{{
-		Name:         "workspace",
-		VolumeSource: emptyVolumeSource,
-	}, {
-		Name:         "home",
-		VolumeSource: emptyVolumeSource,
-	}}
 
 	zeroQty = resource.MustParse("0")
 
@@ -101,7 +87,28 @@ const (
 	readyAnnotationValue = "READY"
 )
 
-func makeCredentialInitializer(credsImage, serviceAccountName, namespace string, kubeclient kubernetes.Interface) (*v1alpha1.Step, []corev1.Volume, error) {
+// implicitVolumesAndVolumeMounts returns implicit Volumes and VolumeMounts for
+// workspace and home volumes, with a generated name.
+func implicitVolumesAndVolumeMounts() ([]corev1.Volume, []corev1.VolumeMount) {
+	workspaceVolumeName := names.SimpleNameGenerator.RestrictLength("workspace-")
+	homeVolumeName := names.SimpleNameGenerator.RestrictLength("home-")
+	return []corev1.Volume{{
+			Name:         workspaceVolumeName,
+			VolumeSource: emptyVolumeSource,
+		}, {
+			Name:         homeVolumeName,
+			VolumeSource: emptyVolumeSource,
+		}},
+		[]corev1.VolumeMount{{
+			Name:      workspaceVolumeName,
+			MountPath: workspaceDir,
+		}, {
+			Name:      homeVolumeName,
+			MountPath: homeDir,
+		}}
+}
+
+func makeCredentialInitializer(credsImage, serviceAccountName, namespace string, implicitVolumes []corev1.Volume, implicitVolumeMounts []corev1.VolumeMount, kubeclient kubernetes.Interface) (*v1alpha1.Step, []corev1.Volume, error) {
 	if serviceAccountName == "" {
 		serviceAccountName = "default"
 	}
@@ -184,7 +191,7 @@ func makeWorkingDirScript(workingDirs map[string]bool) string {
 	return script
 }
 
-func makeWorkingDirInitializer(bashNoopImage string, steps []v1alpha1.Step) *v1alpha1.Step {
+func makeWorkingDirInitializer(bashNoopImage string, steps []v1alpha1.Step, implicitVolumeMounts []corev1.VolumeMount) *v1alpha1.Step {
 	workingDirs := make(map[string]bool)
 	for _, step := range steps {
 		workingDirs[step.WorkingDir] = true
@@ -206,7 +213,7 @@ func makeWorkingDirInitializer(bashNoopImage string, steps []v1alpha1.Step) *v1a
 
 // initOutputResourcesDefaultDir checks if there are any output image resources expecting a default path
 // and creates an init container to create that folder
-func initOutputResourcesDefaultDir(bashNoopImage string, taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec) []v1alpha1.Step {
+func initOutputResourcesDefaultDir(bashNoopImage string, taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, implicitVolumeMounts []corev1.VolumeMount) []v1alpha1.Step {
 	var makeDirSteps []v1alpha1.Step
 	if len(taskRun.Spec.Outputs.Resources) > 0 {
 		for _, r := range taskRun.Spec.Outputs.Resources {
@@ -244,18 +251,20 @@ func TryGetPod(taskRunStatus v1alpha1.TaskRunStatus, gp GetPod) (*corev1.Pod, er
 // MakePod converts TaskRun and TaskSpec objects to a Pod which implements the taskrun specified
 // by the supplied CRD.
 func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient kubernetes.Interface) (*corev1.Pod, error) {
-	cred, secrets, err := makeCredentialInitializer(images.CredsImage, taskRun.GetServiceAccountName(), taskRun.Namespace, kubeclient)
+	implicitVolumes, implicitVolumeMounts := implicitVolumesAndVolumeMounts()
+
+	cred, secrets, err := makeCredentialInitializer(images.CredsImage, taskRun.GetServiceAccountName(), taskRun.Namespace, implicitVolumes, implicitVolumeMounts, kubeclient)
 	if err != nil {
 		return nil, err
 	}
 	initSteps := []v1alpha1.Step{*cred}
 	var podSteps []v1alpha1.Step
 
-	if workingDir := makeWorkingDirInitializer(images.BashNoopImage, taskSpec.Steps); workingDir != nil {
+	if workingDir := makeWorkingDirInitializer(images.BashNoopImage, taskSpec.Steps, implicitVolumeMounts); workingDir != nil {
 		initSteps = append(initSteps, *workingDir)
 	}
 
-	initSteps = append(initSteps, initOutputResourcesDefaultDir(images.BashNoopImage, taskRun, taskSpec)...)
+	initSteps = append(initSteps, initOutputResourcesDefaultDir(images.BashNoopImage, taskRun, taskSpec, implicitVolumeMounts)...)
 
 	maxIndicesByResource := findMaxResourceRequest(taskSpec.Steps, corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage)
 
diff --git a/pkg/reconciler/taskrun/resources/pod_test.go b/pkg/reconciler/taskrun/resources/pod_test.go
index e2a8b067f93..45dca1487f7 100644
--- a/pkg/reconciler/taskrun/resources/pod_test.go
+++ b/pkg/reconciler/taskrun/resources/pod_test.go
@@ -41,6 +41,8 @@ var (
 	})
 	credsImage    = "override-with-creds:latest"
 	bashNoopImage = "override-with-bash-noop:latest"
+
+	implicitVolumes, implicitVolumeMounts = implicitVolumesAndVolumeMounts()
 )
 
 func TestTryGetPod(t *testing.T) {