Skip to content
14 changes: 14 additions & 0 deletions alpha/declcfg/declcfg_to_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,19 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
return nil, fmt.Errorf("error parsing bundle %q version %q: %v", b.Name, rawVersion, err)
}

// Parse release version from the package property.
var relver semver.Version
if props.Packages[0].Release != "" {
relver, err = semver.Parse(fmt.Sprintf("0.0.0-%s", props.Packages[0].Release))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would parse successfully if the value of props.Packages[0].Release was 1+1. We would correctly get an error from line 146, but it would be confusing to a user, I think, because we're leaking that we expect the release to only contain "prerelease", which is kind of a out of left field unless you know that we just happen to use prerelease syntax for our release value specification.

I think it may be better to do something like this: https://github.com/joelanford/operator-controller/blob/2cf768082d2e4f129e6ec82740afab55201cbd8e/internal/operator-controller/bundle/versionrelease.go#L79-L101

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that I wanted to use this approach is because I wanted to leverage the version.Compare and version.Validate (as well as PRVersion.Compare)... including all the consistent messaging that semver offers from version.Parse in addition to our layered-on validations.

Copy link
Contributor Author

@grokspawn grokspawn Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the leakage is actually not a bad thing. Since the underlying type is defined in this way already, is it actually even leakage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But line 146 needs a fix, because with a synthetic semver.Version struct with explicitly zero major/minor/patch, those checks would be functionally NOOPs.

if err != nil {
return nil, fmt.Errorf("error parsing bundle %q release version %q: %v", b.Name, props.Packages[0].Release, err)
}
// only need to check for build metadata since we are using explicit zero major, minor, and patch versions above
if len(relver.Build) != 0 {
return nil, fmt.Errorf("bundle %q release version %q cannot contain build metadata", b.Name, props.Packages[0].Release)
}
}

channelDefinedEntries[b.Package] = channelDefinedEntries[b.Package].Delete(b.Name)
found := false
for _, mch := range mpkg.Channels {
Expand All @@ -147,6 +160,7 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
mb.Objects = b.Objects
mb.PropertiesP = props
mb.Version = ver
mb.Release = relver
}
}
if !found {
Expand Down
64 changes: 64 additions & 0 deletions alpha/declcfg/declcfg_to_model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,70 @@ func TestConvertToModel(t *testing.T) {
},
},
},
{
name: "Error/InvalidReleaseVersion",
assertion: hasError(`error parsing bundle "foo.v0.1.0" release version "!!!": Invalid character(s) found in prerelease "!!!"`),
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", func(b *Bundle) {
b.Properties = []property.Property{
property.MustBuildPackageRelease("foo", "0.1.0", "!!!"),
}
})},
},
},
{
name: "Error/InvalidBundleNormalizedName",
assertion: hasError(`invalid index:
└── invalid package "foo":
└── invalid channel "alpha":
└── invalid bundle "foo.v0.1.0-alpha.1.0.0":
└── name "foo.v0.1.0-alpha.1.0.0" does not match normalized name "foo-v0.1.0-alpha.1.0.0"`),
cfg: DeclarativeConfig{
Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: "foo.v0.1.0-alpha.1.0.0"})},
Bundles: []Bundle{newTestBundle("foo", "0.1.0", func(b *Bundle) {
b.Properties = []property.Property{
property.MustBuildPackageRelease("foo", "0.1.0", "alpha.1.0.0"),
}
b.Name = "foo.v0.1.0-alpha.1.0.0"
})},
},
},
{
name: "Success/ValidBundleReleaseVersion",
assertion: require.NoError,
cfg: DeclarativeConfig{
Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: "foo-v0.1.0-alpha.1.0.0"})},
Bundles: []Bundle{newTestBundle("foo", "0.1.0", func(b *Bundle) {
b.Properties = []property.Property{
property.MustBuildPackageRelease("foo", "0.1.0", "alpha.1.0.0"),
}
b.Name = "foo-v0.1.0-alpha.1.0.0"
})},
},
},
{
name: "Error/BundleReleaseWithBuildMetadata",
assertion: hasError(`invalid index:
└── invalid package "foo":
└── invalid channel "alpha":
└── invalid bundle "foo.v0.1.0+alpha.1.0.0-0.0.1":
├── name "foo.v0.1.0+alpha.1.0.0-0.0.1" does not match normalized name "foo-v0.1.0+alpha.1.0.0-0.0.1"
└── cannot use build metadata in version with a release version`),
cfg: DeclarativeConfig{
Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: "foo.v0.1.0+alpha.1.0.0-0.0.1"})},
Bundles: []Bundle{newTestBundle("foo", "0.1.0", func(b *Bundle) {
b.Properties = []property.Property{
property.MustBuildPackageRelease("foo", "0.1.0+alpha.1.0.0", "0.0.1"),
}
b.Name = "foo.v0.1.0+alpha.1.0.0-0.0.1"
})},
},
},
}

for _, s := range specs {
Expand Down
57 changes: 50 additions & 7 deletions alpha/model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package model
import (
"errors"
"fmt"
"slices"
"sort"
"strings"

Expand Down Expand Up @@ -103,24 +104,24 @@ func (m *Package) Validate() error {
}

func (m *Package) validateUniqueBundleVersions() error {
versionsMap := map[string]semver.Version{}
versionsMap := map[string]string{}
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]()
versionsMap[b.VersionString()] = b.VersionString()
if bundlesWithVersion[b.VersionString()] == nil {
bundlesWithVersion[b.VersionString()] = sets.New[string]()
}
bundlesWithVersion[b.Version.String()].Insert(b.Name)
bundlesWithVersion[b.VersionString()].Insert(b.Name)
}
}

versionsSlice := maps.Values(versionsMap)
semver.Sort(versionsSlice)
slices.Sort(versionsSlice)

var errs []error
for _, v := range versionsSlice {
bundles := sets.List(bundlesWithVersion[v.String()])
bundles := sets.List(bundlesWithVersion[v])
if len(bundles) > 1 {
errs = append(errs, fmt.Errorf("{%s: [%s]}", v, strings.Join(bundles, ", ")))
}
Expand Down Expand Up @@ -331,6 +332,41 @@ type Bundle struct {
// These fields are used to compare bundles in a diff.
PropertiesP *property.Properties
Version semver.Version
Release semver.Version
}

func (b *Bundle) VersionString() string {
if len(b.Release.Pre) > 0 {
pres := []string{}
for _, pre := range b.Release.Pre {
pres = append(pres, pre.String())
}
relString := strings.Join(pres, ".")
return strings.Join([]string{b.Version.String(), relString}, "-")
}
return b.Version.String()
}

func (b *Bundle) normalizeName() string {
// if the bundle has release versioning, then the name must include this in standard form:
// <package-name>-v<version>-<release version>
// if no release versioning exists, then just return the bundle name
if len(b.Release.Pre) > 0 {
return strings.Join([]string{b.Package.Name, "v" + b.VersionString()}, "-")
}
return b.Name
}

// order by version, then
// release, if present
func (b *Bundle) Compare(other *Bundle) int {
if b.Name == other.Name {
return 0
}
if b.Version.NE(other.Version) {
return b.Version.Compare(other.Version)
}
return b.Release.Compare(other.Release)
}

func (b *Bundle) Validate() error {
Expand All @@ -339,6 +375,9 @@ func (b *Bundle) Validate() error {
if b.Name == "" {
result.subErrors = append(result.subErrors, errors.New("name must be set"))
}
if b.Name != b.normalizeName() {
result.subErrors = append(result.subErrors, fmt.Errorf("name %q does not match normalized name %q", b.Name, b.normalizeName()))
}
if b.Channel == nil {
result.subErrors = append(result.subErrors, errors.New("channel must be set"))
}
Expand Down Expand Up @@ -379,6 +418,10 @@ func (b *Bundle) Validate() error {
result.subErrors = append(result.subErrors, fmt.Errorf("invalid deprecation: %v", err))
}

if len(b.Version.Build) > 0 && len(b.Release.Pre) > 0 {
result.subErrors = append(result.subErrors, fmt.Errorf("cannot use build metadata in version with a release version"))
}

return result.orNil()
}

Expand Down
36 changes: 36 additions & 0 deletions alpha/model/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"testing"

"github.com/blang/semver/v4"
Expand Down Expand Up @@ -288,6 +289,41 @@ func TestValidators(t *testing.T) {
},
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/DuplicateBundleVersionsReleases",
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-v0.0.1-hotfix.0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1"), Release: semver.MustParse(fmt.Sprintf("0.0.0-%s", "100")), Package: pkg},
"anakin-v0.0.2-hotfix.0.0.1": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.1"), Release: semver.MustParse(fmt.Sprintf("0.0.0-%s", "100")), Package: pkg},
},
},
},
},
assertion: hasError(`duplicate versions found in bundles: [{0.0.1: [anakin.v0.0.1, anakin.v0.0.2]} {0.0.1-100: [anakin.v0.0.1, anakin.v0.0.2]}]`),
},
{
name: "Package/Error/BundleReleaseNormalizedName",
v: &Package{
Name: "anakin",
Channels: map[string]*Channel{
"light": {
Package: pkg,
Name: "light",
Bundles: map[string]*Bundle{
"anakin.v0.0.1.alpha1": {Name: "anakin.v0.0.1.alpha1", Version: semver.MustParse("0.0.1"), Release: semver.MustParse(fmt.Sprintf("0.0.0-%s", "alpha1")), Package: pkg},
},
},
},
},
assertion: hasError(`name "anakin.v0.0.1.alpha1" does not match normalized name "anakin-v0.0.1-alpha1"`),
},
{
name: "Package/Error/NoDefaultChannel",
v: &Package{
Expand Down
4 changes: 4 additions & 0 deletions alpha/property/property.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func (p Property) String() string {
type Package struct {
PackageName string `json:"packageName"`
Version string `json:"version"`
Release string `json:"release,omitzero"`
}

// NOTICE: The Channel properties are for internal use only.
Expand Down Expand Up @@ -247,6 +248,9 @@ func jsonMarshal(p interface{}) ([]byte, error) {
func MustBuildPackage(name, version string) Property {
return MustBuild(&Package{PackageName: name, Version: version})
}
func MustBuildPackageRelease(name, version, relVersion string) Property {
return MustBuild(&Package{PackageName: name, Version: version, Release: relVersion})
}
func MustBuildPackageRequired(name, versionRange string) Property {
return MustBuild(&PackageRequired{name, versionRange})
}
Expand Down
28 changes: 23 additions & 5 deletions alpha/property/property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,12 @@ func TestParse(t *testing.T) {
},
expectProps: &Properties{
Packages: []Package{
{"package1", "0.1.0"},
{"package2", "0.2.0"},
{PackageName: "package1", Version: "0.1.0"},
{PackageName: "package2", Version: "0.2.0"},
},
PackagesRequired: []PackageRequired{
{"package3", ">=1.0.0 <2.0.0-0"},
{"package4", ">=2.0.0 <3.0.0-0"},
{PackageName: "package3", VersionRange: ">=1.0.0 <2.0.0-0"},
{PackageName: "package4", VersionRange: ">=2.0.0 <3.0.0-0"},
},
GVKs: []GVK{
{"group", "Kind1", "v1"},
Expand Down Expand Up @@ -206,10 +206,28 @@ func TestBuild(t *testing.T) {
specs := []spec{
{
name: "Success/Package",
input: &Package{"name", "0.1.0"},
input: &Package{PackageName: "name", Version: "0.1.0"},
assertion: require.NoError,
expectedProperty: propPtr(MustBuildPackage("name", "0.1.0")),
},
{
name: "Success/Package-ReleaseVersionNumber",
input: &Package{PackageName: "name", Version: "0.1.0", Release: "1"},
assertion: require.NoError,
expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "1")),
},
{
name: "Success/Package-ReleaseVersionAlpha",
input: &Package{PackageName: "name", Version: "0.1.0", Release: "gamma"},
assertion: require.NoError,
expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "gamma")),
},
{
name: "Success/Package-ReleaseVersionMixed",
input: &Package{PackageName: "name", Version: "0.1.0", Release: "gamma1"},
assertion: require.NoError,
expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "gamma1")),
},
{
name: "Success/PackageRequired",
input: &PackageRequired{"name", ">=0.1.0"},
Expand Down
26 changes: 19 additions & 7 deletions alpha/template/converter/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,34 @@ import (
"sigs.k8s.io/yaml"

"github.com/operator-framework/operator-registry/alpha/template/basic"
"github.com/operator-framework/operator-registry/alpha/template/substitutes"
"github.com/operator-framework/operator-registry/pkg/image"
)

type Converter struct {
FbcReader io.Reader
OutputFormat string
Registry image.Registry
FbcReader io.Reader
OutputFormat string
Registry image.Registry
DestinationTemplateType string // TODO: when we have a template factory, we can pass it here
}

func (c *Converter) Convert() error {
bt, err := basic.FromReader(c.FbcReader)
if err != nil {
return err
var b []byte
switch c.DestinationTemplateType {
case "basic":
bt, err := basic.FromReader(c.FbcReader)
if err != nil {
return err
}
b, _ = json.MarshalIndent(bt, "", " ")
case "substitutes":
st, err := substitutes.FromReader(c.FbcReader)
if err != nil {
return err
}
b, _ = json.MarshalIndent(st, "", " ")
}

b, _ := json.MarshalIndent(bt, "", " ")
if c.OutputFormat == "json" {
fmt.Fprintln(os.Stdout, string(b))
} else {
Expand Down
Loading