Merge pull request #907 from prydonius/906-linter-exit-status

fix(lint): Return non-zero exit status when lint errors present

cmd/helm/lint.go

@@ -28,6 +28,7 @@ import (
 
 	"k8s.io/helm/pkg/chartutil"
 	"k8s.io/helm/pkg/lint"
+	"k8s.io/helm/pkg/lint/support"
 )
 
 var longLintHelp = `
@@ -51,6 +52,7 @@ func init() {
 }
 
 var errLintNoChart = errors.New("no chart found for linting (missing Chart.yaml)")
+var errLintFailed = errors.New("lint failed")
 
 func lintCmd(cmd *cobra.Command, args []string) error {
 	path := "."
@@ -92,15 +94,20 @@ func lintChart(path string) error {
 		return errLintNoChart
 	}
 
-	issues := lint.All(path)
+	linter := lint.All(path)
 
-	if len(issues) == 0 {
+	if len(linter.Messages) == 0 {
 		fmt.Println("Lint OK")
+		return nil
 	}
 
-	for _, i := range issues {
+	for _, i := range linter.Messages {
 		fmt.Printf("%s\n", i)
 	}
 
+	if linter.HighestSeverity == support.ErrorSev {
+		return errLintFailed
+	}
+
 	return nil
 }

pkg/lint/lint.go

@@ -24,8 +24,7 @@ import (
 )
 
 // All runs all of the available linters on the given base directory.
-func All(basedir string) []support.Message {
+func All(basedir string) support.Linter {
-
 	// Using abs path to get directory context
 	chartDir, _ := filepath.Abs(basedir)
 
@@ -33,5 +32,5 @@ func All(basedir string) []support.Message {
 	rules.Chartfile(&linter)
 	rules.Values(&linter)
 	rules.Templates(&linter)
-	return linter.Messages
+	return linter
 }

pkg/lint/lint_test.go

@@ -17,9 +17,10 @@ limitations under the License.
 package lint
 
 import (
-	"k8s.io/helm/pkg/lint/support"
 	"strings"
 
+	"k8s.io/helm/pkg/lint/support"
+
 	"testing"
 )
 
@@ -29,7 +30,7 @@ const badYamlFileDir = "rules/testdata/albatross"
 const goodChartDir = "rules/testdata/goodone"
 
 func TestBadChart(t *testing.T) {
-	m := All(badChartDir)
+	m := All(badChartDir).Messages
 	if len(m) != 4 {
 		t.Errorf("Number of errors %v", len(m))
 		t.Errorf("All didn't fail with expected errors, got %#v", m)
@@ -60,7 +61,7 @@ func TestBadChart(t *testing.T) {
 }
 
 func TestInvalidYaml(t *testing.T) {
-	m := All(badYamlFileDir)
+	m := All(badYamlFileDir).Messages
 	if len(m) != 1 {
 		t.Errorf("All didn't fail with expected errors, got %#v", m)
 	}
@@ -70,7 +71,7 @@ func TestInvalidYaml(t *testing.T) {
 }
 
 func TestBadValues(t *testing.T) {
-	m := All(badValuesFileDir)
+	m := All(badValuesFileDir).Messages
 	if len(m) != 1 {
 		t.Errorf("All didn't fail with expected errors, got %#v", m)
 	}
@@ -80,7 +81,7 @@ func TestBadValues(t *testing.T) {
 }
 
 func TestGoodChart(t *testing.T) {
-	m := All(goodChartDir)
+	m := All(goodChartDir).Messages
 	if len(m) != 0 {
 		t.Errorf("All failed but shouldn't have: %#v", m)
 	}

pkg/lint/support/message.go

@@ -44,7 +44,9 @@ type Message struct {
 // Linter encapsulates a linting run of a particular chart.
 type Linter struct {
 	Messages []Message
-	ChartDir string
+	// The highest severity of all the failing lint rules
+	HighestSeverity int
+	ChartDir        string
 }
 
 // LintError describes an error encountered while linting.
@@ -68,6 +70,10 @@ func (l *Linter) RunLinterRule(severity int, lintError LintError) bool {
 
 	if lintError != nil {
 		l.Messages = append(l.Messages, Message{Text: lintError.Error(), Severity: severity})
+
+		if severity > l.HighestSeverity {
+			l.HighestSeverity = severity
+		}
 	}
 	return lintError == nil
 }

pkg/lint/support/message_test.go

@@ -26,26 +26,33 @@ var lintError LintError = fmt.Errorf("Foobar")
 
 func TestRunLinterRule(t *testing.T) {
 	var tests = []struct {
-		Severity         int
+		Severity                int
-		LintError        error
+		LintError               error
-		ExpectedMessages int
+		ExpectedMessages        int
-		ExpectedReturn   bool
+		ExpectedReturn          bool
+		ExpectedHighestSeverity int
 	}{
-		{ErrorSev, lintError, 1, false},
+		{InfoSev, lintError, 1, false, InfoSev},
-		{WarningSev, lintError, 2, false},
+		{WarningSev, lintError, 2, false, WarningSev},
-		{InfoSev, lintError, 3, false},
+		{ErrorSev, lintError, 3, false, ErrorSev},
 		// No error so it returns true
-		{ErrorSev, nil, 3, true},
+		{ErrorSev, nil, 3, true, ErrorSev},
+		// Retains highest severity
+		{InfoSev, lintError, 4, false, ErrorSev},
 		// Invalid severity values
-		{4, lintError, 3, false},
+		{4, lintError, 4, false, ErrorSev},
-		{22, lintError, 3, false},
+		{22, lintError, 4, false, ErrorSev},
-		{-1, lintError, 3, false},
+		{-1, lintError, 4, false, ErrorSev},
 	}
 
 	for _, test := range tests {
 		isValid := linter.RunLinterRule(test.Severity, test.LintError)
 		if len(linter.Messages) != test.ExpectedMessages {
-			t.Errorf("RunLinterRule(%d, %v), linter.Messages should have now %d message, we got %d", test.Severity, test.LintError, test.ExpectedMessages, len(linter.Messages))
+			t.Errorf("RunLinterRule(%d, %v), linter.Messages should now have %d message, we got %d", test.Severity, test.LintError, test.ExpectedMessages, len(linter.Messages))
+		}
+
+		if linter.HighestSeverity != test.ExpectedHighestSeverity {
+			t.Errorf("RunLinterRule(%d, %v), linter.HighestSeverity should be %d, we got %d", test.Severity, test.LintError, test.ExpectedHighestSeverity, linter.HighestSeverity)
 		}
 
 		if isValid != test.ExpectedReturn {