Merge pull request #784 from SaschaSchwarze0/sascha-sanitize-volume-name

Ensure volume name is sanitized from the secret name

pkg/reconciler/buildrun/resources/sources/git.go

@@ -60,7 +60,7 @@ func AppendGitStep(
 
 		// define the volume mount on the container
 		gitStep.VolumeMounts = append(gitStep.VolumeMounts, corev1.VolumeMount{
-			Name:      fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, source.Credentials.Name),
+			Name:      SanitizeVolumeNameForSecretName(source.Credentials.Name),
 			MountPath: secretMountPath,
 			ReadOnly:  true,
 		})

pkg/reconciler/buildrun/resources/sources/git_test.go

@@ -66,7 +66,7 @@ var _ = Describe("Git", func() {
 			sources.AppendGitStep(cfg, taskSpec, buildv1alpha1.Source{
 				URL: "[email protected]:shipwright-io/build.git",
 				Credentials: &corev1.LocalObjectReference{
-					Name: "a-secret",
+					Name: "a.secret",
 				},
 			}, "default")
 		})
@@ -80,7 +80,7 @@ var _ = Describe("Git", func() {
 			Expect(len(taskSpec.Volumes)).To(Equal(1))
 			Expect(taskSpec.Volumes[0].Name).To(Equal("shp-a-secret"))
 			Expect(taskSpec.Volumes[0].VolumeSource.Secret).NotTo(BeNil())
-			Expect(taskSpec.Volumes[0].VolumeSource.Secret.SecretName).To(Equal("a-secret"))
+			Expect(taskSpec.Volumes[0].VolumeSource.Secret.SecretName).To(Equal("a.secret"))
 		})
 
 		It("adds a step", func() {

pkg/reconciler/buildrun/resources/sources/utils.go

@@ -31,7 +31,7 @@ func AppendSecretVolume(
 	taskSpec *tektonv1beta1.TaskSpec,
 	secretName string,
 ) {
-	volumeName := fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, secretName)
+	volumeName := SanitizeVolumeNameForSecretName(secretName)
 
 	// ensure we do not add the secret twice
 	for _, volume := range taskSpec.Volumes {
@@ -52,10 +52,10 @@ func AppendSecretVolume(
 	})
 }
 
-// SanitizeVolumeName ensures that there are no forbidden names in the volume name and that its name is not too long
-func SanitizeVolumeName(name string) string {
+// SanitizeVolumeNameForSecretName creates the name of a Volume for a Secret
+func SanitizeVolumeNameForSecretName(secretName string) string {
 	// remove forbidden characters
-	sanitizedName := dnsLabel1123Forbidden.ReplaceAllString(name, "-")
+	sanitizedName := dnsLabel1123Forbidden.ReplaceAllString(fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, secretName), "-")
 
 	// ensure maximum length
 	if len(sanitizedName) > 63 {

pkg/reconciler/buildrun/resources/sources/utils_test.go

@@ -17,16 +17,16 @@ var _ = Describe("Utils", func() {
 
 	Context("for different candidate volume names", func() {
 
-		It("retains a name that is okay", func() {
-			Expect(sources.SanitizeVolumeName("okay-name")).To(Equal("okay-name"))
+		It("adds only the prefix if the name is okay", func() {
+			Expect(sources.SanitizeVolumeNameForSecretName("okay-name")).To(Equal("shp-okay-name"))
 		})
 
-		It("replaces characters that are not allowed", func() {
-			Expect(sources.SanitizeVolumeName("bad.name")).To(Equal("bad-name"))
+		It("adds the prefix and replaces characters that are not allowed", func() {
+			Expect(sources.SanitizeVolumeNameForSecretName("bad.name")).To(Equal("shp-bad-name"))
 		})
 
-		It("reduces the length if needed", func() {
-			Expect(sources.SanitizeVolumeName("long-name-long-name-long-name-long-name-long-name-long-name-long-name-")).To(Equal("long-name-long-name-long-name-long-name-long-name-long-name-lon"))
+		It("adds the prefix and reduces the length if needed", func() {
+			Expect(sources.SanitizeVolumeNameForSecretName("long-name-long-name-long-name-long-name-long-name-long-name-long-name-")).To(Equal("shp-long-name-long-name-long-name-long-name-long-name-long-name"))
 		})
 	})