fix: buggy nextY logic for maxOCPVersion

The handling logic of versions that are pre-releases by the
nextY() func (that determines the next Y release) was erroneous.

Eg: nextY("4.16.0") returns "4.17" correctly, but nextY("4.16.0-rc1")
returns "4.16" (the correct value is still "4.17").

This PR fixes the nextY function.

Also has improvement for the "not-upgradeable to next OCP" version message.

Author: anik120

Diff for: pkg/controller/operators/openshift/helpers.go

@@ -105,8 +105,12 @@ func (s skew) String() string {
 	if s.err != nil {
 		return fmt.Sprintf("%s/%s has invalid %s properties: %s", s.namespace, s.name, MaxOpenShiftVersionProperty, s.err)
 	}
-
-	return fmt.Sprintf("%s/%s is incompatible with OpenShift minor versions greater than %s", s.namespace, s.name, s.maxOpenShiftVersion)
+	// is it safe to MustParse maxOpenshiftVersion, with the assumption
+	// that we build a skew object only after verifying that the
+	// version string is parseable.
+	maxOCPVersion := semver.MustParse(s.maxOpenShiftVersion)
+	nextY := nextY(maxOCPVersion).String()
+	return fmt.Sprintf("ClusterServiceVersions blocking upgrade to %s or higher. The maximum supported OCP version for %s/%s is %s", nextY, s.namespace, s.name, s.maxOpenShiftVersion)
 }
 
 type transientError struct {
@@ -131,11 +135,6 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
 		return nil, fmt.Errorf("failed to determine current OpenShift Y-stream release")
 	}
 
-	next, err := nextY(*current)
-	if err != nil {
-		return nil, err
-	}
-
 	csvList := &operatorsv1alpha1.ClusterServiceVersionList{}
 	if err := cli.List(ctx, csvList); err != nil {
 		return nil, &transientError{fmt.Errorf("failed to list ClusterServiceVersions: %w", err)}
@@ -158,11 +157,11 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
 			continue
 		}
 
-		if max == nil || max.GTE(next) {
+		if max == nil || max.GTE(nextY(*current)) {
 			continue
 		}
 
-		s.maxOpenShiftVersion = fmt.Sprintf("%d.%d", max.Major, max.Minor)
+		s.maxOpenShiftVersion = fmt.Sprintf("%d.%d.%d", max.Major, max.Minor, max.Patch)
 
 		incompatible = append(incompatible, s)
 	}
@@ -224,18 +223,8 @@ func getCurrentRelease() (*semver.Version, error) {
 	return currentRelease.version, nil
 }
 
-func nextY(v semver.Version) (semver.Version, error) {
-	v.Build = nil // Builds are irrelevant
-
-	if len(v.Pre) > 0 {
-		// Dropping pre-releases is equivalent to incrementing Y
-		v.Pre = nil
-		v.Patch = 0
-
-		return v, nil
-	}
-
-	return v, v.IncrementMinor() // Sets Y=Y+1 and Z=0
+func nextY(v semver.Version) semver.Version {
+	return semver.Version{Major: v.Major, Minor: v.Minor + 1} // Sets Y=Y+1
 }
 
 const (

Diff for: pkg/controller/operators/openshift/helpers_test.go

@@ -423,7 +423,7 @@ func TestIncompatibleOperators(t *testing.T) {
 		},
 		{
 			description: "ClusterPre",
-			version:     "1.1.0-pre", // Next Y-stream is 1.1.0, NOT 1.2.0
+			version:     "1.1.0-pre", // Next Y-stream is 1.2.0
 			in: skews{
 				{
 					name:                "almond",
@@ -432,8 +432,14 @@ func TestIncompatibleOperators(t *testing.T) {
 				},
 			},
 			expect: expect{
-				err:          false,
-				incompatible: nil,
+				err: false,
+				incompatible: skews{
+					{
+						name:                "almond",
+						namespace:           "default",
+						maxOpenShiftVersion: "1.1",
+					},
+				},
 			},
 		},
 	} {
@@ -597,3 +603,27 @@ func TestNotCopiedSelector(t *testing.T) {
 		})
 	}
 }
+
+func TestOCPVersionNextY(t *testing.T) {
+	for _, tc := range []struct {
+		description     string
+		inVersion       semver.Version
+		expectedVersion semver.Version
+	}{
+		{
+			description:     "Version: 4.16.0. Expected output: 4.17",
+			inVersion:       semver.MustParse("4.16.0"),
+			expectedVersion: semver.MustParse("4.17.0"),
+		},
+		{
+			description:     "Version: 4.16.0-rc1. Expected output: 4.17",
+			inVersion:       semver.MustParse("4.16.0-rc1"),
+			expectedVersion: semver.MustParse("4.17.0"),
+		},
+	} {
+		t.Run(tc.description, func(t *testing.T) {
+			outVersion := nextY(tc.inVersion)
+			require.Equal(t, outVersion, tc.expectedVersion)
+		})
+	}
+}