From 9e5531e773bcbc210aea80d73166c114090269e2 Mon Sep 17 00:00:00 2001
From: Moynur <moynur.github@gmail.com>
Date: Mon, 18 Mar 2024 23:26:07 +0000
Subject: [PATCH] feat: improve helm chart name validation

Signed-off-by: Moynur <moynur.github@gmail.com>
---
 pkg/lint/rules/chartfile.go      | 48 ++++++++++++++++++++-
 pkg/lint/rules/chartfile_test.go | 74 ++++++++++++++++++++++++++------
 2 files changed, 109 insertions(+), 13 deletions(-)

diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go
index 724c3f2ea..a86b08cda 100644
--- a/pkg/lint/rules/chartfile.go
+++ b/pkg/lint/rules/chartfile.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/asaskevich/govalidator"
@@ -112,14 +113,59 @@ func validateChartYamlStrictFormat(chartFileError error) error {
 	return nil
 }
 
+const (
+	chartNamePattern      = "^[a-z0-9-]*$"
+	chartNameStartPattern = "^[a-z]"
+)
+
+type chartNameValidator func(*chart.Metadata) error
+
 func validateChartName(cf *chart.Metadata) error {
+	rules := []chartNameValidator{
+		isNotEmpty,
+		nameIsNotPath,
+		doesNotContainInvalidCharacters,
+		beginsWithAlphabeticCharacter,
+	}
+	for _, rule := range rules {
+		if err := rule(cf); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func isNotEmpty(cf *chart.Metadata) error {
 	if cf.Name == "" {
 		return errors.New("name is required")
 	}
+	return nil
+}
+
+func nameIsNotPath(cf *chart.Metadata) error {
 	name := filepath.Base(cf.Name)
 	if name != cf.Name {
-		return fmt.Errorf("chart name %q is invalid", cf.Name)
+		return fmt.Errorf("chart name '%s' should not include a path", cf.Name)
+	}
+	return nil
+}
+
+func doesNotContainInvalidCharacters(cf *chart.Metadata) error {
+	match, _ := regexp.MatchString(chartNamePattern, cf.Name)
+	if !match {
+		return errors.New("name must not contain any upper case letters or any special characters other than '-'")
 	}
+
+	return nil
+}
+
+func beginsWithAlphabeticCharacter(cf *chart.Metadata) error {
+	match, _ := regexp.MatchString(chartNameStartPattern, cf.Name[0:1])
+	if !match {
+		return errors.New("name must begin with a lowercase alphabetic character (a-z)")
+	}
+
 	return nil
 }
 
diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go
index bbb14a5e8..f04df025e 100644
--- a/pkg/lint/rules/chartfile_test.go
+++ b/pkg/lint/rules/chartfile_test.go
@@ -18,6 +18,7 @@ package rules
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -26,6 +27,8 @@ import (
 	chart "helm.sh/helm/v4/pkg/chart/v2"
 	chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
 	"helm.sh/helm/v4/pkg/lint/support"
+
+	"github.com/stretchr/testify/assert"
 )
 
 const (
@@ -66,18 +69,6 @@ func TestValidateChartYamlFormat(t *testing.T) {
 	}
 }
 
-func TestValidateChartName(t *testing.T) {
-	err := validateChartName(badChart)
-	if err == nil {
-		t.Errorf("validateChartName to return a linter error, got no error")
-	}
-
-	err = validateChartName(badChartName)
-	if err == nil {
-		t.Error("expected validateChartName to return a linter error for an invalid name, got no error")
-	}
-}
-
 func TestValidateChartVersion(t *testing.T) {
 	var failTest = []struct {
 		Version  string
@@ -272,3 +263,62 @@ func TestChartfile(t *testing.T) {
 		}
 	})
 }
+
+func TestValidateChartName(t *testing.T) {
+	tests := []struct {
+		name    string
+		cm      *chart.Metadata
+		wantErr assert.ErrorAssertionFunc
+	}{
+		{
+			name:    "no chart name so returns an error",
+			cm:      &chart.Metadata{Name: ""},
+			wantErr: assert.Error,
+		},
+		{
+			name:    "chart name begins with a number so returns an error",
+			cm:      &chart.Metadata{Name: "1at-the-beginning"},
+			wantErr: assert.Error,
+		},
+		{
+			name:    "valid chart name with dashes so returns no error",
+			cm:      &chart.Metadata{Name: "good-chart-name"},
+			wantErr: assert.NoError,
+		},
+		{
+			name:    "valid chart name without dashes so returns no error",
+			cm:      &chart.Metadata{Name: "name"},
+			wantErr: assert.NoError,
+		},
+		{
+			name:    "chart name has special characters which isn't a dash so returns error",
+			cm:      &chart.Metadata{Name: "some.odd?chart,name!"},
+			wantErr: assert.Error,
+		},
+		{
+			name:    "chart name has underscore special characters which isn't a dash so returns error",
+			cm:      &chart.Metadata{Name: "some_slightly_wrong_chart_name"},
+			wantErr: assert.Error,
+		},
+		{
+			name:    "chart name has upper case characters so returns error",
+			cm:      &chart.Metadata{Name: "bad-Chart-name"},
+			wantErr: assert.Error,
+		},
+		{
+			name:    "bad chart name",
+			cm:      badChartName,
+			wantErr: assert.Error,
+		},
+		{
+			name:    "bad chart",
+			cm:      badChart,
+			wantErr: assert.Error,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.wantErr(t, validateChartName(tt.cm), fmt.Sprintf("validateChartName(%v)", tt.cm.Name))
+		})
+	}
+}