[wip]OPRUN-4764: CSV: Extend RelatedImage with labels

crds/operators.coreos.com_clusterserviceversions.yaml

@@ -9048,6 +9048,24 @@ spec:
                     properties:
                       image:
                         type: string
+                      labels:
+                        description: |-
+                          Map of string keys and values that can be used to organize and categorize
+                          (scope and select) this related image, for instance, by the product features it belongs to.
+                          Keys and values follow the Kubernetes label syntax and constraints, see
+                          https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set.
+                          The semantics of the labels is defined by their consumer, which typically picks related images with
+                          Kubernetes label selectors. The first consumer is `oc-mirror`, see
+                          https://github.com/openshift/oc-mirror/blob/main/README.md for how it selects the related images to mirror.
+                          Labeling a related image makes it optional for mirroring: `oc-mirror` mirrors it only if the entry for its
+                          package in the ImageSetConfiguration carries a selector that matches the labels. If that package entry has
+                          no matching selector, the image is skipped. Related images without labels are always mirrored.
+                          Note that `operator-sdk generate bundle` may not recognize these labels and drop them from the generated
+                          CSV. It is the operator author's responsibility to make sure the labels end up in the bundle's CSV,
+                          restoring them manually if needed.
+                        type: object
+                        additionalProperties:
+                          type: string
                       name:
                         type: string
                 release:

pkg/operators/v1alpha1/clusterserviceversion_types.go

@@ -395,6 +395,21 @@ type Icon struct {
 type RelatedImage struct {
 	Name  string `json:"name"`
 	Image string `json:"image"`
+	// Map of string keys and values that can be used to organize and categorize
+	// (scope and select) this related image, for instance, by the product features it belongs to.
+	// Keys and values follow the Kubernetes label syntax and constraints, see
+	// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set.
+	// The semantics of the labels is defined by their consumer, which typically picks related images with
+	// Kubernetes label selectors. The first consumer is `oc-mirror`, see
+	// https://github.com/openshift/oc-mirror/blob/main/README.md for how it selects the related images to mirror.
+	// Labeling a related image makes it optional for mirroring: `oc-mirror` mirrors it only if the entry for its
+	// package in the ImageSetConfiguration carries a selector that matches the labels. If that package entry has
+	// no matching selector, the image is skipped. Related images without labels are always mirrored.
+	// Note that `operator-sdk generate bundle` may not recognize these labels and drop them from the generated
+	// CSV. It is the operator author's responsibility to make sure the labels end up in the bundle's CSV,
+	// restoring them manually if needed.
+	// +optional
+	Labels map[string]string `json:"labels,omitempty"`
 }
 
 // ClusterServiceVersionPhase is a label for the condition of a ClusterServiceVersion at the current time.

pkg/operators/v1alpha1/types_test.go

@@ -211,6 +211,66 @@ func TestCatalogSource_Poll(t *testing.T) {
 	}
 }
 
+func TestRelatedImageLabelsRoundTrip(t *testing.T) {
+	tests := []struct {
+		name string
+		in   []byte
+		out  RelatedImage
+	}{
+		{
+			name: "no labels",
+			in:   []byte(`{"name":"operator","image":"quay.io/example-com/foo-operator@sha256:abc"}`),
+			out: RelatedImage{
+				Name:  "operator",
+				Image: "quay.io/example-com/foo-operator@sha256:abc",
+			},
+		},
+		{
+			name: "empty labels",
+			in:   []byte(`{"name":"operator","image":"quay.io/example-com/foo-operator@sha256:abc","labels":{}}`),
+			out: RelatedImage{
+				Name:   "operator",
+				Image:  "quay.io/example-com/foo-operator@sha256:abc",
+				Labels: map[string]string{},
+			},
+		},
+		{
+			name: "several labels",
+			in:   []byte(`{"name":"operator","image":"quay.io/example-com/foo-operator@sha256:abc","labels":{"CoolFeatureA":"true","GreatFeatureB":"true"}}`),
+			out: RelatedImage{
+				Name:  "operator",
+				Image: "quay.io/example-com/foo-operator@sha256:abc",
+				Labels: map[string]string{
+					"CoolFeatureA":  "true",
+					"GreatFeatureB": "true",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ri := RelatedImage{}
+			require.NoError(t, json.Unmarshal(tt.in, &ri))
+			require.Equal(t, tt.out, ri)
+
+			marshalled, err := json.Marshal(ri)
+			require.NoError(t, err)
+			// labels is omitempty: an empty map is not serialized back.
+			if len(tt.out.Labels) > 0 {
+				require.JSONEq(t, string(tt.in), string(marshalled))
+			}
+
+			// DeepCopy must not alias the labels of the original.
+			cp := ri.DeepCopy()
+			require.Equal(t, ri, *cp)
+			if cp.Labels != nil {
+				cp.Labels["added-by-the-copy"] = "true"
+				require.Equal(t, tt.out.Labels, ri.Labels)
+			}
+		})
+	}
+}
+
 func TestUpdateStrategyUnmarshal(t *testing.T) {
 	type TestStruct struct {
 		UpdateStrategy UpdateStrategy `json:"updateStrategy,omitempty"`

pkg/validation/internal/bundle.go

@@ -2,6 +2,7 @@ package internal
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/operator-framework/api/pkg/manifests"
@@ -10,6 +11,7 @@ import (
 	interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
 	"go.podman.io/image/v5/docker/reference"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/validate/content"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
@@ -86,6 +88,38 @@ func validateRelatedImages(bundle *manifests.Bundle) []errors.Error {
 				fmt.Sprintf("relatedImages[%d] has an invalid image pullspec %q: %v", i, relatedImage.Image, err),
 				fmt.Sprintf("spec.relatedImages[%d].image", i)))
 		}
+
+		errs = append(errs, validateRelatedImageLabels(i, relatedImage.Labels)...)
+	}
+
+	return errs
+}
+
+// validateRelatedImageLabels checks that related image labels conform to the
+// Kubernetes label syntax.
+func validateRelatedImageLabels(relatedImageIndex int, labels map[string]string) []errors.Error {
+	var errs []errors.Error
+
+	keys := make([]string, 0, len(labels))
+	for key := range labels {
+		keys = append(keys, key)
+	}
+	// Sort keys to produce stable validation errors.
+	sort.Strings(keys)
+
+	for _, key := range keys {
+		path := fmt.Sprintf("spec.relatedImages[%d].labels[%q]", relatedImageIndex, key)
+		if messages := content.IsLabelKey(key); len(messages) > 0 {
+			errs = append(errs, errors.NewError(errors.ErrorInvalidBundle,
+				fmt.Sprintf("relatedImages[%d] has an invalid label key %q: %s", relatedImageIndex, key, strings.Join(messages, ", ")),
+				path, key))
+		}
+
+		if messages := content.IsLabelValue(labels[key]); len(messages) > 0 {
+			errs = append(errs, errors.NewError(errors.ErrorInvalidBundle,
+				fmt.Sprintf("relatedImages[%d] has an invalid label value %q for key %q: %s", relatedImageIndex, labels[key], key, strings.Join(messages, ", ")),
+				path, labels[key]))
+		}
 	}
 
 	return errs

pkg/validation/internal/bundle_test.go

@@ -354,6 +354,20 @@ func TestValidateRelatedImages(t *testing.T) {
 			},
 			wantError: false,
 		},
+		{
+			name: "valid Kubernetes labels should pass",
+			relatedImages: []v1alpha1.RelatedImage{
+				{
+					Name:  "operator",
+					Image: "quay.io/operator-framework/my-operator:v1.0.0",
+					Labels: map[string]string{
+						"app.kubernetes.io/name": "my-operator",
+						"feature":                "",
+					},
+				},
+			},
+			wantError: false,
+		},
 		{
 			name: "empty image field should error",
 			relatedImages: []v1alpha1.RelatedImage{
@@ -399,6 +413,24 @@ func TestValidateRelatedImages(t *testing.T) {
 			errCount:    1,
 			errContains: []string{"invalid image pullspec"},
 		},
+		{
+			name: "invalid label key should error",
+			relatedImages: []v1alpha1.RelatedImage{
+				{Name: "operator", Image: "quay.io/operator-framework/my-operator:v1.0.0", Labels: map[string]string{"invalid key": "value"}},
+			},
+			wantError:   true,
+			errCount:    1,
+			errContains: []string{"invalid label key", "invalid key"},
+		},
+		{
+			name: "invalid label value should error",
+			relatedImages: []v1alpha1.RelatedImage{
+				{Name: "operator", Image: "quay.io/operator-framework/my-operator:v1.0.0", Labels: map[string]string{"feature": "invalid value"}},
+			},
+			wantError:   true,
+			errCount:    1,
+			errContains: []string{"invalid label value", "invalid value"},
+		},
 		{
 			name: "multiple errors should all be reported",
 			relatedImages: []v1alpha1.RelatedImage{
@@ -452,3 +484,15 @@ func TestValidateRelatedImages(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateRelatedImageLabelsErrorFields(t *testing.T) {
+	labels := map[string]string{"invalid key": "invalid value"}
+	errs := validateRelatedImageLabels(0, labels)
+
+	require.Len(t, errs, 2)
+	for _, err := range errs {
+		require.Equal(t, `spec.relatedImages[0].labels["invalid key"]`, err.Field)
+	}
+	require.Equal(t, "invalid key", errs[0].BadValue)
+	require.Equal(t, "invalid value", errs[1].BadValue)
+}