feat: lint the names of templated resources (#8011)

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

pkg/lint/lint_test.go

@@ -104,7 +104,10 @@ func TestBadValues(t *testing.T) {
 func TestGoodChart(t *testing.T) {
 	m := All(goodChartDir, values, namespace, strict).Messages
 	if len(m) != 0 {
-		t.Errorf("All failed but shouldn't have: %#v", m)
+		t.Error("All returned linter messages when it shouldn't have")
+		for i, msg := range m {
+			t.Logf("Message %d: %s", i, msg)
+		}
 	}
 }
 
@@ -130,6 +133,9 @@ func TestHelmCreateChart(t *testing.T) {
 	m := All(createdChart, values, namespace, true).Messages
 	if ll := len(m); ll != 1 {
 		t.Errorf("All should have had exactly 1 error. Got %d", ll)
+		for i, msg := range m {
+			t.Logf("Message %d: %s", i, msg.Error())
+		}
 	} else if msg := m[0].Err.Error(); !strings.Contains(msg, "icon is recommended") {
 		t.Errorf("Unexpected lint error: %s", msg)
 	}

pkg/lint/rules/template.go

@@ -17,9 +17,11 @@ limitations under the License.
 package rules
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"regexp"
+	"strings"
 
 	"github.com/pkg/errors"
 	"sigs.k8s.io/yaml"
@@ -35,6 +37,14 @@ var (
 	releaseTimeSearch = regexp.MustCompile(`\.Release\.Time`)
 )
 
+// validName is a regular expression for names.
+//
+// This is different than action.ValidName. It conforms to the regular expression
+// `kubectl` says it uses, plus it disallows empty names.
+//
+// For details, see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+var validName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
+
 // Templates lints the templates in the Linter.
 func Templates(linter *support.Linter, values map[string]interface{}, namespace string, strict bool) {
 	path := "templates/"
@@ -57,7 +67,7 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace
 	}
 
 	options := chartutil.ReleaseOptions{
-		Name:      "testRelease",
+		Name:      "test-release",
 		Namespace: namespace,
 	}
 
@@ -111,14 +121,17 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace
 		// linter.RunLinterRule(support.WarningSev, path, validateQuotes(string(preExecutedTemplate)))
 
 		renderedContent := renderedContentMap[filepath.Join(chart.Name(), fileName)]
-		var yamlStruct K8sYamlStruct
-		// Even though K8sYamlStruct only defines Metadata namespace, an error in any other
-		// key will be raised as well
-		err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct)
-
-		// If YAML linting fails, we sill progress. So we don't capture the returned state
-		// on this linter run.
-		linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err))
+		if strings.TrimSpace(renderedContent) != "" {
+			var yamlStruct K8sYamlStruct
+			// Even though K8sYamlStruct only defines a few fields, an error in any other
+			// key will be raised as well
+			err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct)
+
+			// If YAML linting fails, we sill progress. So we don't capture the returned state
+			// on this linter run.
+			linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err))
+			linter.RunLinterRule(support.ErrorSev, path, validateMetadataName(&yamlStruct))
+		}
 	}
 }
 
@@ -149,6 +162,15 @@ func validateYamlContent(err error) error {
 	return errors.Wrap(err, "unable to parse YAML")
 }
 
+func validateMetadataName(obj *K8sYamlStruct) error {
+	// This will return an error if the characters do not abide by the standard OR if the
+	// name is left empty.
+	if validName.MatchString(obj.Metadata.Name) {
+		return nil
+	}
+	return fmt.Errorf("object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name)
+}
+
 func validateNoCRDHooks(manifest []byte) error {
 	if crdHookSearch.Match(manifest) {
 		return errors.New("manifest is a crd-install hook. This hook is no longer supported in v3 and all CRDs should also exist the crds/ directory at the top level of the chart")
@@ -164,9 +186,14 @@ func validateNoReleaseTime(manifest []byte) error {
 }
 
 // K8sYamlStruct stubs a Kubernetes YAML file.
-// Need to access for now to Namespace only
+//
+// DEPRECATED: In Helm 4, this will be made a private type, as it is for use only within
+// the rules package.
 type K8sYamlStruct struct {
-	Metadata struct {
-		Namespace string
-	}
+	Metadata k8sYamlMetadata
+}
+
+type k8sYamlMetadata struct {
+	Namespace string
+	Name      string
 }

pkg/lint/rules/template_test.go

@@ -101,3 +101,32 @@ func TestV3Fail(t *testing.T) {
 		t.Errorf("Unexpected error: %s", res[2].Err)
 	}
 }
+
+func TestValidateMetadataName(t *testing.T) {
+	names := map[string]bool{
+		"":                          false,
+		"foo":                       true,
+		"foo.bar1234baz.seventyone": true,
+		"FOO":                       false,
+		"123baz":                    true,
+		"foo.BAR.baz":               false,
+		"one-two":                   true,
+		"-two":                      false,
+		"one_two":                   false,
+		"a..b":                      false,
+		"%^&#$%*@^*@&#^":            false,
+	}
+	for input, expectPass := range names {
+		obj := K8sYamlStruct{Metadata: k8sYamlMetadata{Name: input}}
+		if err := validateMetadataName(&obj); (err == nil) != expectPass {
+			st := "fail"
+			if expectPass {
+				st = "succeed"
+			}
+			t.Errorf("Expected %q to %s", input, st)
+			if err != nil {
+				t.Log(err)
+			}
+		}
+	}
+}

pkg/lint/rules/testdata/goodone/templates/goodone.yaml

@@ -1,2 +1,2 @@
 metadata:
-  name: {{.Values.name | default "foo" | title}}
+  name: {{ .Values.name | default "foo" | lower }}

pkg/lint/rules/testdata/goodone/values.yaml

@@ -1 +1 @@
-name: "goodone here"
+name: "goodone-here"