From 71699686870ff0bbeb0c3e1d8b1cfe7162434ddb Mon Sep 17 00:00:00 2001
From: Victor Tjhia <victortjhia@google.com>
Date: Wed, 25 Oct 2023 18:32:08 -0700
Subject: [PATCH] TestHelmLatestVersion: Remove helm chart version suffix
 (#966)

* Remove helm chart version suffix

* Move validation
---
 e2e/nomostest/artifactregistry/helm.go  |  4 ++-
 e2e/nomostest/artifactregistry/image.go | 37 +++++++++++++------------
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/e2e/nomostest/artifactregistry/helm.go b/e2e/nomostest/artifactregistry/helm.go
index 8157613001..612955ed86 100644
--- a/e2e/nomostest/artifactregistry/helm.go
+++ b/e2e/nomostest/artifactregistry/helm.go
@@ -130,7 +130,9 @@ func updateYAMLFile(name string, updateFn func(map[string]interface{}) error) er
 // SetVersion updates the local version of the helm chart to the specified
 // version with a random suffix
 func (r *HelmChart) SetVersion(version string) error {
-	r.Image.SetVersion(version)
+	if err := r.Image.SetVersion(version); err != nil {
+		return err
+	}
 	version = r.Image.Version
 	chartFilePath := filepath.Join(r.Image.BuildPath, "Chart.yaml")
 	err := updateYAMLFile(chartFilePath, func(chartMap map[string]interface{}) error {
diff --git a/e2e/nomostest/artifactregistry/image.go b/e2e/nomostest/artifactregistry/image.go
index dd40efa680..297d159739 100644
--- a/e2e/nomostest/artifactregistry/image.go
+++ b/e2e/nomostest/artifactregistry/image.go
@@ -23,7 +23,6 @@ import (
 	"strings"
 
 	"github.com/ettle/strcase"
-	"github.com/google/uuid"
 	"kpt.dev/configsync/e2e"
 	"kpt.dev/configsync/e2e/nomostest"
 	"kpt.dev/configsync/e2e/nomostest/testlogger"
@@ -48,8 +47,10 @@ func SetupImage(nt *nomostest.NT, name, version string) (*Image, error) {
 	if name == "" {
 		return nil, fmt.Errorf("image name must not be empty")
 	}
-	if version == "" {
-		return nil, fmt.Errorf("image version must not be empty")
+
+	// Version will error out if it's empty or longer than 20 characters
+	if err := validateImageVersion(version); err != nil {
+		return nil, err
 	}
 	chart := &Image{
 		Shell:   nt.Shell,
@@ -62,9 +63,8 @@ func SetupImage(nt *nomostest.NT, name, version string) (*Image, error) {
 		// Use chart name to avoid overlap.
 		BuildPath: filepath.Join(nt.TmpDir, name),
 		// Use test name to avoid overlap. Truncate to 40 characters.
-		Name: generateImageName(nt, name),
-		// Use random suffix to avoid overlap. Truncate to 20 characters.
-		Version: generateImageVersion(version),
+		Name:    generateImageName(nt, name),
+		Version: version,
 	}
 	nt.T.Cleanup(func() {
 		if err := chart.Delete(); err != nil {
@@ -216,10 +216,12 @@ func (r *Image) SetName(nt *nomostest.NT, name string) {
 
 // SetVersion updates the local version of the image to the specified tag with a
 // random suffix
-func (r *Image) SetVersion(version string) {
-	version = generateImageVersion(version)
-	r.Logger.Infof("Updating image version to %q", version)
+func (r *Image) SetVersion(version string) error {
+	if err := validateImageVersion(version); err != nil {
+		return err
+	}
 	r.Version = version
+	return nil
 }
 
 // Delete the package from the remote registry, including all versions and tags.
@@ -248,14 +250,15 @@ func generateImageName(nt *nomostest.NT, chartName string) string {
 	return chartName
 }
 
-// generateImageVersion returns the version with a random 8 character suffix.
-// Result will be no more than 20 characters and can function as a k8s metadata.name.
+// validateImageVersion will validate if chart version string is not empty and
+// is 20 characters maximum and can function as a k8s metadata.name.
 // Chart name and version must be less than 63 characters combined.
-func generateImageVersion(chartVersion string) string {
-	if len(chartVersion) > 12 {
-		chartVersion = chartVersion[:12]
-		chartVersion = strings.Trim(strings.Trim(chartVersion, "-"), ".")
+func validateImageVersion(chartVersion string) error {
+	if chartVersion == "" {
+		return fmt.Errorf("image version must not be empty")
+	}
+	if len(chartVersion) > 20 {
+		return fmt.Errorf("chart version string %q should not exceed 20 characters", chartVersion)
 	}
-	return fmt.Sprintf("%s-%s", chartVersion,
-		strings.ReplaceAll(uuid.NewString(), "-", "")[:7])
+	return nil
 }