model: validate unique bundle versions (#1417)

Author: joelanford

alpha/declcfg/declcfg_to_model_test.go

@@ -4,9 +4,11 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/blang/semver/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/operator-framework/operator-registry/alpha/model"
 	"github.com/operator-framework/operator-registry/alpha/property"
 )
 
@@ -450,6 +452,41 @@ func TestConvertToModel(t *testing.T) {
 	}
 }
 
+func TestConvertToModelBundle(t *testing.T) {
+	cfg := DeclarativeConfig{
+		Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
+		Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: "foo.v0.1.0"})},
+		Bundles:  []Bundle{newTestBundle("foo", "0.1.0")},
+	}
+	m, err := ConvertToModel(cfg)
+	require.NoError(t, err)
+
+	pkg, ok := m["foo"]
+	require.True(t, ok, "expected package 'foo' to be present")
+	ch, ok := pkg.Channels["alpha"]
+	require.True(t, ok, "expected channel 'alpha' to be present")
+	b, ok := ch.Bundles["foo.v0.1.0"]
+	require.True(t, ok, "expected bundle 'foo.v0.1.0' to be present")
+
+	assert.Equal(t, pkg, b.Package)
+	assert.Equal(t, ch, b.Channel)
+	assert.Equal(t, "foo.v0.1.0", b.Name)
+	assert.Equal(t, "foo-bundle:v0.1.0", b.Image)
+	assert.Equal(t, "", b.Replaces)
+	assert.Nil(t, b.Skips)
+	assert.Equal(t, "", b.SkipRange)
+	assert.Len(t, b.Properties, 3)
+	assert.Equal(t, []model.RelatedImage{{Name: "bundle", Image: "foo-bundle:v0.1.0"}}, b.RelatedImages)
+	assert.Nil(t, b.Deprecation)
+	assert.Len(t, b.Objects, 2)
+	assert.NotEmpty(t, b.CsvJSON)
+	assert.NotNil(t, b.PropertiesP)
+	assert.Len(t, b.PropertiesP.BundleObjects, 2)
+	assert.Len(t, b.PropertiesP.Packages, 1)
+	assert.Equal(t, semver.MustParse("0.1.0"), b.Version)
+
+}
+
 func TestConvertToModelRoundtrip(t *testing.T) {
 	expected := buildValidDeclarativeConfig(validDeclarativeConfigSpec{IncludeUnrecognized: true, IncludeDeprecations: false}) // TODO: turn on deprecation when we have model-->declcfg conversion
 

alpha/declcfg/helpers_test.go

@@ -6,6 +6,7 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/blang/semver/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -244,6 +245,7 @@ func getBundle(pkg *model.Package, ch *model.Channel, version, replaces string,
 			getCSVJson(pkg.Name, version),
 			getCRDJSON(),
 		},
+		Version: semver.MustParse(version),
 	}
 }
 

alpha/model/model.go

@@ -11,6 +11,7 @@ import (
 	"github.com/h2non/filetype/matchers"
 	"github.com/h2non/filetype/types"
 	svg "github.com/h2non/go-is-svg"
+	"golang.org/x/exp/maps"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	"github.com/operator-framework/operator-registry/alpha/property"
@@ -86,6 +87,10 @@ func (m *Package) Validate() error {
 		}
 	}
 
+	if err := m.validateUniqueBundleVersions(); err != nil {
+		result.subErrors = append(result.subErrors, err)
+	}
+
 	if m.DefaultChannel != nil && !foundDefault {
 		result.subErrors = append(result.subErrors, fmt.Errorf("default channel %q not found in channels list", m.DefaultChannel.Name))
 	}
@@ -97,6 +102,36 @@ func (m *Package) Validate() error {
 	return result.orNil()
 }
 
+func (m *Package) validateUniqueBundleVersions() error {
+	versionsMap := map[string]semver.Version{}
+	bundlesWithVersion := map[string]sets.Set[string]{}
+	for _, ch := range m.Channels {
+		for _, b := range ch.Bundles {
+			versionsMap[b.Version.String()] = b.Version
+			if bundlesWithVersion[b.Version.String()] == nil {
+				bundlesWithVersion[b.Version.String()] = sets.New[string]()
+			}
+			bundlesWithVersion[b.Version.String()].Insert(b.Name)
+		}
+	}
+
+	versionsSlice := maps.Values(versionsMap)
+	semver.Sort(versionsSlice)
+
+	var errs []error
+	for _, v := range versionsSlice {
+		bundles := sets.List(bundlesWithVersion[v.String()])
+		if len(bundles) > 1 {
+			errs = append(errs, fmt.Errorf("{%s: [%s]}", v, strings.Join(bundles, ", ")))
+		}
+	}
+
+	if len(errs) > 0 {
+		return fmt.Errorf("duplicate versions found in bundles: %v", errs)
+	}
+	return nil
+}
+
 type Icon struct {
 	Data      []byte `json:"base64data"`
 	MediaType string `json:"mediatype"`

alpha/model/model_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/blang/semver/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -258,6 +259,25 @@ func TestValidators(t *testing.T) {
 			},
 			assertion: hasError("package must contain at least one channel"),
 		},
+		{
+			name: "Package/Error/DuplicateBundleVersions",
+			v: &Package{
+				Name: "anakin",
+				Channels: map[string]*Channel{
+					"light": {
+						Package: pkg,
+						Name:    "light",
+						Bundles: map[string]*Bundle{
+							"anakin.v0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")},
+							"anakin.v0.0.2": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.1")},
+							"anakin.v1.0.1": {Name: "anakin.v1.0.1", Version: semver.MustParse("1.0.1")},
+							"anakin.v1.0.2": {Name: "anakin.v1.0.2", Version: semver.MustParse("1.0.1")},
+						},
+					},
+				},
+			},
+			assertion: hasError(`duplicate versions found in bundles: [{0.0.1: [anakin.v0.0.1, anakin.v0.0.2]} {1.0.1: [anakin.v1.0.1, anakin.v1.0.2]}]`),
+		},
 		{
 			name: "Package/Error/NoDefaultChannel",
 			v: &Package{
@@ -612,6 +632,7 @@ func makePackageChannelBundle() (*Package, *Channel) {
 			property.MustBuildPackage("anakin", "0.0.1"),
 			property.MustBuildGVK("skywalker.me", "v1alpha1", "PodRacer"),
 		},
+		Version: semver.MustParse("0.0.1"),
 	}
 	bundle2 := &Bundle{
 		Name:     "anakin.v0.0.2",
@@ -621,6 +642,7 @@ func makePackageChannelBundle() (*Package, *Channel) {
 			property.MustBuildPackage("anakin", "0.0.2"),
 			property.MustBuildGVK("skywalker.me", "v1alpha1", "PodRacer"),
 		},
+		Version: semver.MustParse("0.0.2"),
 	}
 	ch := &Channel{
 		Name: "light",

go.mod

@@ -36,6 +36,7 @@ require (
 	github.com/stretchr/testify v1.9.0
 	github.com/tidwall/btree v1.7.0
 	go.etcd.io/bbolt v1.3.10
+	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
 	golang.org/x/mod v0.20.0
 	golang.org/x/net v0.28.0
 	golang.org/x/sync v0.8.0
@@ -172,7 +173,6 @@ require (
 	go.opentelemetry.io/otel/trace v1.28.0 // indirect
 	go.opentelemetry.io/proto/otlp v1.3.1 // indirect
 	golang.org/x/crypto v0.26.0 // indirect
-	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
 	golang.org/x/oauth2 v0.22.0 // indirect
 	golang.org/x/term v0.23.0 // indirect
 	golang.org/x/time v0.5.0 // indirect

pkg/api/api_to_model.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"sort"
 
+	"github.com/blang/semver/v4"
+
 	"github.com/operator-framework/operator-registry/alpha/model"
 	"github.com/operator-framework/operator-registry/alpha/property"
 )
@@ -20,6 +22,11 @@ func ConvertAPIBundleToModelBundle(b *Bundle) (*model.Bundle, error) {
 		return nil, fmt.Errorf("get related iamges: %v", err)
 	}
 
+	vers, err := semver.Parse(b.Version)
+	if err != nil {
+		return nil, fmt.Errorf("parse version %q: %v", b.Version, err)
+	}
+
 	return &model.Bundle{
 		Name:          b.CsvName,
 		Image:         b.BundlePath,
@@ -30,6 +37,7 @@ func ConvertAPIBundleToModelBundle(b *Bundle) (*model.Bundle, error) {
 		Objects:       b.Object,
 		Properties:    bundleProps,
 		RelatedImages: relatedImages,
+		Version:       vers,
 	}, nil
 }
 

pkg/api/conversion_test.go

@@ -4,10 +4,12 @@ import (
 	"encoding/json"
 	"testing"
 
-	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/blang/semver/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+
 	"github.com/operator-framework/operator-registry/alpha/model"
 	"github.com/operator-framework/operator-registry/alpha/property"
 )
@@ -77,6 +79,7 @@ func testModelBundle(t *testing.T) model.Bundle {
 				Image: "quay.io/coreos/etcd-operator@sha256:66a37fd61a06a43969854ee6d3e21087a98b93838e284a6086b13917f96b0d9b",
 			},
 		},
+		Version: semver.MustParse("0.9.4"),
 	}
 	return b
 }