From 2658ff43b0a040fca86e34dbe2e91388ffaa2a8c Mon Sep 17 00:00:00 2001 From: jamestexas <18285880+jamestexas@users.noreply.github.com> Date: Mon, 29 Jul 2024 17:48:37 -0500 Subject: [PATCH] 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 --- src/types/validate.go | 33 +++++++++++++- src/types/validate_test.go | 89 +++++++++++++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/src/types/validate.go b/src/types/validate.go index e045e69520..696e926784 100644 --- 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 index b0ebc60e8f..ae43da3384 100644 --- 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