From 088c2bd233919a373d311e48e09f4175eca77ba1 Mon Sep 17 00:00:00 2001
From: Ferran Rodenas <frodenas@gmail.com>
Date: Thu, 12 Jan 2017 15:15:01 +0100
Subject: [PATCH] Linter validations for Chart icon property

---
 pkg/lint/lint_test.go                         | 13 ++++---
 pkg/lint/rules/chartfile.go                   | 16 +++++++++
 pkg/lint/rules/chartfile_test.go              | 34 ++++++++++++++++++-
 pkg/lint/rules/testdata/albatross/Chart.yaml  |  1 +
 .../rules/testdata/badvaluesfile/Chart.yaml   |  1 +
 pkg/lint/rules/testdata/goodone/Chart.yaml    |  1 +
 6 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go
index 66541c179..1162ee972 100644
--- a/pkg/lint/lint_test.go
+++ b/pkg/lint/lint_test.go
@@ -31,13 +31,18 @@ const goodChartDir = "rules/testdata/goodone"
 
 func TestBadChart(t *testing.T) {
 	m := All(badChartDir).Messages
-	if len(m) != 4 {
+	if len(m) != 5 {
 		t.Errorf("Number of errors %v", len(m))
 		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, e3 bool
+	// There should be one INFO, 2 WARNINGs and one ERROR messages, check for them
+	var i, w, e, e2, e3 bool
 	for _, msg := range m {
+		if msg.Severity == support.InfoSev {
+			if strings.Contains(msg.Err.Error(), "icon is recommended") {
+				i = true
+			}
+		}
 		if msg.Severity == support.WarningSev {
 			if strings.Contains(msg.Err.Error(), "directory not found") {
 				w = true
@@ -55,7 +60,7 @@ func TestBadChart(t *testing.T) {
 			}
 		}
 	}
-	if !e || !e2 || !e3 || !w {
+	if !e || !e2 || !e3 || !w || !i {
 		t.Errorf("Didn't find all the expected errors, got %#v", m)
 	}
 }
diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go
index f319dca9d..fb94cc4b9 100644
--- a/pkg/lint/rules/chartfile.go
+++ b/pkg/lint/rules/chartfile.go
@@ -54,6 +54,8 @@ func Chartfile(linter *support.Linter) {
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartEngine(chartFile))
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartMaintainer(chartFile))
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartSources(chartFile))
+	linter.RunLinterRule(support.InfoSev, chartFileName, validateChartIconPresence(chartFile))
+	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartIconURL(chartFile))
 }
 
 func validateChartYamlNotDirectory(chartPath string) error {
@@ -152,3 +154,17 @@ func validateChartSources(cf *chart.Metadata) error {
 	}
 	return nil
 }
+
+func validateChartIconPresence(cf *chart.Metadata) error {
+	if cf.Icon == "" {
+		return errors.New("icon is recommended")
+	}
+	return nil
+}
+
+func validateChartIconURL(cf *chart.Metadata) error {
+	if cf.Icon != "" && !govalidator.IsRequestURL(cf.Icon) {
+		return fmt.Errorf("invalid icon URL '%s'", cf.Icon)
+	}
+	return nil
+}
diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go
index 3bec2f1a5..99dc4de0f 100644
--- a/pkg/lint/rules/chartfile_test.go
+++ b/pkg/lint/rules/chartfile_test.go
@@ -194,12 +194,39 @@ func TestValidateChartSources(t *testing.T) {
 	}
 }
 
+func TestValidateChartIconPresence(t *testing.T) {
+	err := validateChartIconPresence(badChart)
+	if err == nil {
+		t.Errorf("validateChartIconPresence to return a linter error, got no error")
+	}
+}
+
+func TestValidateChartIconURL(t *testing.T) {
+	var failTest = []string{"RiverRun", "john@winterfell", "riverrun.io"}
+	var successTest = []string{"http://riverrun.io", "https://riverrun.io", "https://riverrun.io/blackfish.png"}
+	for _, test := range failTest {
+		badChart.Icon = test
+		err := validateChartIconURL(badChart)
+		if err == nil || !strings.Contains(err.Error(), "invalid icon URL") {
+			t.Errorf("validateChartIconURL(%s) to return \"invalid icon URL\", got no error", test)
+		}
+	}
+
+	for _, test := range successTest {
+		badChart.Icon = test
+		err := validateChartSources(badChart)
+		if err != nil {
+			t.Errorf("validateChartIconURL(%s) to return no error, got %s", test, err.Error())
+		}
+	}
+}
+
 func TestChartfile(t *testing.T) {
 	linter := support.Linter{ChartDir: badChartDir}
 	Chartfile(&linter)
 	msgs := linter.Messages
 
-	if len(msgs) != 3 {
+	if len(msgs) != 4 {
 		t.Errorf("Expected 3 errors, got %d", len(msgs))
 	}
 
@@ -214,4 +241,9 @@ func TestChartfile(t *testing.T) {
 	if !strings.Contains(msgs[2].Err.Error(), "version 0.0.0 is less than or equal to 0") {
 		t.Errorf("Unexpected message 2: %s", msgs[2].Err)
 	}
+
+	if !strings.Contains(msgs[3].Err.Error(), "icon is recommended") {
+		t.Errorf("Unexpected message 3: %s", msgs[3].Err)
+	}
+
 }
diff --git a/pkg/lint/rules/testdata/albatross/Chart.yaml b/pkg/lint/rules/testdata/albatross/Chart.yaml
index 4aa22d376..c108fa5e5 100644
--- a/pkg/lint/rules/testdata/albatross/Chart.yaml
+++ b/pkg/lint/rules/testdata/albatross/Chart.yaml
@@ -1,3 +1,4 @@
 name: albatross
 description: testing chart
 version: 199.44.12345-Alpha.1+cafe009
+icon: http://riverrun.io
diff --git a/pkg/lint/rules/testdata/badvaluesfile/Chart.yaml b/pkg/lint/rules/testdata/badvaluesfile/Chart.yaml
index a6ec87d2c..bed845249 100644
--- a/pkg/lint/rules/testdata/badvaluesfile/Chart.yaml
+++ b/pkg/lint/rules/testdata/badvaluesfile/Chart.yaml
@@ -2,3 +2,4 @@ name: badvaluesfile
 description: A Helm chart for Kubernetes
 version: 0.0.1
 home: ""
+icon: http://riverrun.io
diff --git a/pkg/lint/rules/testdata/goodone/Chart.yaml b/pkg/lint/rules/testdata/goodone/Chart.yaml
index b7d321c41..de05463ca 100644
--- a/pkg/lint/rules/testdata/goodone/Chart.yaml
+++ b/pkg/lint/rules/testdata/goodone/Chart.yaml
@@ -1,3 +1,4 @@
 name: goodone
 description: good testing chart
 version: 199.44.12345-Alpha.1+cafe009
+icon: http://riverrun.io