From c0d739fa883f9bb2a7b2782e1488a523f7e5157a Mon Sep 17 00:00:00 2001
From: Sascha Grunert <sgrunert@redhat.com>
Date: Tue, 14 May 2024 12:08:22 +0200
Subject: [PATCH] Fix version comparison for final version after RC

Fixing the very specific use case of the failing semver comparison
mentioned in https://github.com/kubernetes/kubernetes/issues/117115

Follow-up on https://github.com/kubernetes/release/pull/3223 and
https://github.com/kubernetes/release/pull/3254

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
---
 pkg/release/publish.go      | 29 ++++++++++++++++++++-
 pkg/release/publish_test.go | 52 +++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/pkg/release/publish.go b/pkg/release/publish.go
index 6fafbe255ad..b2d76ca2632 100644
--- a/pkg/release/publish.go
+++ b/pkg/release/publish.go
@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/blang/semver/v4"
 	"github.com/sirupsen/logrus"
 
 	"sigs.k8s.io/release-sdk/gcli"
@@ -283,7 +284,7 @@ func (p *Publisher) VerifyLatestUpdate(
 		return false, fmt.Errorf("invalid GCS version format %s", gcsVersion)
 	}
 
-	if sv.LTE(gcsSemverVersion) {
+	if IsUpToDate(gcsSemverVersion, sv) {
 		logrus.Infof(
 			"Not updating version, because %s <= %s", version, gcsVersion,
 		)
@@ -294,6 +295,32 @@ func (p *Publisher) VerifyLatestUpdate(
 	return true, nil
 }
 
+func IsUpToDate(oldVersion, newVersion semver.Version) bool {
+	oldPre := oldVersion.Pre
+	newPre := newVersion.Pre
+
+	oldStrippedPre := semver.Version{Major: oldVersion.Major, Minor: oldVersion.Minor, Patch: oldVersion.Patch}
+	newStrippedPre := semver.Version{Major: newVersion.Major, Minor: newVersion.Minor, Patch: newVersion.Patch}
+
+	// Verfy specific use case in our tagging logic:
+	// 1.30.0-rc.2.10+00000000000000
+	// needs to be considered lower than
+	// 1.30.0-11+00000000000000
+	// which is not given by newVersion.LTE(oldVersion) below.
+	if len(oldPre) == 3 && // [rc 2 10]
+		oldPre[0].String() == "rc" &&
+		len(newPre) == 1 && // [11]
+		newPre[0].IsNum &&
+		// For example when:
+		// oldVersion: 1.29.0-rc.0.20+00000000000000
+		// newVersion: 1.28.1-2+00000000000000
+		newStrippedPre.GE(oldStrippedPre) {
+		return false
+	}
+
+	return newVersion.LTE(oldVersion)
+}
+
 // PublishToGcs publishes a release to GCS
 // publishFile - the GCS location to look in
 // buildDir - build output directory
diff --git a/pkg/release/publish_test.go b/pkg/release/publish_test.go
index f76d1709cbd..6edb1602aa1 100644
--- a/pkg/release/publish_test.go
+++ b/pkg/release/publish_test.go
@@ -21,6 +21,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/blang/semver/v4"
 	"github.com/stretchr/testify/require"
 	"k8s.io/release/pkg/release"
 	"k8s.io/release/pkg/release/releasefakes"
@@ -325,3 +326,54 @@ func TestPublishReleaseNotesIndex(t *testing.T) {
 		}
 	}
 }
+
+func TestIsUpToDate(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name, oldVersion, newVersion string
+		expected                     bool
+	}{
+		{
+			name:       "Final version after RC",
+			oldVersion: "1.30.0-rc.2.10+00000000000000",
+			newVersion: "1.30.0-11+00000000000000",
+			expected:   false,
+		},
+		{
+			name:       "More commits",
+			oldVersion: "1.30.0-10+00000000000000",
+			newVersion: "1.30.0-11+00000000000000",
+			expected:   false,
+		},
+		{
+			name:       "Newer release",
+			oldVersion: "1.29.0-0+00000000000000",
+			newVersion: "1.29.1-0+00000000000000",
+			expected:   false,
+		},
+		{
+			name:       "Counter reset after RC",
+			oldVersion: "1.30.0-rc.2.10+00000000000000",
+			newVersion: "1.30.0-1+00000000000000",
+			expected:   false,
+		},
+		{
+			name:       "Patch after newer RC (artificial corner case)",
+			oldVersion: "1.29.0-rc.0.20+00000000000000",
+			newVersion: "1.28.1-2+00000000000000",
+			expected:   true,
+		},
+	} {
+		oldVersion := semver.MustParse(tc.oldVersion)
+		newVersion := semver.MustParse(tc.newVersion)
+		expected := tc.expected
+
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			res := release.IsUpToDate(oldVersion, newVersion)
+			require.Equal(t, expected, res)
+		})
+	}
+}