fbc: promote bundle version from property to first-class field

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

Author: joelanford

alpha/action/migrations/002_promote_bundle_version.go

@@ -0,0 +1,44 @@
+package migrations
+
+import (
+	"encoding/json"
+	"slices"
+
+	"github.com/Masterminds/semver/v3"
+
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
+	"github.com/operator-framework/operator-registry/alpha/property"
+)
+
+func promoteBundleVersion(cfg *declcfg.DeclarativeConfig) error {
+	promoteVersion := func(b *declcfg.Bundle) error {
+		// Promote the version from the olm.package property to the bundle field.
+		for _, p := range b.Properties {
+			if p.Type != property.TypePackage {
+				continue
+			}
+			var pkg property.Package
+			if err := json.Unmarshal(p.Value, &pkg); err != nil {
+				return err
+			}
+			version, err := semver.StrictNewVersion(pkg.Version)
+			if err != nil {
+				return err
+			}
+			b.Version = version
+		}
+
+		// Delete the olm.package properties
+		b.Properties = slices.DeleteFunc(b.Properties, func(p property.Property) bool {
+			return p.Type == property.TypePackage
+		})
+		return nil
+	}
+
+	for i := range cfg.Bundles {
+		if err := promoteVersion(&cfg.Bundles[i]); err != nil {
+			return err
+		}
+	}
+	return nil
+}

alpha/action/migrations/migrations.go

@@ -55,6 +55,7 @@ var allMigrations = []Migration{
 	newMigration(NoMigrations, "do nothing", func(_ *declcfg.DeclarativeConfig) error { return nil }),
 	newMigration("bundle-object-to-csv-metadata", `migrates bundles' "olm.bundle.object" to "olm.csv.metadata"`, bundleObjectToCSVMetadata),
 	newMigration("split-icon", `split package icon out into separate "olm.icon" blob`, splitIcon),
+	newMigration("promote-bundle-version", `promote bundle version into first-class bundle field, remove olm.package properties`, promoteBundleVersion),
 }
 
 func NewMigrations(name string) (*Migrations, error) {

alpha/action/migrations/migrations_test.go

@@ -37,7 +37,8 @@ func TestMigrations(t *testing.T) {
 			}
 			return nil
 		},
-		MigrationToken("split-icon"): func(d *declcfg.DeclarativeConfig) error { return nil },
+		MigrationToken("split-icon"):             func(d *declcfg.DeclarativeConfig) error { return nil },
+		MigrationToken("promote-bundle-version"): func(d *declcfg.DeclarativeConfig) error { return nil },
 	}
 
 	tests := []struct {

alpha/declcfg/declcfg.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/Masterminds/semver/v3"
 	"golang.org/x/text/cases"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -83,8 +84,9 @@ type ChannelEntry struct {
 //     evaluation in bundlesEqual().
 type Bundle struct {
 	Schema        string              `json:"schema"`
-	Name          string              `json:"name,omitempty"`
-	Package       string              `json:"package,omitempty"`
+	Name          string              `json:"name"`
+	Package       string              `json:"package"`
+	Version       *semver.Version     `json:"version,omitempty"`
 	Image         string              `json:"image"`
 	Properties    []property.Property `json:"properties,omitempty" hash:"set"`
 	RelatedImages []RelatedImage      `json:"relatedImages,omitempty" hash:"set"`

alpha/declcfg/declcfg_to_model.go

@@ -140,16 +140,18 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
 			return nil, fmt.Errorf("parse properties for bundle %q: %v", b.Name, err)
 		}
 
-		if len(props.Packages) != 1 {
-			return nil, fmt.Errorf("package %q bundle %q must have exactly 1 %q property, found %d", b.Package, b.Name, property.TypePackage, len(props.Packages))
-		}
-
-		if b.Package != props.Packages[0].PackageName {
-			return nil, fmt.Errorf("package %q does not match %q property %q", b.Package, property.TypePackage, props.Packages[0].PackageName)
+		var rawVersion string
+		if b.Version != nil {
+			rawVersion = b.Version.String()
+		} else if len(props.Packages) == 1 {
+			if b.Package != props.Packages[0].PackageName {
+				return nil, fmt.Errorf("package %q does not match %q property %q", b.Package, property.TypePackage, props.Packages[0].PackageName)
+			}
+			rawVersion = props.Packages[0].Version
+		} else {
+			return nil, fmt.Errorf("package %q bundle %q requires either version being set or exactly one property with type %q", b.Package, b.Name, property.TypePackage)
 		}
 
-		// Parse version from the package property.
-		rawVersion := props.Packages[0].Version
 		ver, err := semver.Parse(rawVersion)
 		if err != nil {
 			return nil, fmt.Errorf("error parsing bundle %q version %q: %v", b.Name, rawVersion, err)

alpha/declcfg/declcfg_to_model_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"testing"
 
+	mmsemver "github.com/Masterminds/semver/v3"
 	"github.com/blang/semver/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -120,16 +121,16 @@ func TestConvertToModel(t *testing.T) {
 			},
 		},
 		{
-			name:      "Error/BundleMissingPackageProperty",
-			assertion: hasError(`package "foo" bundle "foo.v0.1.0" must have exactly 1 "olm.package" property, found 0`),
+			name:      "Error/BundleMissingVersionAndPackageProperty",
+			assertion: hasError(`package "foo" bundle "foo.v0.1.0" requires either version being set or exactly one property with type "olm.package"`),
 			cfg: DeclarativeConfig{
 				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
 				Bundles:  []Bundle{newTestBundle("foo", "0.1.0", withNoProperties())},
 			},
 		},
 		{
 			name:      "Error/BundleMultiplePackageProperty",
-			assertion: hasError(`package "foo" bundle "foo.v0.1.0" must have exactly 1 "olm.package" property, found 2`),
+			assertion: hasError(`package "foo" bundle "foo.v0.1.0" requires either version being set or exactly one property with type "olm.package"`),
 			cfg: DeclarativeConfig{
 				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
 				Bundles: []Bundle{newTestBundle("foo", "0.1.0", func(b *Bundle) {
@@ -140,6 +141,15 @@ func TestConvertToModel(t *testing.T) {
 				})},
 			},
 		},
+		{
+			name:      "Success/BundleWithVersionButNoProperties",
+			assertion: require.NoError,
+			cfg: DeclarativeConfig{
+				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
+				Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: testBundleName("foo", "0.1.0")})},
+				Bundles:  []Bundle{newTestBundle("foo", "0.1.0", withVersionField(mmsemver.MustParse("0.1.0")), withNoProperties())},
+			},
+		},
 		{
 			name:      "Success/BundleWithDataButMissingImage",
 			assertion: require.NoError,

alpha/declcfg/helpers_test.go

@@ -6,7 +6,8 @@ import (
 	"sort"
 	"testing"
 
-	"github.com/blang/semver/v4"
+	mmsemver "github.com/Masterminds/semver/v3"
+	bsemver "github.com/blang/semver/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -126,6 +127,12 @@ func buildValidDeclarativeConfig(spec validDeclarativeConfigSpec) DeclarativeCon
 
 type bundleOpt func(*Bundle)
 
+func withVersionField(version *mmsemver.Version) func(*Bundle) {
+	return func(bundle *Bundle) {
+		bundle.Version = version
+	}
+}
+
 func withNoProperties() func(*Bundle) {
 	return func(b *Bundle) {
 		b.Properties = []property.Property{}
@@ -147,6 +154,7 @@ func withNoBundleData() func(*Bundle) {
 
 func newTestBundle(packageName, version string, opts ...bundleOpt) Bundle {
 	csvJSON := fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, testBundleName(packageName, version))
+
 	b := Bundle{
 		Schema:  SchemaBundle,
 		Name:    testBundleName(packageName, version),
@@ -245,7 +253,7 @@ func getBundle(pkg *model.Package, ch *model.Channel, version, replaces string,
 			getCSVJson(pkg.Name, version),
 			getCRDJSON(),
 		},
-		Version: semver.MustParse(version),
+		Version: bsemver.MustParse(version),
 	}
 }
 

alpha/model/model.go

@@ -338,6 +338,7 @@ type Bundle struct {
 	Package       *Package
 	Channel       *Channel
 	Name          string
+	Version       semver.Version
 	Image         string
 	Replaces      string
 	Skips         []string
@@ -354,7 +355,6 @@ type Bundle struct {
 
 	// These fields are used to compare bundles in a diff.
 	PropertiesP *property.Properties
-	Version     semver.Version
 }
 
 func (b *Bundle) Validate() error {
@@ -391,8 +391,20 @@ func (b *Bundle) Validate() error {
 	//	}
 	//}
 
-	if props != nil && len(props.Packages) != 1 {
-		result.subErrors = append(result.subErrors, fmt.Errorf("must be exactly one property with type %q", property.TypePackage))
+	if b.Version.String() == "0.0.0" {
+		result.subErrors = append(result.subErrors, errors.New("version must be set"))
+	}
+
+	if props != nil {
+		if len(props.Packages) > 1 {
+			result.subErrors = append(result.subErrors, errors.New("no more than one olm.package property can be defined"))
+		}
+		if len(props.Packages) == 1 {
+			pkgProp := props.Packages[0]
+			if pkgProp.PackageName != b.Package.Name || pkgProp.Version != b.Version.String() {
+				result.subErrors = append(result.subErrors, errors.New("olm.package property does not match bundle's package name and version"))
+			}
+		}
 	}
 
 	if b.Image == "" && len(b.Objects) == 0 {

alpha/model/model_test.go

@@ -466,6 +466,7 @@ func TestValidators(t *testing.T) {
 				Package:  pkg,
 				Channel:  ch,
 				Name:     "anakin.v0.1.0",
+				Version:  semver.MustParse("0.1.0"),
 				Image:    "registry.io/image",
 				Replaces: "anakin.v0.0.1",
 				Skips:    []string{"anakin.v0.0.2"},
@@ -476,12 +477,29 @@ func TestValidators(t *testing.T) {
 			},
 			assertion: require.NoError,
 		},
+		{
+			name: "Bundle/Success/NoPackageProperty",
+			v: &Bundle{
+				Package:  pkg,
+				Channel:  ch,
+				Name:     "anakin.v0.1.0",
+				Version:  semver.MustParse("0.1.0"),
+				Image:    "registry.io/image",
+				Replaces: "anakin.v0.0.1",
+				Skips:    []string{"anakin.v0.0.2"},
+				Properties: []property.Property{
+					property.MustBuildGVK("skywalker.me", "v1alpha1", "PodRacer"),
+				},
+			},
+			assertion: require.NoError,
+		},
 		{
 			name: "Bundle/Success/ReplacesNotInChannel",
 			v: &Bundle{
 				Package:  pkg,
 				Channel:  ch,
 				Name:     "anakin.v0.1.0",
+				Version:  semver.MustParse("0.1.0"),
 				Image:    "registry.io/image",
 				Replaces: "anakin.v0.0.0",
 				Properties: []property.Property{
@@ -496,6 +514,7 @@ func TestValidators(t *testing.T) {
 				Package: pkg,
 				Channel: ch,
 				Name:    "anakin.v0.1.0",
+				Version: semver.MustParse("0.1.0"),
 				Image:   "",
 				Properties: []property.Property{
 					property.MustBuildPackage("anakin", "0.1.0"),
@@ -513,6 +532,7 @@ func TestValidators(t *testing.T) {
 				Package: pkg,
 				Channel: ch,
 				Name:    "anakin.v0.1.0",
+				Version: semver.MustParse("0.1.0"),
 				Image:   "",
 				Properties: []property.Property{
 					property.MustBuildPackage("anakin", "0.1.0"),
@@ -574,24 +594,56 @@ func TestValidators(t *testing.T) {
 			assertion: hasError(`skip[0] is empty`),
 		},
 		{
-			name: "Bundle/Error/MissingPackage",
+			name: "Bundle/Error/MissingVersion",
 			v: &Bundle{
-				Package:    pkg,
-				Channel:    ch,
-				Name:       "anakin.v0.1.0",
-				Image:      "",
-				Replaces:   "anakin.v0.0.1",
-				Skips:      []string{"anakin.v0.0.2"},
-				Properties: []property.Property{},
+				Package:  pkg,
+				Channel:  ch,
+				Name:     "anakin.v0.1.0",
+				Image:    "",
+				Replaces: "anakin.v0.0.1",
+				Skips:    []string{"anakin.v0.0.2"},
+			},
+			assertion: hasError(`version must be set`),
+		},
+		{
+			name: "Bundle/Error/PackagePropertyMismatchedPackageName",
+			v: &Bundle{
+				Package:  pkg,
+				Channel:  ch,
+				Name:     "anakin.v0.1.0",
+				Version:  semver.MustParse("0.1.0"),
+				Image:    "",
+				Replaces: "anakin.v0.0.1",
+				Skips:    []string{"anakin.v0.0.2"},
+				Properties: []property.Property{
+					property.MustBuildPackage("vader", "0.1.0"),
+				},
+			},
+			assertion: hasError(`olm.package property does not match bundle's package name and version`),
+		},
+		{
+			name: "Bundle/Error/PackagePropertyMismatchedVersion",
+			v: &Bundle{
+				Package:  pkg,
+				Channel:  ch,
+				Name:     "anakin.v0.1.0",
+				Version:  semver.MustParse("0.1.0"),
+				Image:    "",
+				Replaces: "anakin.v0.0.1",
+				Skips:    []string{"anakin.v0.0.2"},
+				Properties: []property.Property{
+					property.MustBuildPackage("anakin", "0.2.0"),
+				},
 			},
-			assertion: hasError(`must be exactly one property with type "olm.package"`),
+			assertion: hasError(`olm.package property does not match bundle's package name and version`),
 		},
 		{
 			name: "Bundle/Error/MultiplePackages",
 			v: &Bundle{
 				Package:  pkg,
 				Channel:  ch,
 				Name:     "anakin.v0.1.0",
+				Version:  semver.MustParse("0.1.0"),
 				Image:    "",
 				Replaces: "anakin.v0.0.1",
 				Skips:    []string{"anakin.v0.0.2"},
@@ -600,7 +652,7 @@ func TestValidators(t *testing.T) {
 					property.MustBuildPackage("anakin", "0.2.0"),
 				},
 			},
-			assertion: hasError(`must be exactly one property with type "olm.package"`),
+			assertion: hasError(`no more than one olm.package property can be defined`),
 		},
 		{
 			name: "RelatedImage/Success/Valid",

go.mod

@@ -5,6 +5,7 @@ go 1.23.3
 toolchain go1.23.6
 
 require (
+	github.com/Masterminds/semver/v3 v3.3.1
 	github.com/akrylysov/pogreb v0.10.2
 	github.com/blang/semver/v4 v4.0.0
 	github.com/containerd/containerd v1.7.27

go.sum

@@ -14,6 +14,8 @@ github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg
 github.com/BurntSushi/toml v1.5.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho=
 github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ=
 github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE=
+github.com/Masterminds/semver/v3 v3.3.1 h1:QtNSWtVZ3nBfk8mAOu/B6v7FMJ+NHTIgUPi7rj+4nv4=
+github.com/Masterminds/semver/v3 v3.3.1/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM=
 github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
 github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
 github.com/Microsoft/hcsshim v0.12.9 h1:2zJy5KA+l0loz1HzEGqyNnjd3fyZA31ZBCGKacp6lLg=