From 7edce9b82bbee66ee8988920d9c16312a319de63 Mon Sep 17 00:00:00 2001 From: vaikas-google Date: Wed, 11 May 2016 15:21:14 -0700 Subject: [PATCH] Validate Chart.yaml version for semver, validate that values.toml parses if present --- pkg/chart/chartfile.go | 12 +++- pkg/chart/chartfile_test.go | 18 ++++++ pkg/chart/testdata/badchartversion/Chart.yaml | 13 +++++ pkg/lint/chartfile.go | 2 +- pkg/lint/lint.go | 1 + pkg/lint/lint_test.go | 57 +++++++++++++++++++ pkg/lint/message.go | 4 +- pkg/lint/message_test.go | 5 ++ pkg/lint/testdata/goodone/Chart.yaml | 3 + .../testdata/goodone/templates/goodone.yaml | 2 + pkg/lint/testdata/goodone/values.toml | 1 + pkg/lint/values.go | 23 ++++++++ 12 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 pkg/chart/testdata/badchartversion/Chart.yaml create mode 100644 pkg/lint/lint_test.go create mode 100644 pkg/lint/testdata/goodone/Chart.yaml create mode 100644 pkg/lint/testdata/goodone/templates/goodone.yaml create mode 100644 pkg/lint/testdata/goodone/values.toml create mode 100644 pkg/lint/values.go diff --git a/pkg/chart/chartfile.go b/pkg/chart/chartfile.go index 23d74c035..21687b435 100644 --- a/pkg/chart/chartfile.go +++ b/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 diff --git a/pkg/chart/chartfile_test.go b/pkg/chart/chartfile_test.go index 975871d03..3c5713499 100644 --- a/pkg/chart/chartfile_test.go +++ b/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 + } +} diff --git a/pkg/chart/testdata/badchartversion/Chart.yaml b/pkg/chart/testdata/badchartversion/Chart.yaml new file mode 100644 index 000000000..7d8fa8189 --- /dev/null +++ b/pkg/chart/testdata/badchartversion/Chart.yaml @@ -0,0 +1,13 @@ +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 diff --git a/pkg/lint/chartfile.go b/pkg/lint/chartfile.go index ff5c833ea..ef1c843b3 100644 --- a/pkg/lint/chartfile.go +++ b/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."}) diff --git a/pkg/lint/lint.go b/pkg/lint/lint.go index 630a8a014..3816cae63 100644 --- a/pkg/lint/lint.go +++ b/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 } diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go new file mode 100644 index 000000000..9759264c0 --- /dev/null +++ b/pkg/lint/lint_test.go @@ -0,0 +1,57 @@ +package lint + +import ( + "strings" + + "testing" +) + +const badChartDir = "testdata/badchartversion" +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 INFO, WARNING and ERROR messages, check for them + var i, w, e = false, false, false + for _, msg := range m { + if msg.Severity == InfoSev { + if strings.Contains(msg.Text, "values.toml") { + i = true + } + } + if msg.Severity == WarningSev { + if strings.Contains(msg.Text, "No templates") { + w = true + } + } + if msg.Severity == ErrorSev { + if strings.Contains(msg.Text, "Chart.yaml does not exist") { + e = true + } + } + } + if !i || !w || !e { + 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") + } + if !strings.Contains(m[0].Text, "deliberateSyntaxError") { + t.Errorf("All didn't have the error for deliberateSyntaxError") + } +} + +func TestGoodChart(t *testing.T) { + m := All(goodChartDir) + if len(m) != 0 { + t.Errorf("All failed but shouldn't have: %#v", m) + } +} diff --git a/pkg/lint/message.go b/pkg/lint/message.go index 240b6015b..ba4eee8ff 100644 --- a/pkg/lint/message.go +++ b/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 { diff --git a/pkg/lint/message_test.go b/pkg/lint/message_test.go index 1d083b369..08ab12ad8 100644 --- a/pkg/lint/message_test.go +++ b/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()) + } } diff --git a/pkg/lint/testdata/goodone/Chart.yaml b/pkg/lint/testdata/goodone/Chart.yaml new file mode 100644 index 000000000..b7d321c41 --- /dev/null +++ b/pkg/lint/testdata/goodone/Chart.yaml @@ -0,0 +1,3 @@ +name: goodone +description: good testing chart +version: 199.44.12345-Alpha.1+cafe009 diff --git a/pkg/lint/testdata/goodone/templates/goodone.yaml b/pkg/lint/testdata/goodone/templates/goodone.yaml new file mode 100644 index 000000000..6c2ceb8db --- /dev/null +++ b/pkg/lint/testdata/goodone/templates/goodone.yaml @@ -0,0 +1,2 @@ +metadata: + name: {{.name | default "foo" | title}} diff --git a/pkg/lint/testdata/goodone/values.toml b/pkg/lint/testdata/goodone/values.toml new file mode 100644 index 000000000..6c50ac9cc --- /dev/null +++ b/pkg/lint/testdata/goodone/values.toml @@ -0,0 +1 @@ +name = "goodone here" diff --git a/pkg/lint/values.go b/pkg/lint/values.go new file mode 100644 index 000000000..206392ed9 --- /dev/null +++ b/pkg/lint/values.go @@ -0,0 +1,23 @@ +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 +}