Merge pull request #1410 from jhadvig/cvo-images-template-support

OTA-1956: pkg/payload: Add Images map and tolerate unknown template fields during upgrades

Author: openshift-merge-bot[bot]

pkg/payload/payload.go

@@ -153,7 +153,7 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, requiredFeatureSet
 		return nil, err
 	}
 
-	tasks := loadPayloadTasks(releaseDir, cvoDir, releaseImage, profile)
+	tasks := loadPayloadTasks(releaseDir, cvoDir, releaseImage, profile, payload.ImageRef)
 
 	var onlyKnownCaps *configv1.ClusterVersionCapabilitiesStatus
 
@@ -195,7 +195,13 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, requiredFeatureSet
 			if task.preprocess != nil {
 				raw, err = task.preprocess(raw)
 				if err != nil {
-					errs = append(errs, fmt.Errorf("preprocess %s: %w", file.Name(), err))
+					// Template rendering may fail when an older CVO binary
+					// loads a newer payload that uses template fields the
+					// older binary does not know about (e.g. .Images). Skip
+					// the manifest with a warning — the new CVO binary will
+					// re-load the full payload after it replaces the old one
+					// at run-level 0.
+					klog.Warningf("Skipping manifest %s: template rendering failed (may require newer CVO): %v", file.Name(), err)
 					continue
 				}
 			}
@@ -317,13 +323,14 @@ type payloadTasks struct {
 	skipFiles  sets.Set[string]
 }
 
-func loadPayloadTasks(releaseDir, cvoDir, releaseImage, clusterProfile string) []payloadTasks {
+func loadPayloadTasks(releaseDir, cvoDir, releaseImage, clusterProfile string, imageRef *imagev1.ImageStream) []payloadTasks {
 	cjf := filepath.Join(releaseDir, cincinnatiJSONFile)
 	irf := filepath.Join(releaseDir, imageReferencesFile)
 
 	mrc := manifestRenderConfig{
 		ReleaseImage:   releaseImage,
 		ClusterProfile: clusterProfile,
+		Images:         imagesFromImageRef(imageRef),
 	}
 
 	return []payloadTasks{{

pkg/payload/payload_test.go

@@ -150,6 +150,28 @@ func TestLoadUpdate(t *testing.T) {
 	}
 }
 
+func TestLoadUpdateSkipsUnknownTemplateFields(t *testing.T) {
+	update, err := LoadUpdate("testdata/payload-unknown-template", "image:test", "", "", DefaultClusterProfile, nil, sets.Set[string]{})
+	if err != nil {
+		t.Fatalf("LoadUpdate should not fail when a manifest uses unknown template fields, got: %v", err)
+	}
+
+	// The valid manifest (using known .ReleaseImage) should be loaded
+	var foundValid bool
+	for _, m := range update.Manifests {
+		if m.OriginalFilename == "0000_00_valid.yaml" {
+			foundValid = true
+		}
+		// The manifest with unknown .FutureField should be skipped (not loaded)
+		if m.OriginalFilename == "0000_50_unknown-field.yaml" {
+			t.Error("manifest with unknown template field should have been skipped, but was loaded")
+		}
+	}
+	if !foundValid {
+		t.Error("expected valid manifest (0000_00_valid.yaml) to be loaded")
+	}
+}
+
 func TestLoadUpdateArchitecture(t *testing.T) {
 	type args struct {
 		dir          string

pkg/payload/render.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/openshift/api/config"
 	configv1 "github.com/openshift/api/config/v1"
+	imagev1 "github.com/openshift/api/image/v1"
 	"github.com/openshift/library-go/pkg/manifest"
 )
 
@@ -38,6 +39,12 @@ func Render(outputDir, releaseImage, clusterVersionManifestPath, featureGateMani
 		}
 	)
 
+	imageRef, err := loadImageReferences(releaseManifestsDir)
+	if err != nil {
+		return fmt.Errorf("error loading image references for manifest rendering: %w", err)
+	}
+	renderConfig.Images = imagesFromImageRef(imageRef)
+
 	overrides, err := parseClusterVersionManifest(clusterVersionManifestPath)
 	if err != nil {
 		return fmt.Errorf("error parsing cluster version manifest: %w", err)
@@ -181,6 +188,21 @@ func renderDir(renderConfig manifestRenderConfig, idir, odir string, overrides [
 type manifestRenderConfig struct {
 	ReleaseImage   string
 	ClusterProfile string
+	Images         map[string]string
+}
+
+// imagesFromImageRef builds a map from image short names to their resolved URIs.
+func imagesFromImageRef(imageRef *imagev1.ImageStream) map[string]string {
+	images := make(map[string]string)
+	if imageRef == nil {
+		return images
+	}
+	for _, tag := range imageRef.Spec.Tags {
+		if tag.From != nil && tag.From.Kind == "DockerImage" {
+			images[tag.Name] = tag.From.Name
+		}
+	}
+	return images
 }
 
 // renderManifest Executes go text template from `manifestBytes` with `config`.

pkg/payload/render_test.go

@@ -11,14 +11,82 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/utils/ptr"
 
 	configv1 "github.com/openshift/api/config/v1"
+	imagev1 "github.com/openshift/api/image/v1"
 	"github.com/openshift/library-go/pkg/manifest"
 )
 
+func TestImagesFromImageRef(t *testing.T) {
+	tests := []struct {
+		name     string
+		imageRef *imagev1.ImageStream
+		expected map[string]string
+	}{
+		{
+			name:     "nil ImageStream returns empty map",
+			imageRef: nil,
+			expected: map[string]string{},
+		},
+		{
+			name: "maps DockerImage tags to their names",
+			imageRef: &imagev1.ImageStream{
+				Spec: imagev1.ImageStreamSpec{
+					Tags: []imagev1.TagReference{
+						{
+							Name: "console",
+							From: &corev1.ObjectReference{Kind: "DockerImage", Name: "quay.io/openshift/console:latest"},
+						},
+						{
+							Name: "cluster-update-console-plugin",
+							From: &corev1.ObjectReference{Kind: "DockerImage", Name: "quay.io/openshift/plugin:v1"},
+						},
+					},
+				},
+			},
+			expected: map[string]string{
+				"console":                       "quay.io/openshift/console:latest",
+				"cluster-update-console-plugin": "quay.io/openshift/plugin:v1",
+			},
+		},
+		{
+			name: "skips non-DockerImage tags",
+			imageRef: &imagev1.ImageStream{
+				Spec: imagev1.ImageStreamSpec{
+					Tags: []imagev1.TagReference{
+						{
+							Name: "docker-tag",
+							From: &corev1.ObjectReference{Kind: "DockerImage", Name: "example.com/img:v1"},
+						},
+						{
+							Name: "image-stream-tag",
+							From: &corev1.ObjectReference{Kind: "ImageStreamTag", Name: "other:latest"},
+						},
+						{
+							Name: "no-from",
+						},
+					},
+				},
+			},
+			expected: map[string]string{
+				"docker-tag": "example.com/img:v1",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual := imagesFromImageRef(tt.imageRef)
+			if diff := cmp.Diff(tt.expected, actual); diff != "" {
+				t.Errorf("imagesFromImageRef mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestRenderManifest(t *testing.T) {
 
 	tests := []struct {

pkg/payload/testdata/payload-unknown-template/manifests/0000_00_valid.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: valid-manifest
+  namespace: test
+  annotations:
+    include.release.openshift.io/self-managed-high-availability: "true"
+data:
+  release-image: '{{.ReleaseImage}}'

pkg/payload/testdata/payload-unknown-template/manifests/0000_50_unknown-field.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: future-manifest
+  namespace: test
+  annotations:
+    include.release.openshift.io/self-managed-high-availability: "true"
+data:
+  image: '{{index .FutureField "some-key"}}'

pkg/payload/testdata/payload-unknown-template/release-manifests/image-references

@@ -0,0 +1,7 @@
+{
+  "kind": "ImageStream",
+  "apiVersion": "image.openshift.io/v1",
+  "metadata": {
+    "name": "1.0.0-test"
+  }
+}

pkg/payload/testdata/payload-unknown-template/release-manifests/release-metadata

@@ -0,0 +1 @@
+{"kind":"cincinnati-metadata-v0","version":"1.0.0-test"}