Merge pull request #704 from vaikas-google/master

Validate Version in Chart.yaml and parse values.toml for helm limt.

pkg/chart/chartfile.go

@@ -19,6 +19,7 @@ package chart
 import (
 	"io/ioutil"
 
+	"github.com/Masterminds/semver"
 	"gopkg.in/yaml.v2"
 )
 
@@ -46,7 +47,16 @@ func LoadChartfile(filename string) (*Chartfile, error) {
 		return nil, err
 	}
 	var y Chartfile
-	return &y, yaml.Unmarshal(b, &y)
+	err = yaml.Unmarshal(b, &y)
+	if err != nil {
+		return nil, err
+	}
+	// Validate that the Version is actually a valid semver version
+	_, err = semver.NewVersion(y.Version)
+	if err != nil {
+		return nil, err
+	}
+	return &y, nil
 }
 
 // Save saves a Chart.yaml file

pkg/chart/chartfile_test.go

@@ -20,6 +20,8 @@ import (
 	"testing"
 )
 
+const badChart = "testdata/badchartversion/Chart.yaml"
+
 func TestLoadChartfile(t *testing.T) {
 	f, err := LoadChartfile(testfile)
 	if err != nil {
@@ -39,3 +41,19 @@ func TestLoadChartfile(t *testing.T) {
 		t.Errorf("Expected https://example.com/foo/bar, got %s", f.Source)
 	}
 }
+
+func TestLoadChartfileFailsWithInvalidVersion(t *testing.T) {
+	f, err := LoadChartfile(badChart)
+	if err == nil {
+		t.Errorf("LoadChartFile didn't fail with invalid version")
+		return
+	}
+	if err.Error() != "Invalid Semantic Version" {
+		t.Errorf("LoadChartFile didn't return the expected error")
+		return
+	}
+	if f != nil {
+		t.Errorf("LoadChartFile returned a chart despite error")
+		return
+	}
+}

pkg/chart/testdata/badchartversion/Chart.yaml

+name: frobnitz
+description: This is a frobniz.
+version: "garbage"
+keywords:
+  - bad bad chart file
+maintainers:
+  - name: The Helm Team
+    email: helm@example.com
+  - name: Someone Else
+    email: nobody@example.com
+source:
+  - https://example.com/foo/bar
+home: http://example.com

pkg/lint/chartfile.go

@@ -13,7 +13,7 @@ func Chartfile(basepath string) (m []Message) {
 
 	path := filepath.Join(basepath, "Chart.yaml")
 	if fi, err := os.Stat(path); err != nil {
-		m = append(m, Message{Severity: ErrorSev, Text: "No Chart.yaml file"})
+		m = append(m, Message{Severity: ErrorSev, Text: "Chart.yaml file: " + path + " does not exist"})
 		return
 	} else if fi.IsDir() {
 		m = append(m, Message{Severity: ErrorSev, Text: "Chart.yaml is a directory."})

pkg/lint/lint.go

@@ -4,5 +4,6 @@ package lint
 func All(basedir string) []Message {
 	out := Chartfile(basedir)
 	out = append(out, Templates(basedir)...)
+	out = append(out, Values(basedir)...)
 	return out
 }

pkg/lint/lint_test.go

+package lint
+
+import (
+	"strings"
+
+	"testing"
+)
+
+const badChartDir = "testdata/badchartfile"
+const badValuesFileDir = "testdata/badvaluesfile"
+const badYamlFileDir = "testdata/albatross"
+const goodChartDir = "testdata/goodone"
+
+func TestBadChart(t *testing.T) {
+	m := All(badChartDir)
+	if len(m) != 3 {
+		t.Errorf("All didn't fail with expected errors, got %#v", m)
+	}
+	// There should be 2 WARNINGs and one ERROR messages, check for them
+	var w, e, e2 = false, false, false
+	for _, msg := range m {
+		if msg.Severity == WarningSev {
+			if strings.Contains(msg.Text, "No templates") {
+				w = true
+			}
+		}
+		if msg.Severity == ErrorSev {
+			if strings.Contains(msg.Text, "must be greater than 0.0.0") {
+				e = true
+			}
+			if strings.Contains(msg.Text, "'name' is required") {
+				e2 = true
+			}
+		}
+	}
+	if !e || !e2 || !w {
+		t.Errorf("Didn't find all the expected errors, got %#v", m)
+	}
+}
+
+func TestInvalidYaml(t *testing.T) {
+	m := All(badYamlFileDir)
+	if len(m) != 1 {
+		t.Errorf("All didn't fail with expected errors, got %#v", m)
+	}
+	if !strings.Contains(m[0].Text, "deliberateSyntaxError") {
+		t.Errorf("All didn't have the error for deliberateSyntaxError")
+	}
+}
+
+func TestBadValues(t *testing.T) {
+	m := All(badValuesFileDir)
+	if len(m) != 1 {
+		t.Errorf("All didn't fail with expected errors, got %#v", m)
+	}
+	if !strings.Contains(m[0].Text, "Bare keys cannot contain ':'") {
+		t.Errorf("All didn't have the error for invalid key format")
+	}
+}
+
+func TestGoodChart(t *testing.T) {
+	m := All(goodChartDir)
+	if len(m) != 0 {
+		t.Errorf("All failed but shouldn't have: %#v", m)
+	}
+}

pkg/lint/message.go

@@ -8,6 +8,8 @@ type Severity int
 const (
 	// UnknownSev indicates that the severity of the error is unknown, and should not stop processing.
 	UnknownSev = iota
+	// InfoSev indicates information, for example missing values.toml file
+	InfoSev
 	// WarningSev indicates that something does not meet code standards, but will likely function.
 	WarningSev
 	// ErrorSev indicates that something will not likely function.
@@ -15,7 +17,7 @@ const (
 )
 
 // sev matches the *Sev states.
-var sev = []string{"INFO", "WARNING", "ERROR"}
+var sev = []string{"UNKNOWN", "INFO", "WARNING", "ERROR"}
 
 // Message is a linting output message
 type Message struct {

pkg/lint/message_test.go

@@ -17,4 +17,9 @@ func TestMessage(t *testing.T) {
 	if m.String() != "[WARNING] Bar" {
 		t.Errorf("Unexpected output: %s", m.String())
 	}
+
+	m = Message{InfoSev, "FooBar"}
+	if m.String() != "[INFO] FooBar" {
+		t.Errorf("Unexpected output: %s", m.String())
+	}
 }

pkg/lint/testdata/badvaluesfile/Chart.yaml

+name: badvaluesfile
+description: A Helm chart for Kubernetes
+version: 0.0.1
+home: ""

pkg/lint/testdata/badvaluesfile/templates/badvaluesfile.yaml

+metadata:
+  name: {{.name | default "foo" | title}}

pkg/lint/testdata/badvaluesfile/values.toml

+# Invalid value for badvaluesfile for testing lint fails with invalid toml format
+name: "value"

pkg/lint/testdata/goodone/Chart.yaml

+name: goodone
+description: good testing chart
+version: 199.44.12345-Alpha.1+cafe009

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

+metadata:
+  name: {{.name | default "foo" | title}}

pkg/lint/testdata/goodone/values.toml

+name = "goodone here"

pkg/lint/values.go

+package lint
+
+import (
+	"os"
+
+	"github.com/kubernetes/helm/pkg/chart"
+	"path/filepath"
+)
+
+// Values lints a chart's values.toml file.
+func Values(basepath string) (messages []Message) {
+	vf := filepath.Join(basepath, "values.toml")
+	messages = []Message{}
+	if _, err := os.Stat(vf); err != nil {
+		messages = append(messages, Message{Severity: InfoSev, Text: "No values.toml file"})
+		return
+	}
+	_, err := chart.ReadValuesFile(vf)
+	if err != nil {
+		messages = append(messages, Message{Severity: ErrorSev, Text: err.Error()})
+	}
+	return messages
+}