Merge pull request #203 from tmshort/oprun-4599

OPRUN-4599: dynamically resolve catalog image tag from OCP release version

Author: openshift-merge-bot[bot]

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,37 @@ 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:    "5.0",
+			version: semver.Version{Major: 5, Minor: 0, Patch: 0},
+			want:    "v5.0",
+		},
+		{
+			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, "unexpected catalog image tag")
+		})
+	}
+}
+
 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

@@ -20,6 +20,11 @@ import (
 	"k8s.io/klog/v2"
 )
 
+// catalogVersionSentinel is a placeholder value that openshift.yaml sets for
+// options.openshift.catalogs.version to indicate the catalog tag should be
+// resolved dynamically from the running cluster's OCP major.minor version.
+const catalogVersionSentinel = "ocp-release"
+
 // Expected path structure:
 // ${assets}/helm/${subDir}/olmv1/ = chart
 // ${assets}/helm/${subDir}/openshift.yaml = primary values file
@@ -80,6 +85,11 @@ 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)
 	}
+	// When openshift.yaml sets options.openshift.catalogs.version to catalogVersionSentinel,
+	// replace it with the tag derived from the running cluster's OCP major.minor version.
+	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 +232,21 @@ type DocumentInfo struct {
 	Order    int
 }
 
+// applyCatalogImageTagOverride replaces options.openshift.catalogs.version in the
+// Helm values when it equals catalogVersionSentinel, substituting the tag derived
+// from the running cluster's OCP major.minor version. It is a no-op when clusterTag
+// is empty or when the current value is not catalogVersionSentinel.
+func applyCatalogImageTagOverride(values *helmvalues.HelmValues, clusterTag string) error {
+	if clusterTag == "" {
+		return nil
+	}
+	currentTag, found := values.GetStringValue("options.openshift.catalogs.version")
+	if !found || currentTag != catalogVersionSentinel {
+		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,56 @@ 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:  catalogVersionSentinel,
+			clusterTag:  "",
+			expectedTag: catalogVersionSentinel,
+		},
+		{
+			name:        "sentinel present, cluster is 4.23 - override to v4.23",
+			initialTag:  catalogVersionSentinel,
+			clusterTag:  "v4.23",
+			expectedTag: "v4.23",
+		},
+		{
+			name:        "Helm pins v4.23 - not the sentinel, no override",
+			initialTag:  "v4.23",
+			clusterTag:  "v4.23",
+			expectedTag: "v4.23",
+		},
+		{
+			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,17 @@ func (v *HelmValues) HasEnabledFeatureGates() (bool, error) {
 	return false, nil
 }
 
+// GetStringValue reads a dot-delimited string value from the Helm values map.
+// Returns the value and true if found; returns "", false if the key is absent or not a string.
+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