Fix version comparison for versions with preRelease components

Improve unit testing, so reverse comparison of versions also works

Fixes #53055

pkg/util/version/version.go

@@ -181,12 +181,11 @@ func (v *Version) String() string {
 // compareInternal returns -1 if v is less than other, 1 if it is greater than other, or 0
 // if they are equal
 func (v *Version) compareInternal(other *Version) int {
-	for i := range v.components {
+
+	vLen := len(v.components)
+	oLen := len(other.components)
+	for i := 0; i < vLen && i < oLen; i++ {
 		switch {
-		case i >= len(other.components):
-			if v.components[i] != 0 {
-				return 1
-			}
 		case other.components[i] < v.components[i]:
 			return 1
 		case other.components[i] > v.components[i]:
@@ -194,6 +193,14 @@ func (v *Version) compareInternal(other *Version) int {
 		}
 	}
 
+	// If components are common but one has more items and they are not zeros, it is bigger
+	switch {
+	case oLen < vLen && !onlyZeros(v.components[oLen:]):
+		return 1
+	case oLen > vLen && !onlyZeros(other.components[vLen:]):
+		return -1
+	}
+
 	if !v.semver || !other.semver {
 		return 0
 	}
@@ -209,10 +216,7 @@ func (v *Version) compareInternal(other *Version) int {
 
 	vPR := strings.Split(v.preRelease, ".")
 	oPR := strings.Split(other.preRelease, ".")
-	for i := range vPR {
-		if i >= len(oPR) {
-			return 1
-		}
+	for i := 0; i < len(vPR) && i < len(oPR); i++ {
 		vNum, err := strconv.ParseUint(vPR[i], 10, 0)
 		if err == nil {
 			oNum, err := strconv.ParseUint(oPR[i], 10, 0)
@@ -234,9 +238,26 @@ func (v *Version) compareInternal(other *Version) int {
 		}
 	}
 
+	switch {
+	case len(oPR) < len(vPR):
+		return 1
+	case len(oPR) > len(vPR):
+		return -1
+	}
+
 	return 0
 }
 
+// returns false if array contain any non-zero element
+func onlyZeros(array []uint) bool {
+	for _, num := range array {
+		if num != 0 {
+			return false
+		}
+	}
+	return true
+}
+
 // AtLeast tests if a version is at least equal to a given minimum version. If both
 // Versions are Semantic Versions, this will use the Semantic Version comparison
 // algorithm. Otherwise, it will compare only the numeric components, with non-present

pkg/util/version/version_test.go

@@ -45,13 +45,24 @@ func testOne(v *Version, item, prev testItem) error {
 		if err != nil {
 			return fmt.Errorf("unexpected parse error: %v", err)
 		}
+		rv, err := parse(prev.version, v.semver)
+		if err != nil {
+			return fmt.Errorf("unexpected parse error: %v", err)
+		}
+		rcmp, err := rv.Compare(item.version)
+		if err != nil {
+			return fmt.Errorf("unexpected parse error: %v", err)
+		}
+
 		switch {
 		case cmp == -1:
 			return fmt.Errorf("unexpected ordering %q < %q", item.version, prev.version)
 		case cmp == 0 && !item.equalsPrev:
-			return fmt.Errorf("unexpected comparison %q == %q", item.version, item.version)
+			return fmt.Errorf("unexpected comparison %q == %q", item.version, prev.version)
 		case cmp == 1 && item.equalsPrev:
-			return fmt.Errorf("unexpected comparison %q != %q", item.version, item.version)
+			return fmt.Errorf("unexpected comparison %q != %q", item.version, prev.version)
+		case cmp != -rcmp:
+			return fmt.Errorf("unexpected reverse comparison %q <=> %q %v %v %v %v", item.version, prev.version, cmp, rcmp, v.Components(), rv.Components())
 		}
 	}
 
@@ -76,6 +87,8 @@ func TestSemanticVersions(t *testing.T) {
 		{version: "1.0.0-x.7.z.92"},
 		{version: "1.0.0"},
 		{version: "1.0.0+20130313144700", equalsPrev: true},
+		{version: "1.8.0-alpha.3"},
+		{version: "1.8.0-alpha.3.673+73326ef01d2d7c"},
 		{version: "1.9.0"},
 		{version: "1.10.0"},
 		{version: "1.11.0"},
@@ -187,6 +200,7 @@ func TestGenericVersions(t *testing.T) {
 		{version: "1.2", unparsed: "1.2"},
 		{version: "1.2a.3", unparsed: "1.2", equalsPrev: true},
 		{version: "1.2.3", unparsed: "1.2.3"},
+		{version: "1.2.3.0", unparsed: "1.2.3.0", equalsPrev: true},
 		{version: "1.2.3a", unparsed: "1.2.3", equalsPrev: true},
 		{version: "1.2.3-foo.", unparsed: "1.2.3", equalsPrev: true},
 		{version: "1.2.3-.foo", unparsed: "1.2.3", equalsPrev: true},
@@ -201,8 +215,10 @@ func TestGenericVersions(t *testing.T) {
 		{version: "1.2.3.4b3", unparsed: "1.2.3.4", equalsPrev: true},
 		{version: "1.2.3.4.5", unparsed: "1.2.3.4.5"},
 		{version: "1.9.0", unparsed: "1.9.0"},
+		{version: "1.9.0.0.0.0.0.0", unparsed: "1.9.0.0.0.0.0.0", equalsPrev: true},
 		{version: "1.10.0", unparsed: "1.10.0"},
 		{version: "1.11.0", unparsed: "1.11.0"},
+		{version: "1.11.0.0.5", unparsed: "1.11.0.0.5"},
 		{version: "2.0.0", unparsed: "2.0.0"},
 		{version: "2.1.0", unparsed: "2.1.0"},
 		{version: "2.1.1", unparsed: "2.1.1"},