Merge pull request #31064 from kamilswiec/main

lint: throw warning when chart version is not semverv2

internal/chart/v3/lint/lint_test.go

@@ -60,7 +60,7 @@ func TestBadChartV3(t *testing.T) {
 			}
 		}
 		if msg.Severity == support.ErrorSev {
-			if strings.Contains(msg.Err.Error(), "version '0.0.0.0' is not a valid SemVer") {
+			if strings.Contains(msg.Err.Error(), "version '0.0.0.0' is not a valid SemVerV2") {
 				e = true
 			}
 			if strings.Contains(msg.Err.Error(), "name is required") {

internal/chart/v3/lint/rules/chartfile.go

@@ -140,9 +140,9 @@ func validateChartVersion(cf *chart.Metadata) error {
 		return errors.New("version is required")
 	}
 
-	version, err := semver.NewVersion(cf.Version)
+	version, err := semver.StrictNewVersion(cf.Version)
 	if err != nil {
-		return fmt.Errorf("version '%s' is not a valid SemVer", cf.Version)
+		return fmt.Errorf("version '%s' is not a valid SemVerV2", cf.Version)
 	}
 
 	c, err := semver.NewConstraint(">0.0.0-0")

internal/chart/v3/lint/rules/chartfile_test.go

@@ -84,9 +84,11 @@ func TestValidateChartVersion(t *testing.T) {
 		ErrorMsg string
 	}{
 		{"", "version is required"},
-		{"1.2.3.4", "version '1.2.3.4' is not a valid SemVer"},
+		{"1.2.3.4", "version '1.2.3.4' is not a valid SemVerV2"},
-		{"waps", "'waps' is not a valid SemVer"},
+		{"waps", "'waps' is not a valid SemVerV2"},
-		{"-3", "'-3' is not a valid SemVer"},
+		{"-3", "'-3' is not a valid SemVerV2"},
+		{"1.1", "'1.1' is not a valid SemVerV2"},
+		{"1", "'1' is not a valid SemVerV2"},
 	}
 
 	var successTest = []string{"0.0.1", "0.0.1+build", "0.0.1-beta"}

pkg/chart/v2/lint/lint_test.go

@@ -42,12 +42,12 @@ const invalidChartFileDir = "rules/testdata/invalidchartfile"
 
 func TestBadChart(t *testing.T) {
 	m := RunAll(badChartDir, values, namespace).Messages
-	if len(m) != 8 {
+	if len(m) != 9 {
 		t.Errorf("Number of errors %v", len(m))
 		t.Errorf("All didn't fail with expected errors, got %#v", m)
 	}
-	// There should be one INFO, one WARNING, and 2 ERROR messages, check for them
+	// There should be one INFO, 2 WARNING and 2 ERROR messages, check for them
-	var i, w, e, e2, e3, e4, e5, e6 bool
+	var i, w, w2, e, e2, e3, e4, e5, e6 bool
 	for _, msg := range m {
 		if msg.Severity == support.InfoSev {
 			if strings.Contains(msg.Err.Error(), "icon is recommended") {
@@ -83,8 +83,13 @@ func TestBadChart(t *testing.T) {
 				e6 = true
 			}
 		}
+		if msg.Severity == support.WarningSev {
+			if strings.Contains(msg.Err.Error(), "version '0.0.0.0' is not a valid SemVerV2") {
+				w2 = true
+			}
+		}
 	}
-	if !e || !e2 || !e3 || !e4 || !e5 || !i || !e6 || !w {
+	if !e || !e2 || !e3 || !e4 || !e5 || !i || !e6 || !w || !w2 {
 		t.Errorf("Didn't find all the expected errors, got %#v", m)
 	}
 }

pkg/chart/v2/lint/rules/chartfile.go

@@ -67,6 +67,7 @@ func Chartfile(linter *support.Linter) {
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartIconURL(chartFile))
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartType(chartFile))
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartDependencies(chartFile))
+	linter.RunLinterRule(support.WarningSev, chartFileName, validateChartVersionStrictSemVerV2(chartFile))
 }
 
 func validateChartVersionType(data map[string]interface{}) error {
@@ -158,6 +159,16 @@ func validateChartVersion(cf *chart.Metadata) error {
 	return nil
 }
 
+func validateChartVersionStrictSemVerV2(cf *chart.Metadata) error {
+	_, err := semver.StrictNewVersion(cf.Version)
+
+	if err != nil {
+		return fmt.Errorf("version '%s' is not a valid SemVerV2", cf.Version)
+	}
+
+	return nil
+}
+
 func validateChartMaintainer(cf *chart.Metadata) error {
 	for _, maintainer := range cf.Maintainers {
 		if maintainer == nil {

pkg/chart/v2/lint/rules/chartfile_test.go

@@ -108,6 +108,35 @@ func TestValidateChartVersion(t *testing.T) {
 	}
 }
 
+func TestValidateChartVersionStrictSemVerV2(t *testing.T) {
+	var failTest = []struct {
+		Version  string
+		ErrorMsg string
+	}{
+		{"", "version '' is not a valid SemVerV2"},
+		{"1", "version '1' is not a valid SemVerV2"},
+		{"1.1", "version '1.1' is not a valid SemVerV2"},
+	}
+
+	var successTest = []string{"1.1.1", "0.0.1+build", "0.0.1-beta"}
+
+	for _, test := range failTest {
+		badChart.Version = test.Version
+		err := validateChartVersionStrictSemVerV2(badChart)
+		if err == nil || !strings.Contains(err.Error(), test.ErrorMsg) {
+			t.Errorf("validateChartVersion(%s) to return \"%s\", got no error", test.Version, test.ErrorMsg)
+		}
+	}
+
+	for _, version := range successTest {
+		badChart.Version = version
+		err := validateChartVersionStrictSemVerV2(badChart)
+		if err != nil {
+			t.Errorf("validateChartVersion(%s) to return no error, got a linter error", version)
+		}
+	}
+}
+
 func TestValidateChartMaintainer(t *testing.T) {
 	var failTest = []struct {
 		Name     string
@@ -226,7 +255,7 @@ func TestChartfile(t *testing.T) {
 		linter := support.Linter{ChartDir: badChartDir}
 		Chartfile(&linter)
 		msgs := linter.Messages
-		expectedNumberOfErrorMessages := 6
+		expectedNumberOfErrorMessages := 7
 
 		if len(msgs) != expectedNumberOfErrorMessages {
 			t.Errorf("Expected %d errors, got %d", expectedNumberOfErrorMessages, len(msgs))
@@ -256,13 +285,16 @@ func TestChartfile(t *testing.T) {
 		if !strings.Contains(msgs[5].Err.Error(), "dependencies are not valid in the Chart file with apiVersion") {
 			t.Errorf("Unexpected message 5: %s", msgs[5].Err)
 		}
+		if !strings.Contains(msgs[6].Err.Error(), "version '0.0.0.0' is not a valid SemVerV2") {
+			t.Errorf("Unexpected message 6: %s", msgs[6].Err)
+		}
 	})
 
 	t.Run("Chart.yaml validity issues due to type mismatch", func(t *testing.T) {
 		linter := support.Linter{ChartDir: anotherBadChartDir}
 		Chartfile(&linter)
 		msgs := linter.Messages
-		expectedNumberOfErrorMessages := 3
+		expectedNumberOfErrorMessages := 4
 
 		if len(msgs) != expectedNumberOfErrorMessages {
 			t.Errorf("Expected %d errors, got %d", expectedNumberOfErrorMessages, len(msgs))
@@ -280,5 +312,8 @@ func TestChartfile(t *testing.T) {
 		if !strings.Contains(msgs[2].Err.Error(), "appVersion should be of type string") {
 			t.Errorf("Unexpected message 2: %s", msgs[2].Err)
 		}
+		if !strings.Contains(msgs[3].Err.Error(), "version '7.2445e+06' is not a valid SemVerV2") {
+			t.Errorf("Unexpected message 3: %s", msgs[3].Err)
+		}
 	})
 }

pkg/chart/v2/metadata.go

@@ -52,7 +52,7 @@ type Metadata struct {
 	Home string `json:"home,omitempty"`
 	// Source is the URL to the source code of this chart
 	Sources []string `json:"sources,omitempty"`
-	// A SemVer 2 conformant version string of the chart. Required.
+	// A version string of the chart. Required.
 	Version string `json:"version,omitempty"`
 	// A one-sentence description of the chart
 	Description string `json:"description,omitempty"`

pkg/cmd/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt

@@ -9,6 +9,7 @@
 [ERROR] Chart.yaml: apiVersion is required. The value must be either "v1" or "v2"
 [ERROR] Chart.yaml: version is required
 [INFO] Chart.yaml: icon is recommended
+[WARNING] Chart.yaml: version '' is not a valid SemVerV2
 [WARNING] templates/: directory does not exist
 [ERROR] : unable to load chart
 	validation: chart.metadata.name is required