feat(validation): add error messages for Helm release name validation

Added error messages for detecting empty or invalid Helm release names that do not conform to DNS-1035 label standards. feat(validation): implement Helm release name validation Added a new function 'ValidateReleaseName' to ensure that Helm release names conform to DNS-1035 label standards. Integrated this validation into the 'ZarfChart' struct's 'Validate' method to check release names and fallback mechanisms. test(validation): add unit tests for Helm release name validation Added tests to cover various scenarios, including valid release names, invalid names with periods, fallback to chart names, and missing names. Signed-off-by: jamestexas <[email protected]>
zarf-dev · Jul 30, 2024 · 2658ff4 · 2658ff4
1 parent 05fdaef
commit 2658ff4
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 3 deletions.
diff --git a/src/types/validate.go b/src/types/validate.go
@@ -16,7 +16,9 @@ import (
 
 const (
 	// ZarfMaxChartNameLength limits helm chart name size to account for K8s/helm limits and zarf prefix
-	ZarfMaxChartNameLength = 40
+	ZarfMaxChartNameLength     = 40
+	errChartReleaseNameEmpty   = "release name empty, unable to fallback to chart name"
+	errChartReleaseNameInvalid = "invalid release name %s: a DNS-1035 label must consist of lower case alphanumeric characters or -, start with an alphabetic character, and end with an alphanumeric character"
 )
 
 var (
@@ -25,7 +27,8 @@ var (
 	IsLowercaseNumberHyphenNoStartHyphen = regexp.MustCompile(`^[a-z0-9][a-z0-9\-]*$`).MatchString
 	// Define allowed OS, an empty string means it is allowed on all operating systems
 	// same as enums on ZarfComponentOnlyTarget
-	supportedOS = []string{"linux", "darwin", "windows", ""}
+	supportedOS       = []string{"linux", "darwin", "windows", ""}
+	dns1035LabelRegex = regexp.MustCompile(`^[a-z]([-a-z0-9]*[a-z0-9])?$`)
 )
 
 // SupportedOS returns the supported operating systems.
@@ -252,6 +255,28 @@ func (action ZarfComponentAction) Validate() error {
 	return err
 }
 
+// validateReleaseName validates a release name against DNS 1035 spec, using chartName as fallback.
+// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names
+func validateReleaseName(chartName, releaseName string) (err error) {
+	// Fallback to chartName if releaseName is empty
+	// NOTE: Similar fallback mechanism happens in src/internal/packager/helm/chart.go:InstallOrUpgradeChart
+	if releaseName == "" {
+		releaseName = chartName
+	}
+
+	// Check if the final releaseName is empty and return an error if so
+	if releaseName == "" {
+		err = fmt.Errorf(errChartReleaseNameEmpty)
+		return
+	}
+
+	// Validate the releaseName against DNS 1035 label spec
+	if !dns1035LabelRegex.MatchString(releaseName) {
+		err = fmt.Errorf(errChartReleaseNameInvalid, releaseName)
+	}
+	return
+}
+
 // Validate runs all validation checks on a chart.
 func (chart ZarfChart) Validate() error {
 	var err error
@@ -277,6 +302,10 @@ func (chart ZarfChart) Validate() error {
 		err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrChartVersion, chart.Name))
 	}
 
+	if nameErr := validateReleaseName(chart.Name, chart.ReleaseName); nameErr != nil {
+		err = errors.Join(err, nameErr)
+	}
+
 	return err
 }
 

diff --git a/src/types/validate_test.go b/src/types/validate_test.go
@@ -186,6 +186,73 @@ func TestValidateManifest(t *testing.T) {
 	}
 }
 
+func TestValidateReleaseName(t *testing.T) {
+	tests := []struct {
+		name         string
+		chartName    string
+		releaseName  string
+		expectedErrs []string
+	}{
+		{
+			name:         "valid releaseName with hyphens",
+			chartName:    "chart",
+			releaseName:  "valid-release-hyphenated",
+			expectedErrs: nil,
+		},
+		{
+			name:         "valid releaseName with numbers",
+			chartName:    "chart",
+			releaseName:  "valid-0470",
+			expectedErrs: nil,
+		},
+		{
+			name:        "invalid releaseName with periods",
+			chartName:   "chart",
+			releaseName: "namedwithperiods-a.b.c",
+			expectedErrs: []string{
+				fmt.Sprintf(errChartReleaseNameInvalid, "namedwithperiods-a.b.c"),
+			},
+		},
+		{
+			name:         "empty releaseName, valid chartName",
+			chartName:    "valid-chart",
+			releaseName:  "",
+			expectedErrs: nil,
+		},
+		{
+			name:        "empty releaseName and chartName",
+			chartName:   "",
+			releaseName: "",
+			expectedErrs: []string{
+				errChartReleaseNameEmpty,
+			},
+		},
+		{
+			name:        "empty releaseName, invalid chartName",
+			chartName:   "invalid_chart!",
+			releaseName: "",
+			expectedErrs: []string{
+				fmt.Sprintf(errChartReleaseNameInvalid, "invalid_chart!"),
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			err := validateReleaseName(tt.chartName, tt.releaseName)
+			if tt.expectedErrs == nil {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				errs := strings.Split(err.Error(), "\n")
+				require.ElementsMatch(t, tt.expectedErrs, errs)
+			}
+		})
+	}
+}
+
 func TestValidateChart(t *testing.T) {
 	t.Parallel()
 	longName := strings.Repeat("a", ZarfMaxChartNameLength+1)
@@ -196,7 +263,7 @@ func TestValidateChart(t *testing.T) {
 	}{
 		{
 			name:         "valid",
-			chart:        ZarfChart{Name: "chart1", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"},
+			chart:        ZarfChart{Name: "chart1", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0", ReleaseName: "this-is-valid"},
 			expectedErrs: nil,
 		},
 		{
@@ -222,6 +289,26 @@ func TestValidateChart(t *testing.T) {
 				fmt.Sprintf(lang.PkgValidateErrChartURLOrPath, "invalid"),
 			},
 		},
+		// Various release name tests
+		{
+			name:  "invalid releaseName",
+			chart: ZarfChart{ReleaseName: "namedwithperiods-0.47.0", Name: "releaseName", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"},
+			expectedErrs: []string{
+				fmt.Sprintf(errChartReleaseNameInvalid, "namedwithperiods-0.47.0"),
+			},
+		},
+		{
+			name:         "missing releaseName fallsback to name",
+			chart:        ZarfChart{Name: "chart3", Namespace: "namespace", URL: "http://whatever", Version: "v1.0.0"},
+			expectedErrs: nil,
+		},
+		{
+			name:  "missing name and releaseName",
+			chart: ZarfChart{Namespace: "namespace", URL: "http://whatever", Version: "v1.0.0"},
+			expectedErrs: []string{
+				errChartReleaseNameEmpty,
+			},
+		},
 	}
 	for _, tt := range tests {
 		tt := tt