Rename name generator methods, fix bug on hyphen and max length

pkg/apis/pipeline/v1alpha1/artifact_bucket.go

@@ -82,13 +82,13 @@ func (b *ArtifactBucket) GetCopyFromContainerSpec(name, sourcePath, destinationP
 	envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts("bucket", secretVolumeMountPath, b.Secrets)
 
 	return []corev1.Container{{
-		Name:  names.SimpleNameGenerator.GenerateName(fmt.Sprintf("artifact-dest-mkdir-%s", name)),
+		Name:  names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("artifact-dest-mkdir-%s", name)),
 		Image: *bashNoopImage,
 		Args: []string{
 			"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " "),
 		},
 	}, {
-		Name:         names.SimpleNameGenerator.GenerateName(fmt.Sprintf("artifact-copy-from-%s", name)),
+		Name:         names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("artifact-copy-from-%s", name)),
 		Image:        *gsutilImage,
 		Args:         args,
 		Env:          envVars,
@@ -103,7 +103,7 @@ func (b *ArtifactBucket) GetCopyToContainerSpec(name, sourcePath, destinationPat
 	envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts("bucket", secretVolumeMountPath, b.Secrets)
 
 	return []corev1.Container{{
-		Name:         names.SimpleNameGenerator.GenerateName(fmt.Sprintf("artifact-copy-to-%s", name)),
+		Name:         names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("artifact-copy-to-%s", name)),
 		Image:        *gsutilImage,
 		Args:         args,
 		Env:          envVars,
@@ -117,7 +117,7 @@ func (b *ArtifactBucket) GetSecretsVolumes() []corev1.Volume {
 	volumes := []corev1.Volume{}
 	for _, sec := range b.Secrets {
 		volumes = append(volumes, corev1.Volume{
-			Name: names.SimpleNameGenerator.GenerateName(fmt.Sprintf("bucket-secret-%s", sec.SecretName)),
+			Name: names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("bucket-secret-%s", sec.SecretName)),
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
 					SecretName: sec.SecretName,

pkg/apis/pipeline/v1alpha1/artifact_pvc.go

@@ -49,7 +49,7 @@ func (p *ArtifactPVC) StorageBasePath(pr *PipelineRun) string {
 // GetCopyFromContainerSpec returns a container used to download artifacts from temporary storage
 func (p *ArtifactPVC) GetCopyFromContainerSpec(name, sourcePath, destinationPath string) []corev1.Container {
 	return []corev1.Container{{
-		Name:  names.SimpleNameGenerator.GenerateName(fmt.Sprintf("source-copy-%s", name)),
+		Name:  names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("source-copy-%s", name)),
 		Image: *bashNoopImage,
 		Args:  []string{"-args", strings.Join([]string{"cp", "-r", fmt.Sprintf("%s/.", sourcePath), destinationPath}, " ")},
 	}}
@@ -58,14 +58,14 @@ func (p *ArtifactPVC) GetCopyFromContainerSpec(name, sourcePath, destinationPath
 // GetCopyToContainerSpec returns a container used to upload artifacts for temporary storage
 func (p *ArtifactPVC) GetCopyToContainerSpec(name, sourcePath, destinationPath string) []corev1.Container {
 	return []corev1.Container{{
-		Name:  names.SimpleNameGenerator.GenerateName(fmt.Sprintf("source-mkdir-%s", name)),
+		Name:  names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("source-mkdir-%s", name)),
 		Image: *bashNoopImage,
 		Args: []string{
 			"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " "),
 		},
 		VolumeMounts: []corev1.VolumeMount{getPvcMount(p.Name)},
 	}, {
-		Name:  names.SimpleNameGenerator.GenerateName(fmt.Sprintf("source-copy-%s", name)),
+		Name:  names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("source-copy-%s", name)),
 		Image: *bashNoopImage,
 		Args: []string{
 			"-args", strings.Join([]string{"cp", "-r", fmt.Sprintf("%s/.", sourcePath), destinationPath}, " "),
@@ -84,7 +84,7 @@ func getPvcMount(name string) corev1.VolumeMount {
 // CreateDirContainer returns a container step to create a dir
 func CreateDirContainer(name, destinationPath string) corev1.Container {
 	return corev1.Container{
-		Name:  names.SimpleNameGenerator.GenerateName(fmt.Sprintf("create-dir-%s", name)),
+		Name:  names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("create-dir-%s", name)),
 		Image: *bashNoopImage,
 		Args:  []string{"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " ")},
 	}

pkg/apis/pipeline/v1alpha1/build_gcs_resource.go

@@ -140,7 +140,7 @@ func (s *BuildGCSResource) GetDownloadContainerSpec() ([]corev1.Container, error
 
 	return []corev1.Container{
 		CreateDirContainer(s.Name, s.DestinationDir), {
-			Name:  names.SimpleNameGenerator.GenerateName(fmt.Sprintf("storage-fetch-%s", s.Name)),
+			Name:  names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("storage-fetch-%s", s.Name)),
 			Image: *buildGCSFetcherImage,
 			Args:  args,
 		}}, nil
@@ -158,7 +158,7 @@ func (s *BuildGCSResource) GetUploadContainerSpec() ([]corev1.Container, error)
 	args := []string{"--location", s.Location, "--dir", s.DestinationDir}
 
 	return []corev1.Container{{
-		Name:  names.SimpleNameGenerator.GenerateName(fmt.Sprintf("storage-upload-%s", s.Name)),
+		Name:  names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("storage-upload-%s", s.Name)),
 		Image: *buildGCSUploaderImage,
 		Args:  args,
 	}}, nil

pkg/apis/pipeline/v1alpha1/cluster_resource.go

@@ -168,7 +168,7 @@ func (s *ClusterResource) GetDownloadContainerSpec() ([]corev1.Container, error)
 	}
 
 	clusterContainer := corev1.Container{
-		Name:  names.SimpleNameGenerator.GenerateName("kubeconfig"),
+		Name:  names.SimpleNameGenerator.GenerateNameWithSuffix("kubeconfig"),
 		Image: *kubeconfigWriterImage,
 		Args: []string{
 			"-clusterConfig", s.String(),

pkg/apis/pipeline/v1alpha1/gcs_resource.go

@@ -119,7 +119,7 @@ func (s *GCSResource) GetUploadContainerSpec() ([]corev1.Container, error) {
 	envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts(s.Name, gcsSecretVolumeMountPath, s.Secrets)
 
 	return []corev1.Container{{
-		Name:         names.SimpleNameGenerator.GenerateName(fmt.Sprintf("upload-%s", s.Name)),
+		Name:         names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("upload-%s", s.Name)),
 		Image:        *gsutilImage,
 		Args:         args,
 		VolumeMounts: secretVolumeMount,
@@ -142,7 +142,7 @@ func (s *GCSResource) GetDownloadContainerSpec() ([]corev1.Container, error) {
 	envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts(s.Name, gcsSecretVolumeMountPath, s.Secrets)
 	return []corev1.Container{
 		CreateDirContainer(s.Name, s.DestinationDir), {
-			Name:         names.SimpleNameGenerator.GenerateName(fmt.Sprintf("fetch-%s", s.Name)),
+			Name:         names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("fetch-%s", s.Name)),
 			Image:        *gsutilImage,
 			Args:         args,
 			Env:          envVars,

pkg/apis/pipeline/v1alpha1/git_resource.go

@@ -113,7 +113,7 @@ func (s *GitResource) GetDownloadContainerSpec() ([]corev1.Container, error) {
 	args = append(args, []string{"-path", dPath}...)
 
 	return []corev1.Container{{
-		Name:       names.SimpleNameGenerator.GenerateName(gitSource + "-" + s.Name),
+		Name:       names.SimpleNameGenerator.GenerateNameWithSuffix(gitSource + "-" + s.Name),
 		Image:      *gitImage,
 		Args:       args,
 		WorkingDir: workspaceDir,

pkg/names/generate.go

@@ -25,14 +25,14 @@ import (
 // NameGenerator generates names for objects. Some backends may have more information
 // available to guide selection of new names and this interface hides those details.
 type NameGenerator interface {
-	// GenerateName generates a valid name from the base name, adding a random suffix to the
+	// GenerateNameWithSuffix generates a valid name from the base name, adding a random suffix to the
 	// the base. If base is valid, the returned name must also be valid. The generator is
 	// responsible for knowing the maximum valid name length.
-	GenerateName(base string) string
+	GenerateNameWithSuffix(base string) string
 
-	// GenerateBuildStepName generates a valid name from the name of a step specified in a Task,
+	// GenerateName generates a valid name from the name of a step specified in a Task,
 	// shortening it to the maximum valid name length if needed.
-	GenerateBuildStepName(base string) string
+	GenerateName(base string) string
 }
 
 // simpleNameGenerator generates random names.
@@ -50,14 +50,14 @@ const (
 	maxGeneratedNameLength = maxNameLength - randomLength
 )
 
-func (simpleNameGenerator) GenerateName(base string) string {
-	if len(base) > maxGeneratedNameLength {
-		base = base[:maxGeneratedNameLength]
+func (simpleNameGenerator) GenerateNameWithSuffix(base string) string {
+	if len(base) > maxGeneratedNameLength - 1 {
+		base = base[:maxGeneratedNameLength - 1]
 	}
 	return fmt.Sprintf("%s-%s", base, utilrand.String(randomLength))
 }
 
-func (simpleNameGenerator) GenerateBuildStepName(base string) string {
+func (simpleNameGenerator) GenerateName(base string) string {
 	if len(base) > maxGeneratedNameLength {
 		base = base[:maxGeneratedNameLength]
 	}

pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go

@@ -244,7 +244,7 @@ func getTaskRunName(taskRunsStatus map[string]v1alpha1.TaskRunStatus, prName str
 		}
 	}
 
-	return names.SimpleNameGenerator.GenerateName(base)
+	return names.SimpleNameGenerator.GenerateNameWithSuffix(base)
 }
 
 // GetPipelineConditionStatus will return the Condition that the PipelineRun prName should be

pkg/reconciler/v1alpha1/taskrun/resources/pod.go

@@ -123,7 +123,7 @@ func gcsToContainer(source v1alpha1.SourceSpec, index int) (*corev1.Container, e
 		containerName = containerName + strconv.Itoa(index)
 	}
 
-	containerName = names.SimpleNameGenerator.GenerateName(containerName)
+	containerName = names.SimpleNameGenerator.GenerateNameWithSuffix(containerName)
 
 	return &corev1.Container{
 		Name:         containerName,
@@ -183,7 +183,7 @@ func makeCredentialInitializer(build *v1alpha1.Build, kubeclient kubernetes.Inte
 		}
 
 		if matched {
-			name := names.SimpleNameGenerator.GenerateName(fmt.Sprintf("secret-volume-%s", secret.Name))
+			name := names.SimpleNameGenerator.GenerateNameWithSuffix(fmt.Sprintf("secret-volume-%s", secret.Name))
 			volumeMounts = append(volumeMounts, corev1.VolumeMount{
 				Name:      name,
 				MountPath: credentials.VolumeName(secret.Name),
@@ -200,7 +200,7 @@ func makeCredentialInitializer(build *v1alpha1.Build, kubeclient kubernetes.Inte
 	}
 
 	return &corev1.Container{
-		Name:         names.SimpleNameGenerator.GenerateName(initContainerPrefix + credsInit),
+		Name:         names.SimpleNameGenerator.GenerateNameWithSuffix(initContainerPrefix + credsInit),
 		Image:        *credsImage,
 		Args:         args,
 		VolumeMounts: volumeMounts,
@@ -270,7 +270,7 @@ func MakePod(build *v1alpha1.Build, kubeclient kubernetes.Interface) (*corev1.Po
 		if step.Name == "" {
 			step.Name = fmt.Sprintf("%v%d", unnamedInitContainerPrefix, i)
 		} else {
-			step.Name = names.SimpleNameGenerator.GenerateBuildStepName(fmt.Sprintf("%v%v", initContainerPrefix, step.Name))
+			step.Name = names.SimpleNameGenerator.GenerateName(fmt.Sprintf("%v%v", initContainerPrefix, step.Name))
 		}
 
 		initContainers = append(initContainers, step)
@@ -298,7 +298,7 @@ func MakePod(build *v1alpha1.Build, kubeclient kubernetes.Interface) (*corev1.Po
 			// Generate a unique name based on the build's name.
 			// Add a unique suffix to avoid confusion when a build
 			// is deleted and re-created with the same name.
-			// We don't use GenerateName here because k8s fakes don't support it.
+			// We don't use GenerateNameWithSuffix here because k8s fakes don't support it.
 			Name: fmt.Sprintf("%s-pod-%s", build.Name, gibberish),
 			// If our parent TaskRun is deleted, then we should be as well.
 			OwnerReferences: build.OwnerReferences,

test/helm_task_test.go

@@ -157,7 +157,7 @@ func getCreateImageTask(namespace string, t *testing.T, logger *logging.BaseLogg
 		t.Fatalf("KO_DOCKER_REPO env variable is required")
 	}
 
-	imageName = fmt.Sprintf("%s/%s", dockerRepo, names.SimpleNameGenerator.GenerateName(sourceImageName))
+	imageName = fmt.Sprintf("%s/%s", dockerRepo, names.SimpleNameGenerator.GenerateNameWithSuffix(sourceImageName))
 	logger.Infof("Image to be pusblished: %s", imageName)
 
 	return tb.Task(createImageTaskName, namespace, tb.TaskSpec(
@@ -230,7 +230,7 @@ func setupClusterBindingForHelm(c *clients, t *testing.T, namespace string, logg
 
 	clusterRoleBindings[0] = &rbacv1.ClusterRoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: names.SimpleNameGenerator.GenerateName("tiller"),
+			Name: names.SimpleNameGenerator.GenerateNameWithSuffix("tiller"),
 		},
 		RoleRef: rbacv1.RoleRef{
 			APIGroup: "rbac.authorization.k8s.io",
@@ -246,7 +246,7 @@ func setupClusterBindingForHelm(c *clients, t *testing.T, namespace string, logg
 
 	clusterRoleBindings[1] = &rbacv1.ClusterRoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: names.SimpleNameGenerator.GenerateName("default-tiller"),
+			Name: names.SimpleNameGenerator.GenerateNameWithSuffix("default-tiller"),
 		},
 		RoleRef: rbacv1.RoleRef{
 			APIGroup: "rbac.authorization.k8s.io",
@@ -262,7 +262,7 @@ func setupClusterBindingForHelm(c *clients, t *testing.T, namespace string, logg
 
 	clusterRoleBindings[2] = &rbacv1.ClusterRoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: names.SimpleNameGenerator.GenerateName("default-tiller"),
+			Name: names.SimpleNameGenerator.GenerateNameWithSuffix("default-tiller"),
 		},
 		RoleRef: rbacv1.RoleRef{
 			APIGroup: "rbac.authorization.k8s.io",

test/init_test.go

@@ -53,7 +53,7 @@ func getContextLogger(n string) *logging.BaseLogger {
 
 func setup(t *testing.T, logger *logging.BaseLogger) (*clients, string) {
 	t.Helper()
-	namespace := names.SimpleNameGenerator.GenerateName("arendelle")
+	namespace := names.SimpleNameGenerator.GenerateNameWithSuffix("arendelle")
 
 	c := newClients(t, knativetest.Flags.Kubeconfig, knativetest.Flags.Cluster, namespace)
 	createNamespace(namespace, logger, c.KubeClient)