[wip]OPRUN-4727: Add labels to relatedImages in olm.bundle

alpha/action/relatedimage_labels_test.go

@@ -0,0 +1,52 @@
+package action
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
+	"github.com/operator-framework/operator-registry/pkg/registry"
+)
+
+// TestGetRelatedImagesLabels covers `opm render <bundle image>`: labels the
+// operator author put on the CSV's relatedImages must survive into the
+// olm.bundle blob.
+func TestGetRelatedImagesLabels(t *testing.T) {
+	csvSpec := `{
+		"relatedImages": [
+			{
+				"name": "lightsaber",
+				"image": "quay.io/anakin/lightsaber:v0.1.0",
+				"labels": {"feature": "duel"}
+			},
+			{
+				"name": "podracer",
+				"image": "quay.io/anakin/podracer:v0.1.0"
+			}
+		]
+	}`
+
+	var spec map[string]interface{}
+	require.NoError(t, json.Unmarshal([]byte(csvSpec), &spec))
+
+	csv := &unstructured.Unstructured{Object: map[string]interface{}{
+		"apiVersion": "operators.coreos.com/v1alpha1",
+		"kind":       "ClusterServiceVersion",
+		"metadata":   map[string]interface{}{"name": "anakin.v0.1.0"},
+		"spec":       spec,
+	}}
+
+	b := registry.NewBundle("anakin.v0.1.0", &registry.Annotations{PackageName: "anakin"}, csv)
+	b.BundleImage = "quay.io/anakin/bundle:v0.1.0"
+
+	relatedImages, err := getRelatedImages(b)
+	require.NoError(t, err)
+	require.Equal(t, []declcfg.RelatedImage{
+		{Name: "lightsaber", Image: "quay.io/anakin/lightsaber:v0.1.0", Labels: map[string]string{"feature": "duel"}},
+		{Name: "podracer", Image: "quay.io/anakin/podracer:v0.1.0"},
+		{Image: "quay.io/anakin/bundle:v0.1.0"},
+	}, relatedImages)
+}

alpha/declcfg/declcfg.go

@@ -101,6 +101,13 @@ type Bundle struct {
 type RelatedImage struct {
 	Name  string `json:"name"`
 	Image string `json:"image"`
+
+	// Labels classify this related image, for instance, by the product
+	// features it belongs to. Keys and values follow the Kubernetes label
+	// syntax and constraints. The semantics of the labels is defined by
+	// their consumer, which typically picks related images with Kubernetes
+	// label selectors.
+	Labels map[string]string `json:"labels,omitempty"`
 }
 
 type Deprecation struct {

alpha/declcfg/declcfg_to_model.go

@@ -270,8 +270,9 @@ func relatedImagesToModelRelatedImages(in []RelatedImage) []model.RelatedImage {
 	var out []model.RelatedImage
 	for _, p := range in {
 		out = append(out, model.RelatedImage{
-			Name:  p.Name,
-			Image: p.Image,
+			Name:   p.Name,
+			Image:  p.Image,
+			Labels: p.Labels,
 		})
 	}
 	return out

alpha/declcfg/model_to_declcfg.go

@@ -125,8 +125,9 @@ func ModelRelatedImagesToRelatedImages(relatedImages []model.RelatedImage) []Rel
 	var out []RelatedImage
 	for _, ri := range relatedImages {
 		out = append(out, RelatedImage{
-			Name:  ri.Name,
-			Image: ri.Image,
+			Name:   ri.Name,
+			Image:  ri.Image,
+			Labels: ri.Labels,
 		})
 	}
 	return out

alpha/declcfg/relatedimages_test.go

@@ -0,0 +1,125 @@
+package declcfg
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/operator-framework/operator-registry/alpha/model"
+)
+
+func TestRelatedImageLabelsModelRoundTrip(t *testing.T) {
+	labels := map[string]string{"feature": "duel"}
+
+	b := newTestBundle("foo", "0.1.0")
+	b.RelatedImages = append(b.RelatedImages, RelatedImage{
+		Name:   "lightsaber",
+		Image:  "quay.io/anakin/lightsaber:v0.1.0",
+		Labels: labels,
+	})
+	cfg := DeclarativeConfig{
+		Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
+		Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: "foo.v0.1.0"})},
+		Bundles:  []Bundle{b},
+	}
+
+	m, err := ConvertToModel(cfg)
+	require.NoError(t, err)
+	require.Equal(t, []model.RelatedImage{
+		{Name: "bundle", Image: "foo-bundle:v0.1.0"},
+		{Name: "lightsaber", Image: "quay.io/anakin/lightsaber:v0.1.0", Labels: labels},
+	}, m["foo"].Channels["alpha"].Bundles["foo.v0.1.0"].RelatedImages)
+
+	actual := ConvertFromModel(m)
+	require.Len(t, actual.Bundles, 1)
+	require.Equal(t, b.RelatedImages, actual.Bundles[0].RelatedImages)
+}
+
+func TestRelatedImageLabelsRoundTrip(t *testing.T) {
+	type spec struct {
+		name     string
+		in       string
+		expected []RelatedImage
+	}
+	specs := []spec{
+		{
+			name: "WithLabels",
+			in: `{
+    "schema": "olm.bundle",
+    "name": "anakin.v0.0.1",
+    "package": "anakin",
+    "image": "quay.io/anakin/bundle:v0.0.1",
+    "relatedImages": [
+        {
+            "name": "lightsaber",
+            "image": "quay.io/anakin/lightsaber:v0.0.1",
+            "labels": {
+                "feature": "duel",
+                "olm.operatorframework.io/optional": "true"
+            }
+        }
+    ]
+}`,
+			expected: []RelatedImage{{
+				Name:  "lightsaber",
+				Image: "quay.io/anakin/lightsaber:v0.0.1",
+				Labels: map[string]string{
+					"feature":                           "duel",
+					"olm.operatorframework.io/optional": "true",
+				},
+			}},
+		},
+		{
+			name: "WithoutLabels",
+			in: `{
+    "schema": "olm.bundle",
+    "name": "anakin.v0.0.1",
+    "package": "anakin",
+    "image": "quay.io/anakin/bundle:v0.0.1",
+    "relatedImages": [
+        {
+            "name": "lightsaber",
+            "image": "quay.io/anakin/lightsaber:v0.0.1"
+        }
+    ]
+}`,
+			expected: []RelatedImage{{
+				Name:  "lightsaber",
+				Image: "quay.io/anakin/lightsaber:v0.0.1",
+			}},
+		},
+	}
+
+	for _, s := range specs {
+		t.Run(s.name, func(t *testing.T) {
+			cfg, err := LoadReader(strings.NewReader(s.in))
+			require.NoError(t, err)
+			require.Len(t, cfg.Bundles, 1)
+			require.Equal(t, s.expected, cfg.Bundles[0].RelatedImages)
+
+			for _, tc := range []struct {
+				format string
+				write  func(DeclarativeConfig, *bytes.Buffer) error
+			}{
+				{"json", func(c DeclarativeConfig, buf *bytes.Buffer) error { return WriteJSON(c, buf) }},
+				{"yaml", func(c DeclarativeConfig, buf *bytes.Buffer) error { return WriteYAML(c, buf) }},
+			} {
+				t.Run(tc.format, func(t *testing.T) {
+					buf := &bytes.Buffer{}
+					require.NoError(t, tc.write(*cfg, buf))
+
+					roundTripped, err := LoadReader(bytes.NewReader(buf.Bytes()))
+					require.NoError(t, err)
+					require.Len(t, roundTripped.Bundles, 1)
+					require.Equal(t, s.expected, roundTripped.Bundles[0].RelatedImages)
+
+					if s.expected[0].Labels == nil {
+						require.NotContains(t, buf.String(), "labels")
+					}
+				})
+			}
+		})
+	}
+}

alpha/model/model.go

@@ -13,6 +13,7 @@ import (
 	"github.com/h2non/filetype/types"
 	svg "github.com/h2non/go-is-svg"
 	"golang.org/x/exp/maps"
+	"k8s.io/apimachinery/pkg/api/validate/content"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	"github.com/operator-framework/operator-registry/alpha/property"
@@ -418,6 +419,15 @@ func (b *Bundle) Validate() error {
 	//		result.subErrors = append(result.subErrors, WithIndex(i, err))
 	//	}
 	//}
+	// The labels are a newer, opt-in field, so they can be validated without
+	// tripping over the legacy data described above.
+	for i, relatedImage := range b.RelatedImages {
+		if errs := relatedImage.validateLabels(); len(errs) > 0 {
+			riResult := newValidationError(fmt.Sprintf("invalid relatedImages[%d]", i))
+			riResult.subErrors = errs
+			result.subErrors = append(result.subErrors, riResult)
+		}
+	}
 
 	if props != nil && len(props.Packages) != 1 {
 		result.subErrors = append(result.subErrors, fmt.Errorf("must be exactly one property with type %q", property.TypePackage))
@@ -439,18 +449,40 @@ func (b *Bundle) Validate() error {
 }
 
 type RelatedImage struct {
-	Name  string
-	Image string
+	Name   string
+	Image  string
+	Labels map[string]string
 }
 
 func (i RelatedImage) Validate() error {
 	result := newValidationError("invalid related image")
 	if i.Image == "" {
 		result.subErrors = append(result.subErrors, fmt.Errorf("image must be set"))
 	}
+	result.subErrors = append(result.subErrors, i.validateLabels()...)
 	return result.orNil()
 }
 
+// validateLabels checks the related image labels against the Kubernetes label
+// syntax and constraints. It is separate from Validate so that bundle
+// validation can check the labels without also checking the image reference,
+// which some catalogs in production leave empty.
+func (i RelatedImage) validateLabels() []error {
+	// nolint:prealloc
+	var errs []error
+	keys := maps.Keys(i.Labels)
+	sort.Strings(keys)
+	for _, k := range keys {
+		for _, msg := range content.IsLabelKey(k) {
+			errs = append(errs, fmt.Errorf("invalid label key %q: %s", k, msg))
+		}
+		for _, msg := range content.IsLabelValue(i.Labels[k]) {
+			errs = append(errs, fmt.Errorf("invalid label value %q for key %q: %s", i.Labels[k], k, msg))
+		}
+	}
+	return errs
+}
+
 func (m Model) Normalize() {
 	for _, pkg := range m {
 		for _, ch := range pkg.Channels {