diff --git a/src/types/validate.go b/src/types/validate.go index e045e69520..05289a1781 100644 --- a/src/types/validate.go +++ b/src/types/validate.go @@ -9,14 +9,18 @@ import ( "fmt" "path/filepath" "regexp" + "strings" "github.com/defenseunicorns/pkg/helpers/v2" "github.com/zarf-dev/zarf/src/config/lang" + "k8s.io/apimachinery/pkg/util/validation" ) 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 ( @@ -252,6 +256,29 @@ 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 errs := validation.IsDNS1035Label(releaseName); len(errs) > 0 { + err = fmt.Errorf("invalid release name '%s': %s", releaseName, strings.Join(errs, "; ")) + } + + return +} + // Validate runs all validation checks on a chart. func (chart ZarfChart) Validate() error { var err error @@ -277,6 +304,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..28895a8df8 100644 --- a/src/types/validate_test.go +++ b/src/types/validate_test.go @@ -186,17 +186,82 @@ func TestValidateManifest(t *testing.T) { } } +func TestValidateReleaseName(t *testing.T) { + tests := []struct { + name string + chartName string + releaseName string + expectError bool + errorSubstring string + }{ + { + name: "valid releaseName with hyphens", + chartName: "chart", + releaseName: "valid-release-hyphenated", + expectError: false, + }, + { + name: "valid releaseName with numbers", + chartName: "chart", + releaseName: "valid-0470", + expectError: false, + }, + { + name: "invalid releaseName with periods", + chartName: "chart", + releaseName: "namedwithperiods-a.b.c", + expectError: true, + errorSubstring: "invalid release name 'namedwithperiods-a.b.c'", + }, + { + name: "empty releaseName, valid chartName", + chartName: "valid-chart", + releaseName: "", + expectError: false, + }, + { + name: "empty releaseName and chartName", + chartName: "", + releaseName: "", + expectError: true, + errorSubstring: errChartReleaseNameEmpty, + }, + { + name: "empty releaseName, invalid chartName", + chartName: "invalid_chart!", + releaseName: "", + expectError: true, + errorSubstring: "invalid release name '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.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errorSubstring) + } else { + require.NoError(t, err) + } + }) + } +} + func TestValidateChart(t *testing.T) { t.Parallel() longName := strings.Repeat("a", ZarfMaxChartNameLength+1) tests := []struct { + name string chart ZarfChart expectedErrs []string - name string + partialMatch bool }{ { 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 +287,22 @@ func TestValidateChart(t *testing.T) { fmt.Sprintf(lang.PkgValidateErrChartURLOrPath, "invalid"), }, }, + { + name: "invalid releaseName", + chart: ZarfChart{ReleaseName: "namedwithperiods-0.47.0", Name: "releaseName", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, + expectedErrs: []string{"invalid release name 'namedwithperiods-0.47.0'"}, + partialMatch: true, + }, + { + 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 @@ -232,8 +313,16 @@ func TestValidateChart(t *testing.T) { require.NoError(t, err) return } - errs := strings.Split(err.Error(), "\n") - require.ElementsMatch(t, tt.expectedErrs, errs) + require.Error(t, err) + errString := err.Error() + if tt.partialMatch { + for _, expectedErr := range tt.expectedErrs { + require.Contains(t, errString, expectedErr) + } + } else { + errs := strings.Split(errString, "\n") + require.ElementsMatch(t, tt.expectedErrs, errs) + } }) } }