From 35aa11c2c1f1bb050d5fc96dfb6f07c2475eca41 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Wed, 16 Sep 2026 09:23:35 -0400 Subject: [PATCH 1/2] OPRUN-4727: Add labels to relatedImages in olm.bundle Add the optional `labels` field to declcfg.RelatedImage so operator authors can classify related images by product feature. The labels are carried through the model, the declcfg<->model conversions, and the gRPC serving path, so they survive `opm render`, `opm serve`, and a render/validate round-trip. `opm validate` checks the label keys and values against the Kubernetes label syntax. The check lives in Bundle.Validate rather than RelatedImage.Validate: the latter is still not called during bundle validation because production catalogs contain related images with an empty image reference, and enabling it wholesale would start rejecting them. The field is optional and additive: bundles without labels serialize and validate exactly as before, and an older opm ignores the field. For dev's purpose before operator-framework/api#524 (OPRUN-4764) gets in, github.com/operator-framework/api is replaced with that pull's branch. The replace directive should be dropped once the pull merges and a release carries RelatedImage.Labels. Co-Authored-By: Claude Opus 5 Signed-off-by: Hongkai Liu --- alpha/action/relatedimage_labels_test.go | 52 +++++++++ alpha/declcfg/declcfg.go | 7 ++ alpha/declcfg/declcfg_to_model.go | 5 +- alpha/declcfg/model_to_declcfg.go | 5 +- alpha/declcfg/relatedimages_test.go | 125 ++++++++++++++++++++++ alpha/model/model.go | 36 ++++++- alpha/model/relatedimage_labels_test.go | 129 +++++++++++++++++++++++ go.mod | 6 +- go.sum | 8 +- pkg/api/api_to_model.go | 5 +- pkg/api/model_to_api.go | 5 +- pkg/api/relatedimage_labels_test.go | 57 ++++++++++ 12 files changed, 425 insertions(+), 15 deletions(-) create mode 100644 alpha/action/relatedimage_labels_test.go create mode 100644 alpha/declcfg/relatedimages_test.go create mode 100644 alpha/model/relatedimage_labels_test.go create mode 100644 pkg/api/relatedimage_labels_test.go diff --git a/alpha/action/relatedimage_labels_test.go b/alpha/action/relatedimage_labels_test.go new file mode 100644 index 000000000..14533fff5 --- /dev/null +++ b/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 `: 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", ®istry.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) +} diff --git a/alpha/declcfg/declcfg.go b/alpha/declcfg/declcfg.go index 83c7dc7ab..3e0fd9fa6 100644 --- a/alpha/declcfg/declcfg.go +++ b/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 { diff --git a/alpha/declcfg/declcfg_to_model.go b/alpha/declcfg/declcfg_to_model.go index 730cbce10..4b5f7a321 100644 --- a/alpha/declcfg/declcfg_to_model.go +++ b/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 diff --git a/alpha/declcfg/model_to_declcfg.go b/alpha/declcfg/model_to_declcfg.go index a7732581e..36ee7f2f9 100644 --- a/alpha/declcfg/model_to_declcfg.go +++ b/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 diff --git a/alpha/declcfg/relatedimages_test.go b/alpha/declcfg/relatedimages_test.go new file mode 100644 index 000000000..36e0a5072 --- /dev/null +++ b/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") + } + }) + } + }) + } +} diff --git a/alpha/model/model.go b/alpha/model/model.go index 50e79d1ec..bd0907ed3 100644 --- a/alpha/model/model.go +++ b/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,8 +449,9 @@ 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 { @@ -448,9 +459,30 @@ func (i RelatedImage) Validate() error { 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 { diff --git a/alpha/model/relatedimage_labels_test.go b/alpha/model/relatedimage_labels_test.go new file mode 100644 index 000000000..893c799cf --- /dev/null +++ b/alpha/model/relatedimage_labels_test.go @@ -0,0 +1,129 @@ +package model + +import ( + "testing" + + "github.com/blang/semver/v4" + "github.com/stretchr/testify/require" + + "github.com/operator-framework/operator-registry/alpha/property" +) + +func TestRelatedImageValidateLabels(t *testing.T) { + type spec struct { + name string + labels map[string]string + assertion require.ErrorAssertionFunc + } + specs := []spec{ + { + name: "NoLabels", + labels: nil, + assertion: require.NoError, + }, + { + name: "EmptyLabels", + labels: map[string]string{}, + assertion: require.NoError, + }, + { + name: "ValidLabels", + labels: map[string]string{ + "feature": "duel", + "olm.operatorframework.io/optional": "true", + "empty-value": "", + }, + assertion: require.NoError, + }, + { + name: "InvalidLabelKey", + labels: map[string]string{"not a valid key": "duel"}, + assertion: hasErrorContaining( + `invalid related image`, + `invalid label key "not a valid key"`, + ), + }, + { + name: "InvalidLabelKeyPrefix", + labels: map[string]string{"not_a_domain/feature": "duel"}, + assertion: hasErrorContaining( + `invalid label key "not_a_domain/feature"`, + ), + }, + { + name: "InvalidLabelValue", + labels: map[string]string{"feature": "not a valid value"}, + assertion: hasErrorContaining( + `invalid label value "not a valid value" for key "feature"`, + ), + }, + } + + for _, s := range specs { + t.Run(s.name, func(t *testing.T) { + s.assertion(t, RelatedImage{Name: "foo", Image: "bar", Labels: s.labels}.Validate()) + }) + } +} + +// TestBundleValidateRelatedImageLabels covers the `opm validate` path: bundle +// validation, not RelatedImage.Validate, is what runs against a loaded FBC. +func TestBundleValidateRelatedImageLabels(t *testing.T) { + newBundle := func(relatedImages []RelatedImage) *Bundle { + pkg, ch := makePackageChannelBundle() + return &Bundle{ + Package: pkg, + Channel: ch, + Name: "anakin.v0.0.1", + Image: "anakin-operator:v0.0.1", + Version: semver.MustParse("0.0.1"), + Properties: []property.Property{ + property.MustBuildPackage("anakin", "0.0.1"), + }, + RelatedImages: relatedImages, + } + } + + t.Run("NoRelatedImages", func(t *testing.T) { + require.NoError(t, newBundle(nil).Validate()) + }) + + t.Run("RelatedImagesWithoutLabels", func(t *testing.T) { + require.NoError(t, newBundle([]RelatedImage{{Name: "foo", Image: "bar"}}).Validate()) + }) + + t.Run("RelatedImagesWithValidLabels", func(t *testing.T) { + require.NoError(t, newBundle([]RelatedImage{ + {Name: "foo", Image: "bar", Labels: map[string]string{"feature": "duel"}}, + }).Validate()) + }) + + t.Run("RelatedImageWithoutImageStillPasses", func(t *testing.T) { + // Related images with an empty image reference exist in production + // catalogs; adding label validation must not start rejecting them. + require.NoError(t, newBundle([]RelatedImage{{Name: "foo"}}).Validate()) + }) + + t.Run("RelatedImageWithInvalidLabels", func(t *testing.T) { + err := newBundle([]RelatedImage{ + {Name: "foo", Image: "bar"}, + {Name: "baz", Image: "quux", Labels: map[string]string{"bad key": "bad value"}}, + }).Validate() + require.Error(t, err) + require.ErrorContains(t, err, `relatedImages[1]`) + require.ErrorContains(t, err, `invalid label key "bad key"`) + require.ErrorContains(t, err, `invalid label value "bad value" for key "bad key"`) + }) +} + +func hasErrorContaining(substrings ...string) require.ErrorAssertionFunc { + return func(t require.TestingT, actualError error, _ ...interface{}) { + if stdt, ok := t.(*testing.T); ok { + stdt.Helper() + } + require.Error(t, actualError) + for _, s := range substrings { + require.ErrorContains(t, actualError, s) + } + } +} diff --git a/go.mod b/go.mod index 7ca7ba804..79caf6c8d 100644 --- a/go.mod +++ b/go.mod @@ -115,7 +115,7 @@ require ( github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect - github.com/google/cel-go v0.29.2 // indirect + github.com/google/cel-go v0.31.0 // indirect github.com/google/gnostic-models v0.7.1 // indirect github.com/google/go-containerregistry v0.21.6 // indirect github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 // indirect @@ -218,3 +218,7 @@ require ( sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.3 // indirect ) + +// TODO: drop this replace once https://github.com/operator-framework/api/pull/524 (OPRUN-4764) merges +// and a release of github.com/operator-framework/api carries RelatedImage.Labels. +replace github.com/operator-framework/api => github.com/hongkailiu/operator-framework-api v0.0.0-20260915192809-dc0d2ebcaa20 diff --git a/go.sum b/go.sum index 5516de1c7..5e4e12dba 100644 --- a/go.sum +++ b/go.sum @@ -203,8 +203,8 @@ github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QD github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= -github.com/google/cel-go v0.29.2 h1:ZtDxkeiMmz0mxbKDYiNkE5Lk7V5edMRcaaDf2jX002k= -github.com/google/cel-go v0.29.2/go.mod h1:X0bD6iVNR8pkROSOoHVdgTkzmRcosof7WQqCD6wcMc8= +github.com/google/cel-go v0.31.0 h1:H0bhpFTqOvmHrBGrWKp7ZlhBm5Hh8PYUEXnwxT1LL7A= +github.com/google/cel-go v0.31.0/go.mod h1:X0bD6iVNR8pkROSOoHVdgTkzmRcosof7WQqCD6wcMc8= github.com/google/gnostic-models v0.7.1 h1:SisTfuFKJSKM5CPZkffwi6coztzzeYUhc3v4yxLWH8c= github.com/google/gnostic-models v0.7.1/go.mod h1:whL5G0m6dmc5cPxKc5bdKdEN3UjI7OUGxBlw57miDrQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= @@ -247,6 +247,8 @@ github.com/hashicorp/golang-lru/arc/v2 v2.0.7 h1:QxkVTxwColcduO+LP7eJO56r2hFiG8z github.com/hashicorp/golang-lru/arc/v2 v2.0.7/go.mod h1:Pe7gBlGdc8clY5LJ0LpJXMt5AmgmWNH1g+oFFVUHOEc= github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= +github.com/hongkailiu/operator-framework-api v0.0.0-20260915192809-dc0d2ebcaa20 h1:DO+OSbdxfRvKfsqXb0taYbg9rbaf+JNEj5AkmmZDH2c= +github.com/hongkailiu/operator-framework-api v0.0.0-20260915192809-dc0d2ebcaa20/go.mod h1:nM8ApyShGLM1uN3tZ5fDkOoDEQm6tmnAdacrcuQF+oE= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= @@ -324,8 +326,6 @@ github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJw github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M= github.com/opencontainers/runtime-spec v1.3.0 h1:YZupQUdctfhpZy3TM39nN9Ika5CBWT5diQ8ibYCRkxg= github.com/opencontainers/runtime-spec v1.3.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= -github.com/operator-framework/api v0.45.0 h1:hkROwtsLH3oszp4IW+WsXEFSDgveSahHI7DKStOtrUI= -github.com/operator-framework/api v0.45.0/go.mod h1:IQ4uuISTiIhV09oAurJSGD4KabayhY5nV6k1XmA235M= github.com/otiai10/copy v1.14.1 h1:5/7E6qsUMBaH5AnQ0sSLzzTg1oTECmcCmT6lvF45Na8= github.com/otiai10/copy v1.14.1/go.mod h1:oQwrEDDOci3IM8dJF0d8+jnbfPDllW6vUjNc3DoZm9I= github.com/otiai10/mint v1.6.3 h1:87qsV/aw1F5as1eH1zS/yqHY85ANKVMgkDrf9rcxbQs= diff --git a/pkg/api/api_to_model.go b/pkg/api/api_to_model.go index 50088ab4f..ff6b90dd8 100644 --- a/pkg/api/api_to_model.go +++ b/pkg/api/api_to_model.go @@ -136,8 +136,9 @@ func getRelatedImages(csvJSON string) ([]model.RelatedImage, error) { type csv struct { Spec struct { RelatedImages []struct { - Name string `json:"name"` - Image string `json:"image"` + Name string `json:"name"` + Image string `json:"image"` + Labels map[string]string `json:"labels"` } `json:"relatedImages"` } `json:"spec"` } diff --git a/pkg/api/model_to_api.go b/pkg/api/model_to_api.go index b3368383f..0f5df803a 100644 --- a/pkg/api/model_to_api.go +++ b/pkg/api/model_to_api.go @@ -204,8 +204,9 @@ func convertModelRelatedImagesToCSVRelatedImages(in []model.RelatedImage) []v1al var out []v1alpha1.RelatedImage for _, ri := range in { out = append(out, v1alpha1.RelatedImage{ - Name: ri.Name, - Image: ri.Image, + Name: ri.Name, + Image: ri.Image, + Labels: ri.Labels, }) } return out diff --git a/pkg/api/relatedimage_labels_test.go b/pkg/api/relatedimage_labels_test.go new file mode 100644 index 000000000..ab55f69d4 --- /dev/null +++ b/pkg/api/relatedimage_labels_test.go @@ -0,0 +1,57 @@ +package api + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-registry/alpha/model" +) + +func TestGetRelatedImagesLabels(t *testing.T) { + type spec struct { + name string + csvJSON string + expected []model.RelatedImage + } + specs := []spec{ + { + name: "WithLabels", + csvJSON: `{"spec":{"relatedImages":[{"name":"lightsaber","image":"quay.io/anakin/lightsaber:v0.1.0","labels":{"feature":"duel"}}]}}`, + expected: []model.RelatedImage{{ + Name: "lightsaber", + Image: "quay.io/anakin/lightsaber:v0.1.0", + Labels: map[string]string{"feature": "duel"}, + }}, + }, + { + name: "WithoutLabels", + csvJSON: `{"spec":{"relatedImages":[{"name":"lightsaber","image":"quay.io/anakin/lightsaber:v0.1.0"}]}}`, + expected: []model.RelatedImage{{ + Name: "lightsaber", + Image: "quay.io/anakin/lightsaber:v0.1.0", + }}, + }, + } + + for _, s := range specs { + t.Run(s.name, func(t *testing.T) { + actual, err := getRelatedImages(s.csvJSON) + require.NoError(t, err) + require.Equal(t, s.expected, actual) + }) + } +} + +func TestConvertModelRelatedImagesToCSVRelatedImagesLabels(t *testing.T) { + actual := convertModelRelatedImagesToCSVRelatedImages([]model.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"}, + }) + require.Equal(t, []v1alpha1.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"}, + }, actual) +} From 6c58e943e17446a7099e3f117d680f4adce9f805 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Fri, 18 Sep 2026 10:27:28 -0400 Subject: [PATCH 2/2] OPRUN-4727: Add e2e test for relatedImages labels Mechanize the manual check from the pull request: render a bundle directory whose ClusterServiceVersion labels its relatedImages, and assert the labels survive into the olm.bundle blob. The spec execs the opm built by `make build` rather than driving the suite's in-process cobra command, because `opm render` writes to os.Stdout directly and calls log.Fatal on error, so opm.SetOut() would neither capture the output nor survive a failure. The fixture is a minimal registry+v1 bundle with one labeled and one unlabeled related image, so the test also covers that an absent labels field stays absent. Co-Authored-By: Claude Opus 5 Signed-off-by: Hongkai Liu --- test/e2e/opm_render_test.go | 61 +++++++++++++++++ ...e-labels.v0.1.0.clusterserviceversion.yaml | 66 +++++++++++++++++++ .../metadata/annotations.yaml | 7 ++ 3 files changed, 134 insertions(+) create mode 100644 test/e2e/opm_render_test.go create mode 100644 test/e2e/testdata/bundles/related-image-labels.0.1.0/manifests/related-image-labels.v0.1.0.clusterserviceversion.yaml create mode 100644 test/e2e/testdata/bundles/related-image-labels.0.1.0/metadata/annotations.yaml diff --git a/test/e2e/opm_render_test.go b/test/e2e/opm_render_test.go new file mode 100644 index 000000000..2111d32fd --- /dev/null +++ b/test/e2e/opm_render_test.go @@ -0,0 +1,61 @@ +package e2e_test + +import ( + "bytes" + "os/exec" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/operator-framework/operator-registry/alpha/declcfg" +) + +var _ = Describe("opm render", func() { + // opmBin is the opm built by `make build`, exercised here as a subprocess so + // that the test reads the same stdout an operator author would. The render + // subcommand writes to os.Stdout directly, so opm.SetOut() would not capture it. + var opmBin string + + BeforeEach(func() { + var err error + opmBin, err = filepath.Abs(filepath.Join("..", "..", "bin", "opm")) + Expect(err).NotTo(HaveOccurred()) + Expect(opmBin).To(BeAnExistingFile(), "opm binary not found; run `make build` before `make e2e`") + }) + + Context("for a bundle directory whose CSV labels its relatedImages", func() { + It("carries the labels into the olm.bundle blob", func() { + By("rendering the bundle directory") + var stdout, stderr bytes.Buffer + cmd := exec.Command(opmBin, "render", "testdata/bundles/related-image-labels.0.1.0") + cmd.Stdout = &stdout + cmd.Stderr = &stderr + Expect(cmd.Run()).To(Succeed(), "opm render failed: %s", stderr.String()) + + By("loading the rendered file-based catalog") + cfg, err := declcfg.LoadReader(bytes.NewReader(stdout.Bytes())) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.Bundles).To(HaveLen(1)) + + By("checking the relatedImages labels round-tripped") + // opm render sorts relatedImages by image reference, and appends the + // CSV's containerImage as an unnamed entry. + Expect(cfg.Bundles[0].RelatedImages).To(Equal([]declcfg.RelatedImage{ + { + Name: "labeled", + Image: "quay.io/olmtest/labeled@sha256:c68135620167c41e3d9f6c1d2ca1eb8fa24312b86186d09b8010656b9d25fb47", + Labels: map[string]string{"feature": "example", "tier": "1"}, + }, + { + Name: "", + Image: "quay.io/olmtest/related-image-labels:v0.1.0", + }, + { + Name: "unlabeled", + Image: "quay.io/olmtest/unlabeled@sha256:49ed7d6155342adaa2b12fd80c6761c3081d8e6149d187cb7ff91a247cdf2e7a", + }, + })) + }) + }) +}) diff --git a/test/e2e/testdata/bundles/related-image-labels.0.1.0/manifests/related-image-labels.v0.1.0.clusterserviceversion.yaml b/test/e2e/testdata/bundles/related-image-labels.0.1.0/manifests/related-image-labels.v0.1.0.clusterserviceversion.yaml new file mode 100644 index 000000000..b06de3dc0 --- /dev/null +++ b/test/e2e/testdata/bundles/related-image-labels.0.1.0/manifests/related-image-labels.v0.1.0.clusterserviceversion.yaml @@ -0,0 +1,66 @@ +apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +metadata: + name: related-image-labels.v0.1.0 + namespace: placeholder + annotations: + containerImage: 'quay.io/olmtest/related-image-labels:v0.1.0' +spec: + displayName: Related Image Labels Operator + description: >- + A minimal bundle whose ClusterServiceVersion declares relatedImages both + with and without labels. Used to check that `opm render` carries the labels + into the olm.bundle blob. + version: 0.1.0 + maturity: alpha + provider: + name: Red Hat + apiservicedefinitions: {} + customresourcedefinitions: {} + relatedImages: + - name: labeled + image: quay.io/olmtest/labeled@sha256:c68135620167c41e3d9f6c1d2ca1eb8fa24312b86186d09b8010656b9d25fb47 + labels: + feature: example + tier: "1" + - name: unlabeled + image: quay.io/olmtest/unlabeled@sha256:49ed7d6155342adaa2b12fd80c6761c3081d8e6149d187cb7ff91a247cdf2e7a + installModes: + - type: OwnNamespace + supported: true + - type: SingleNamespace + supported: true + - type: MultiNamespace + supported: false + - type: AllNamespaces + supported: true + install: + strategy: deployment + spec: + permissions: + - serviceAccountName: related-image-labels + rules: + - apiGroups: + - "" + resources: + - configmaps + verbs: + - '*' + deployments: + - name: related-image-labels + spec: + replicas: 1 + selector: + matchLabels: + name: related-image-labels + template: + metadata: + labels: + name: related-image-labels + spec: + serviceAccountName: related-image-labels + containers: + - name: manager + image: quay.io/olmtest/related-image-labels:v0.1.0 + command: + - related-image-labels diff --git a/test/e2e/testdata/bundles/related-image-labels.0.1.0/metadata/annotations.yaml b/test/e2e/testdata/bundles/related-image-labels.0.1.0/metadata/annotations.yaml new file mode 100644 index 000000000..a78f548ee --- /dev/null +++ b/test/e2e/testdata/bundles/related-image-labels.0.1.0/metadata/annotations.yaml @@ -0,0 +1,7 @@ +annotations: + operators.operatorframework.io.bundle.mediatype.v1: "registry+v1" + operators.operatorframework.io.bundle.manifests.v1: "/manifests/" + operators.operatorframework.io.bundle.metadata.v1: "/metadata/" + operators.operatorframework.io.bundle.package.v1: "related-image-labels" + operators.operatorframework.io.bundle.channels.v1: "stable" + operators.operatorframework.io.bundle.channel.default.v1: "stable"