OPRUN-4599: override catalog image tag when Helm values pin v5.0

cmd/cluster-olm-operator/main.go

@@ -230,6 +230,13 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error
 
 	log := klog.FromContext(ctx)
 
+	operatorImageVersion := status.VersionForOperatorFromEnv()
+	currentOCPMinorVersion, err := versionutils.GetCurrentOCPMinorVersion(operatorImageVersion)
+	if err != nil {
+		return err
+	}
+	catalogImageTag := versionutils.GetCatalogImageTag(currentOCPMinorVersion)
+
 	fg, err := cl.ConfigClient.ConfigV1().FeatureGates().Get(ctx, "cluster", metav1.GetOptions{})
 	if err != nil {
 		return fmt.Errorf("unable to retrieve featureSet: %w", err)
@@ -252,8 +259,9 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error
 				Scope:            meta.RESTScopeRoot,
 			},
 		},
-		FeatureGate:    *fg,
-		Infrastructure: infra,
+		FeatureGate:            *fg,
+		Infrastructure:         infra,
+		ClusterCatalogImageTag: catalogImageTag,
 	}
 
 	staticResourceControllers, deploymentControllers, clusterCatalogControllers, relatedObjects, err := cb.BuildControllers("catalogd", "operator-controller")
@@ -288,12 +296,6 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error
 		clusterCatalogControllerList = append(clusterCatalogControllerList, controller)
 	}
 
-	operatorImageVersion := status.VersionForOperatorFromEnv()
-	currentOCPMinorVersion, err := versionutils.GetCurrentOCPMinorVersion(operatorImageVersion)
-	if err != nil {
-		return err
-	}
-
 	upgradeableConditionController := controller.NewStaticUpgradeableConditionController(
 		"OLMStaticUpgradeableConditionController",
 		cl.OperatorClient,
@@ -338,7 +340,7 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error
 	)
 
 	versionGetter := status.NewVersionGetter()
-	versionGetter.SetVersion("operator", status.VersionForOperatorFromEnv())
+	versionGetter.SetVersion("operator", operatorImageVersion)
 
 	// Add all resources to relatedObjects to ensure that must-gather picks them up.
 	// Note: These resources are also hard-coded in the ClusterOperator manifest. This way,

internal/versionutils/version_utils.go

@@ -61,6 +61,12 @@ func ToAllowedSemver(data []byte) (*semver.Version, error) {
 	return &version, nil
 }
 
+// GetCatalogImageTag converts an OCP version to the catalog image tag format used by default catalogs.
+// For example, version 4.22.0 returns "v4.22", and version 5.0.0 returns "v5.0".
+func GetCatalogImageTag(version *semver.Version) string {
+	return fmt.Sprintf("v%d.%d", version.Major, version.Minor)
+}
+
 // ocpVersion500 is the semver representation of OCP 5.0, which is co-released with 4.23 as an
 // equivalent release. Neither upgrades to the other; both upgrade exclusively to 5.1.
 var ocpVersion500 = semver.Version{Major: 5, Minor: 0, Patch: 0}

internal/versionutils/version_utils_test.go

@@ -95,6 +95,47 @@ func TestToAllowedSemver(t *testing.T) {
 	}
 }
 
+func TestGetCatalogImageTag(t *testing.T) {
+	tests := []struct {
+		name    string
+		version semver.Version
+		want    string
+	}{
+		{
+			name:    "4.22",
+			version: semver.Version{Major: 4, Minor: 22, Patch: 0},
+			want:    "v4.22",
+		},
+		{
+			name:    "4.23",
+			version: semver.Version{Major: 4, Minor: 23, Patch: 0},
+			want:    "v4.23",
+		},
+		{
+			name:    "5.0",
+			version: semver.Version{Major: 5, Minor: 0, Patch: 0},
+			want:    "v5.0",
+		},
+		{
+			name:    "5.1",
+			version: semver.Version{Major: 5, Minor: 1, Patch: 0},
+			want:    "v5.1",
+		},
+		{
+			name:    "patch ignored",
+			version: semver.Version{Major: 4, Minor: 22, Patch: 5},
+			want:    "v4.22",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := GetCatalogImageTag(&tt.version)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
 func TestIsOperatorMaxOCPVersionCompatibleWithCluster(t *testing.T) {
 	tests := []struct {
 		name                   string

pkg/controller/builder.go

@@ -40,12 +40,13 @@ import (
 )
 
 type Builder struct {
-	Assets            string
-	Clients           *clients.Clients
-	ControllerContext *controllercmd.ControllerContext
-	KnownRESTMappings map[schema.GroupVersionKind]*meta.RESTMapping
-	FeatureGate       configv1.FeatureGate
-	Infrastructure    *configv1.Infrastructure
+	Assets                 string
+	Clients                *clients.Clients
+	ControllerContext      *controllercmd.ControllerContext
+	KnownRESTMappings      map[schema.GroupVersionKind]*meta.RESTMapping
+	FeatureGate            configv1.FeatureGate
+	Infrastructure         *configv1.Infrastructure
+	ClusterCatalogImageTag string
 }
 
 func (b *Builder) BuildControllers(subDirectories ...string) (map[string]factory.Controller, map[string]factory.Controller, map[string]factory.Controller, []configv1.ObjectReference, error) {

pkg/controller/helm.go

@@ -80,6 +80,14 @@ func (b *Builder) renderHelmTemplate(helmPath, manifestDir string) error {
 	if err := values.SetStringValue("options.operatorController.deployment.image", os.Getenv("OPERATOR_CONTROLLER_IMAGE")); err != nil {
 		return fmt.Errorf("error setting OPERATOR_CONTROLLER_IMAGE: %w", err)
 	}
+	// OPRUN-4599: OCP 4.23 and 5.0 are co-released from the same branch, so
+	// the openshift.yaml values file pins options.openshift.catalogs.version to
+	// "v5.0". A cluster running 4.23 must use v4.23 catalog images instead.
+	// TODO: Remove this block once 4.23/5.0 are no longer co-released and the
+	// openshift.yaml values file no longer pins "v5.0".
+	if err := applyCatalogImageTagOverride(values, b.ClusterCatalogImageTag); err != nil {
+		return fmt.Errorf("error setting catalog image tag: %w", err)
+	}
 
 	// On HighlyAvailable topologies scale to 2 replicas and enable the PDB so that rolling
 	// updates never leave zero running pods. On SingleReplica (SNO) / External topologies
@@ -222,6 +230,21 @@ type DocumentInfo struct {
 	Order    int
 }
 
+// applyCatalogImageTagOverride overrides options.openshift.catalogs.version in the
+// Helm values when the values file pins "v5.0" but the cluster runs a different
+// version (e.g. OCP 4.23, which is co-released with 5.0 but needs v4.23 catalog
+// images). It is a no-op when clusterTag is empty or the pinned tag is not "v5.0".
+func applyCatalogImageTagOverride(values *helmvalues.HelmValues, clusterTag string) error {
+	if clusterTag == "" {
+		return nil
+	}
+	currentTag, found := values.GetStringValue("options.openshift.catalogs.version")
+	if !found || currentTag != "v5.0" {
+		return nil
+	}
+	return values.SetStringValue("options.openshift.catalogs.version", clusterTag)
+}
+
 func splitYAMLDocuments(content string) []string {
 	// Split by document separators but preserve the original text
 	lines := strings.Split(content, "\n")

pkg/controller/helm_test.go

@@ -14,6 +14,7 @@ import (
 
 	internalfeatures "github.com/openshift/cluster-olm-operator/internal/featuregates"
 	"github.com/openshift/cluster-olm-operator/pkg/clients"
+	"github.com/openshift/cluster-olm-operator/pkg/helmvalues"
 )
 
 func TestRenderHelmTemplate(t *testing.T) {
@@ -75,6 +76,74 @@ func TestRenderHelmTemplate(t *testing.T) {
 	require.Equal(t, compareData, testData)
 }
 
+func TestApplyCatalogImageTagOverride(t *testing.T) {
+	const versionKey = "options.openshift.catalogs.version"
+
+	tests := []struct {
+		name        string
+		initialTag  string // set at versionKey before the call; empty means key is absent
+		clusterTag  string
+		expectedTag string // expected value at versionKey after the call; empty means key absent
+	}{
+		{
+			name:        "clusterTag empty - no override regardless of Helm value",
+			initialTag:  "v5.0",
+			clusterTag:  "",
+			expectedTag: "v5.0",
+		},
+		{
+			name:        "Helm pins v5.0, cluster is 4.23 - override to v4.23",
+			initialTag:  "v5.0",
+			clusterTag:  "v4.23",
+			expectedTag: "v4.23",
+		},
+		{
+			name:        "Helm pins v5.0, cluster is also 5.0 - no-op",
+			initialTag:  "v5.0",
+			clusterTag:  "v5.0",
+			expectedTag: "v5.0",
+		},
+		{
+			name:        "Helm pins v5.0, cluster is 5.1 - override to v5.1",
+			initialTag:  "v5.0",
+			clusterTag:  "v5.1",
+			expectedTag: "v5.1",
+		},
+		{
+			name:        "Helm pins v4.23, cluster is 4.23 - guard fires only on v5.0, no override",
+			initialTag:  "v4.23",
+			clusterTag:  "v4.23",
+			expectedTag: "v4.23",
+		},
+		{
+			name:        "Helm pins v4.22, cluster tag provided - guard fires only on v5.0, no override",
+			initialTag:  "v4.22",
+			clusterTag:  "v4.22",
+			expectedTag: "v4.22",
+		},
+		{
+			name:        "version key absent in Helm values - no override",
+			initialTag:  "",
+			clusterTag:  "v4.23",
+			expectedTag: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hv := helmvalues.NewHelmValues()
+			if tt.initialTag != "" {
+				require.NoError(t, hv.SetStringValue(versionKey, tt.initialTag))
+			}
+
+			require.NoError(t, applyCatalogImageTagOverride(hv, tt.clusterTag))
+
+			got, _ := hv.GetStringValue(versionKey)
+			require.Equal(t, tt.expectedTag, got)
+		})
+	}
+}
+
 func TestSplitYAMLDocuments(t *testing.T) {
 	tests := []struct {
 		name     string

pkg/helmvalues/helmvalues.go

@@ -73,6 +73,15 @@ func (v *HelmValues) HasEnabledFeatureGates() (bool, error) {
 	return false, nil
 }
 
+func (v *HelmValues) GetStringValue(location string) (string, bool) {
+	ss := strings.Split(location, ".")
+	val, found, err := unstructured.NestedString(v.values, ss...)
+	if err != nil || !found {
+		return "", false
+	}
+	return val, true
+}
+
 func (v *HelmValues) SetStringValue(location string, newValue string) error {
 	if location == "" {
 		return errors.New("location string has no locations")

pkg/helmvalues/helmvalues_test.go

@@ -257,6 +257,69 @@ func TestHasEnabledFeatureGates(t *testing.T) {
 	}
 }
 
+func TestGetStringValue(t *testing.T) {
+	tests := []struct {
+		name      string
+		values    map[string]interface{}
+		location  string
+		wantVal   string
+		wantFound bool
+	}{
+		{
+			name:      "missing key",
+			values:    map[string]interface{}{},
+			location:  "options.openshift.catalogs.version",
+			wantFound: false,
+		},
+		{
+			name:      "simple key",
+			values:    map[string]interface{}{"key": "value"},
+			location:  "key",
+			wantVal:   "value",
+			wantFound: true,
+		},
+		{
+			name: "nested key",
+			values: map[string]interface{}{
+				"options": map[string]interface{}{
+					"openshift": map[string]interface{}{
+						"catalogs": map[string]interface{}{
+							"version": "v5.0",
+						},
+					},
+				},
+			},
+			location:  "options.openshift.catalogs.version",
+			wantVal:   "v5.0",
+			wantFound: true,
+		},
+		{
+			name: "non-string value returns not found",
+			values: map[string]interface{}{
+				"options": map[string]interface{}{
+					"replicas": int64(2),
+				},
+			},
+			location:  "options.replicas",
+			wantFound: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hv := NewHelmValues()
+			hv.values = tt.values
+			got, found := hv.GetStringValue(tt.location)
+			if got != tt.wantVal {
+				t.Errorf("GetStringValue() val = %q, want %q", got, tt.wantVal)
+			}
+			if found != tt.wantFound {
+				t.Errorf("GetStringValue() found = %v, want %v", found, tt.wantFound)
+			}
+		})
+	}
+}
+
 func TestSetStringValue(t *testing.T) {
 	tests := []struct {
 		name        string