From fd20c2fb0a5dbfb21fed76191116ed638117e69f Mon Sep 17 00:00:00 2001 From: grokspawn Date: Mon, 8 Sep 2025 13:15:58 -0500 Subject: [PATCH 1/8] poc release bundle attribute in version property Signed-off-by: grokspawn --- alpha/declcfg/declcfg_to_model.go | 1 + alpha/declcfg/helpers_test.go | 10 ++++++ alpha/model/model.go | 59 +++++++++++++++++++++++++++---- alpha/property/property.go | 26 ++++++++++++-- alpha/property/property_test.go | 17 ++++++--- pkg/cache/cache.go | 4 +++ 6 files changed, 103 insertions(+), 14 deletions(-) diff --git a/alpha/declcfg/declcfg_to_model.go b/alpha/declcfg/declcfg_to_model.go index 342cab403..be94f9056 100644 --- a/alpha/declcfg/declcfg_to_model.go +++ b/alpha/declcfg/declcfg_to_model.go @@ -147,6 +147,7 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) { mb.Objects = b.Objects mb.PropertiesP = props mb.Version = ver + mb.Release = props.Packages[0].Release } } if !found { diff --git a/alpha/declcfg/helpers_test.go b/alpha/declcfg/helpers_test.go index 1d55f9e2a..928b5c853 100644 --- a/alpha/declcfg/helpers_test.go +++ b/alpha/declcfg/helpers_test.go @@ -145,6 +145,16 @@ func withNoBundleData() func(*Bundle) { } } +func withReleaseVersion(packageName, version string, rel property.Release) func(*Bundle) { + return func(b *Bundle) { + for i, p := range b.Properties { + if p.Type == property.TypePackage { + b.Properties[i] = property.MustBuildPackageRelease(packageName, version, rel.Label, rel.Version.String()) + } + } + } +} + 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{ diff --git a/alpha/model/model.go b/alpha/model/model.go index af6c391e6..22e22d037 100644 --- a/alpha/model/model.go +++ b/alpha/model/model.go @@ -3,6 +3,7 @@ package model import ( "errors" "fmt" + "slices" "sort" "strings" @@ -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, ", "))) } @@ -331,6 +332,47 @@ type Bundle struct { // These fields are used to compare bundles in a diff. PropertiesP *property.Properties Version semver.Version + Release property.Release +} + +func (b *Bundle) VersionString() string { + if b.Release.Label != "" || (b.Release.Version.Major != 0 || b.Release.Version.Minor != 0 || b.Release.Version.Patch != 0) { + return strings.Join([]string{b.Version.String(), b.Release.String()}, "-") + } else { + return b.Version.String() + } +} + +func (b *Bundle) normalizeName() string { + // if the bundle has release versioning, then the name must include this in standard form: + // --- + // if no release versioning exists, then just return the bundle name + if b.Release.Label != "" || (b.Release.Version.Major != 0 || b.Release.Version.Minor != 0 || b.Release.Version.Patch != 0) { + return strings.Join([]string{b.Package.Name, b.Version.String(), b.Release.String()}, "-") + } else { + return b.Name + } +} + +// order by release, if present +// - label first, if present; +// - then version, if present; +// +// then version +func (b *Bundle) Compare(other *Bundle) int { + if b.Name == other.Name { + return 0 + } + if b.Release.Label != other.Release.Label { + return strings.Compare(b.Release.Label, other.Release.Label) + } + if b.Release.Version.NE(other.Release.Version) { + return b.Release.Version.Compare(other.Release.Version) + } + if b.Version.NE(other.Version) { + return b.Version.Compare(other.Version) + } + return 0 } func (b *Bundle) Validate() error { @@ -339,6 +381,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")) } diff --git a/alpha/property/property.go b/alpha/property/property.go index 6fb792dda..ce8dc139a 100644 --- a/alpha/property/property.go +++ b/alpha/property/property.go @@ -6,9 +6,11 @@ import ( "errors" "fmt" "reflect" + "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/blang/semver/v4" "github.com/operator-framework/api/pkg/operators/v1alpha1" ) @@ -35,9 +37,15 @@ func (p Property) String() string { return fmt.Sprintf("type: %q, value: %q", p.Type, p.Value) } +type Release struct { + Label string `json:"label"` + Version semver.Version `json:"version"` +} + type Package struct { - PackageName string `json:"packageName"` - Version string `json:"version"` + PackageName string `json:"packageName"` + Version string `json:"version"` + Release Release `json:"release,omitzero"` } // NOTICE: The Channel properties are for internal use only. @@ -247,6 +255,9 @@ func jsonMarshal(p interface{}) ([]byte, error) { func MustBuildPackage(name, version string) Property { return MustBuild(&Package{PackageName: name, Version: version}) } +func MustBuildPackageRelease(name, version, relLabel, relVersion string) Property { + return MustBuild(&Package{PackageName: name, Version: version, Release: Release{Label: relLabel, Version: semver.MustParse(relVersion)}}) +} func MustBuildPackageRequired(name, versionRange string) Property { return MustBuild(&PackageRequired{name, versionRange}) } @@ -286,3 +297,14 @@ func MustBuildCSVMetadata(csv v1alpha1.ClusterServiceVersion) Property { func MustBuildChannelPriority(name string, priority int) Property { return MustBuild(&Channel{ChannelName: name, Priority: priority}) } + +func (r *Release) String() string { + segments := []string{} + if r.Label != "" { + segments = append(segments, r.Label) + } + if r.Version.Major != 0 || r.Version.Minor != 0 || r.Version.Patch != 0 { + segments = append(segments, r.Version.String()) + } + return strings.Join(segments, "-") +} diff --git a/alpha/property/property_test.go b/alpha/property/property_test.go index 171cec7a0..31e68e095 100644 --- a/alpha/property/property_test.go +++ b/alpha/property/property_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "testing" + "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -132,12 +133,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"}, @@ -206,10 +207,16 @@ 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-ReleaseVersion", + input: &Package{PackageName: "name", Version: "0.1.0", Release: Release{Label: "alpha-whatsit", Version: semver.MustParse("1.1.0-bluefoot")}}, + assertion: require.NoError, + expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "alpha-whatsit", "1.1.0-bluefoot")), + }, { name: "Success/PackageRequired", input: &PackageRequired{"name", ">=0.1.0"}, diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index a02297b36..af48352ee 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -367,6 +367,10 @@ func (c *cache) processPackage(ctx context.Context, reader io.Reader) (packageIn if err != nil { return nil, err } + + // TODO: for each input channel, adjust the entries to respect re-ordering by release attributes as required + // do so as FBC so that the routine may be made common, and re-used for OLMv1 + pkgModel, err := declcfg.ConvertToModel(*pkgFbc) if err != nil { return nil, err From c687f2d0f5eb9e24fd3337b433cda951c76bbf62 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Thu, 11 Sep 2025 09:42:01 -0500 Subject: [PATCH 2/8] replaces via substitutes-for template Signed-off-by: grokspawn --- alpha/template/substitutes/substitutes.go | 55 +++++++++++ cmd/opm/alpha/template/cmd.go | 4 + cmd/opm/alpha/template/substitutes.go | 113 ++++++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 alpha/template/substitutes/substitutes.go create mode 100644 cmd/opm/alpha/template/substitutes.go diff --git a/alpha/template/substitutes/substitutes.go b/alpha/template/substitutes/substitutes.go new file mode 100644 index 000000000..99725e18b --- /dev/null +++ b/alpha/template/substitutes/substitutes.go @@ -0,0 +1,55 @@ +package substitutes + +import ( + "context" + "encoding/json" + "fmt" + "io" + + "k8s.io/apimachinery/pkg/util/yaml" + + "github.com/operator-framework/operator-registry/alpha/declcfg" +) + +type Template struct { + RenderBundle func(context.Context, string) (*declcfg.DeclarativeConfig, error) +} + +type SubstitutesTemplate struct { + Schema string `json:"schema"` + Entries []*declcfg.Meta `json:"entries"` + Substitutes []string `json:"substitutes"` +} + +const schema string = "olm.template.substitutes" + +func parseSpec(reader io.Reader) (*SubstitutesTemplate, error) { + st := &SubstitutesTemplate{} + stDoc := json.RawMessage{} + stDecoder := yaml.NewYAMLOrJSONDecoder(reader, 4096) + err := stDecoder.Decode(&stDoc) + if err != nil { + return nil, fmt.Errorf("decoding template schema: %v", err) + } + err = json.Unmarshal(stDoc, st) + if err != nil { + return nil, fmt.Errorf("unmarshalling template: %v", err) + } + + if st.Schema != schema { + return nil, fmt.Errorf("template has unknown schema (%q), should be %q", st.Schema, schema) + } + + return st, nil +} + +func (t Template) Render(ctx context.Context, reader io.Reader) (*declcfg.DeclarativeConfig, error) { + st, err := parseSpec(reader) + if err != nil { + return nil, fmt.Errorf("render: unable to parse template: %v", err) + } + + // TODO: Implement the actual rendering logic using st.Entries and st.Substitutes + _ = st + return nil, nil +} diff --git a/cmd/opm/alpha/template/cmd.go b/cmd/opm/alpha/template/cmd.go index 55ac55187..2e963ebd8 100644 --- a/cmd/opm/alpha/template/cmd.go +++ b/cmd/opm/alpha/template/cmd.go @@ -21,6 +21,10 @@ func NewCmd() *cobra.Command { // sc.Hidden = true runCmd.AddCommand(sc) + subs := newSubstitutesForTemplateCmd() + // subs.Hidden = true + runCmd.AddCommand(subs) + runCmd.PersistentFlags().StringVarP(&output, "output", "o", "json", "Output format (json|yaml)") return runCmd diff --git a/cmd/opm/alpha/template/substitutes.go b/cmd/opm/alpha/template/substitutes.go new file mode 100644 index 000000000..db1e502d5 --- /dev/null +++ b/cmd/opm/alpha/template/substitutes.go @@ -0,0 +1,113 @@ +package template + +import ( + "context" + "fmt" + "io" + "log" + "os" + + "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + + "github.com/operator-framework/operator-registry/alpha/action" + "github.com/operator-framework/operator-registry/alpha/action/migrations" + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/template/substitutes" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" +) + +func newSubstitutesForTemplateCmd() *cobra.Command { + var ( + migrateLevel string + ) + + cmd := &cobra.Command{ + Use: "substitutes [FILE]", + Short: `Generate a file-based catalog from a single 'substitutes template' file +When FILE is '-' or not provided, the template is read from standard input`, + Long: `Generate a file-based catalog from a single 'substitutes template' file +When FILE is '-' or not provided, the template is read from standard input`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + // Handle different input argument types + // When no arguments or "-" is passed to the command, + // assume input is coming from stdin + // Otherwise open the file passed to the command + data, source, err := util.OpenFileOrStdin(cmd, args) + if err != nil { + return err + } + defer data.Close() + + var write func(declcfg.DeclarativeConfig, io.Writer) error + output, err := cmd.Flags().GetString("output") + if err != nil { + log.Fatalf("unable to determine output format") + } + switch output { + case "json": + write = declcfg.WriteJSON + case "yaml": + write = declcfg.WriteYAML + case "mermaid": + write = func(cfg declcfg.DeclarativeConfig, writer io.Writer) error { + mermaidWriter := declcfg.NewMermaidWriter() + return mermaidWriter.WriteChannels(cfg, writer) + } + default: + return fmt.Errorf("invalid output format %q", output) + } + + // The bundle loading impl is somewhat verbose, even on the happy path, + // so discard all logrus default logger logs. Any important failures will be + // returned from template.Render and logged as fatal errors. + logrus.SetOutput(io.Discard) + + reg, err := util.CreateCLIRegistry(cmd) + if err != nil { + log.Fatalf("creating containerd registry: %v", err) + } + defer func() { + _ = reg.Destroy() + }() + + var m *migrations.Migrations + if migrateLevel != "" { + m, err = migrations.NewMigrations(migrateLevel) + if err != nil { + log.Fatal(err) + } + } + + template := substitutes.Template{ + RenderBundle: func(ctx context.Context, ref string) (*declcfg.DeclarativeConfig, error) { + renderer := action.Render{ + Refs: []string{ref}, + Registry: reg, + AllowedRefMask: action.RefBundleImage, + Migrations: m, + } + return renderer.Run(ctx) + }, + } + + out, err := template.Render(cmd.Context(), data) + if err != nil { + log.Fatalf("substitutes %q: %v", source, err) + } + + if out != nil { + if err := write(*out, os.Stdout); err != nil { + log.Fatal(err) + } + } + + return nil + }, + } + + cmd.Flags().StringVar(&migrateLevel, "migrate-level", "", "Name of the last migration to run (default: none)\n"+migrations.HelpText()) + + return cmd +} From d5252c1f2559cc9d54e335383df14b46be063ed3 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Thu, 9 Oct 2025 13:34:29 -0500 Subject: [PATCH 3/8] updates Signed-off-by: grokspawn --- alpha/model/model.go | 19 ++++++------ alpha/model/model_test.go | 35 +++++++++++++++++++++++ alpha/property/property.go | 2 +- alpha/template/substitutes/substitutes.go | 17 +++++++---- pkg/cache/cache.go | 4 --- 5 files changed, 56 insertions(+), 21 deletions(-) diff --git a/alpha/model/model.go b/alpha/model/model.go index 22e22d037..67b14eabb 100644 --- a/alpha/model/model.go +++ b/alpha/model/model.go @@ -345,33 +345,32 @@ func (b *Bundle) VersionString() string { func (b *Bundle) normalizeName() string { // if the bundle has release versioning, then the name must include this in standard form: - // --- + // -v-- // if no release versioning exists, then just return the bundle name if b.Release.Label != "" || (b.Release.Version.Major != 0 || b.Release.Version.Minor != 0 || b.Release.Version.Patch != 0) { - return strings.Join([]string{b.Package.Name, b.Version.String(), b.Release.String()}, "-") + return strings.Join([]string{b.Package.Name, "v" + b.Version.String(), b.Release.String()}, "-") } else { return b.Name } } -// order by release, if present -// - label first, if present; -// - then version, if present; -// -// then version +// order by version, then +// release, if present +// - label first, if present +// - then version, 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) + } if b.Release.Label != other.Release.Label { return strings.Compare(b.Release.Label, other.Release.Label) } if b.Release.Version.NE(other.Release.Version) { return b.Release.Version.Compare(other.Release.Version) } - if b.Version.NE(other.Version) { - return b.Version.Compare(other.Version) - } return 0 } diff --git a/alpha/model/model_test.go b/alpha/model/model_test.go index 248de9c85..4ca588523 100644 --- a/alpha/model/model_test.go +++ b/alpha/model/model_test.go @@ -288,6 +288,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-alpha-0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1"), Release: property.Release{Label: "alpha", Version: semver.MustParse("0.0.1")}, Package: pkg}, + "anakin-v0.0.2-alpha-0.0.1": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.1"), Release: property.Release{Label: "alpha", Version: semver.MustParse("0.0.1")}, Package: pkg}, + }, + }, + }, + }, + assertion: hasError(`duplicate versions found in bundles: [{0.0.1: [anakin.v0.0.1, anakin.v0.0.2]} {0.0.1-alpha-0.0.1: [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.alpha.0.0.1": {Name: "anakin.v0.0.1.alpha.0.0.1", Version: semver.MustParse("0.0.1"), Release: property.Release{Label: "alpha", Version: semver.MustParse("0.0.1")}, Package: pkg}, + }, + }, + }, + }, + assertion: hasError(`name "anakin.v0.0.1.alpha.0.0.1" does not match normalized name "anakin-v0.0.1-alpha-0.0.1"`), + }, { name: "Package/Error/NoDefaultChannel", v: &Package{ diff --git a/alpha/property/property.go b/alpha/property/property.go index ce8dc139a..ebe54e46c 100644 --- a/alpha/property/property.go +++ b/alpha/property/property.go @@ -8,9 +8,9 @@ import ( "reflect" "strings" + "github.com/blang/semver/v4" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/blang/semver/v4" "github.com/operator-framework/api/pkg/operators/v1alpha1" ) diff --git a/alpha/template/substitutes/substitutes.go b/alpha/template/substitutes/substitutes.go index 99725e18b..c41286011 100644 --- a/alpha/template/substitutes/substitutes.go +++ b/alpha/template/substitutes/substitutes.go @@ -15,16 +15,21 @@ type Template struct { RenderBundle func(context.Context, string) (*declcfg.DeclarativeConfig, error) } -type SubstitutesTemplate struct { - Schema string `json:"schema"` - Entries []*declcfg.Meta `json:"entries"` - Substitutes []string `json:"substitutes"` +type Substitute struct { + Name string `json:"name"` + Base string `json:"base"` +} + +type SubstitutesForTemplate struct { + Schema string `json:"schema"` + Entries []*declcfg.Meta `json:"entries"` + Substitutions []Substitute `json:"substitutions"` } const schema string = "olm.template.substitutes" -func parseSpec(reader io.Reader) (*SubstitutesTemplate, error) { - st := &SubstitutesTemplate{} +func parseSpec(reader io.Reader) (*SubstitutesForTemplate, error) { + st := &SubstitutesForTemplate{} stDoc := json.RawMessage{} stDecoder := yaml.NewYAMLOrJSONDecoder(reader, 4096) err := stDecoder.Decode(&stDoc) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index af48352ee..a02297b36 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -367,10 +367,6 @@ func (c *cache) processPackage(ctx context.Context, reader io.Reader) (packageIn if err != nil { return nil, err } - - // TODO: for each input channel, adjust the entries to respect re-ordering by release attributes as required - // do so as FBC so that the routine may be made common, and re-used for OLMv1 - pkgModel, err := declcfg.ConvertToModel(*pkgFbc) if err != nil { return nil, err From 83488030a50902d7398e5c735a32b214539f6b90 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Tue, 14 Oct 2025 13:16:03 -0500 Subject: [PATCH 4/8] with pointer field/omitempty instead of instance Signed-off-by: grokspawn --- alpha/declcfg/declcfg_to_model.go | 4 +++- alpha/property/property.go | 4 ++-- alpha/property/property_test.go | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/alpha/declcfg/declcfg_to_model.go b/alpha/declcfg/declcfg_to_model.go index be94f9056..3c2de19ce 100644 --- a/alpha/declcfg/declcfg_to_model.go +++ b/alpha/declcfg/declcfg_to_model.go @@ -147,7 +147,9 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) { mb.Objects = b.Objects mb.PropertiesP = props mb.Version = ver - mb.Release = props.Packages[0].Release + if props.Packages[0].Release != nil { + mb.Release = *(props.Packages[0].Release) + } } } if !found { diff --git a/alpha/property/property.go b/alpha/property/property.go index ebe54e46c..e026fd026 100644 --- a/alpha/property/property.go +++ b/alpha/property/property.go @@ -45,7 +45,7 @@ type Release struct { type Package struct { PackageName string `json:"packageName"` Version string `json:"version"` - Release Release `json:"release,omitzero"` + Release *Release `json:"release,omitempty"` } // NOTICE: The Channel properties are for internal use only. @@ -256,7 +256,7 @@ func MustBuildPackage(name, version string) Property { return MustBuild(&Package{PackageName: name, Version: version}) } func MustBuildPackageRelease(name, version, relLabel, relVersion string) Property { - return MustBuild(&Package{PackageName: name, Version: version, Release: Release{Label: relLabel, Version: semver.MustParse(relVersion)}}) + return MustBuild(&Package{PackageName: name, Version: version, Release: &Release{Label: relLabel, Version: semver.MustParse(relVersion)}}) } func MustBuildPackageRequired(name, versionRange string) Property { return MustBuild(&PackageRequired{name, versionRange}) diff --git a/alpha/property/property_test.go b/alpha/property/property_test.go index 31e68e095..d55aebecc 100644 --- a/alpha/property/property_test.go +++ b/alpha/property/property_test.go @@ -213,7 +213,7 @@ func TestBuild(t *testing.T) { }, { name: "Success/Package-ReleaseVersion", - input: &Package{PackageName: "name", Version: "0.1.0", Release: Release{Label: "alpha-whatsit", Version: semver.MustParse("1.1.0-bluefoot")}}, + input: &Package{PackageName: "name", Version: "0.1.0", Release: &Release{Label: "alpha-whatsit", Version: semver.MustParse("1.1.0-bluefoot")}}, assertion: require.NoError, expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "alpha-whatsit", "1.1.0-bluefoot")), }, From 754cbc630e92aa4dd43f73c158cb5bdc8951d313 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Wed, 15 Oct 2025 09:27:41 -0500 Subject: [PATCH 5/8] convert to prerelease semver field only Signed-off-by: grokspawn --- alpha/declcfg/declcfg_to_model.go | 4 +--- alpha/declcfg/helpers_test.go | 10 --------- alpha/model/model.go | 28 +++++++++---------------- alpha/model/model_test.go | 14 ++++++------- alpha/property/property.go | 34 +++++++++++-------------------- alpha/property/property_test.go | 19 +++++++++++++---- 6 files changed, 45 insertions(+), 64 deletions(-) diff --git a/alpha/declcfg/declcfg_to_model.go b/alpha/declcfg/declcfg_to_model.go index 3c2de19ce..be94f9056 100644 --- a/alpha/declcfg/declcfg_to_model.go +++ b/alpha/declcfg/declcfg_to_model.go @@ -147,9 +147,7 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) { mb.Objects = b.Objects mb.PropertiesP = props mb.Version = ver - if props.Packages[0].Release != nil { - mb.Release = *(props.Packages[0].Release) - } + mb.Release = props.Packages[0].Release } } if !found { diff --git a/alpha/declcfg/helpers_test.go b/alpha/declcfg/helpers_test.go index 928b5c853..1d55f9e2a 100644 --- a/alpha/declcfg/helpers_test.go +++ b/alpha/declcfg/helpers_test.go @@ -145,16 +145,6 @@ func withNoBundleData() func(*Bundle) { } } -func withReleaseVersion(packageName, version string, rel property.Release) func(*Bundle) { - return func(b *Bundle) { - for i, p := range b.Properties { - if p.Type == property.TypePackage { - b.Properties[i] = property.MustBuildPackageRelease(packageName, version, rel.Label, rel.Version.String()) - } - } - } -} - 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{ diff --git a/alpha/model/model.go b/alpha/model/model.go index 67b14eabb..992d92e40 100644 --- a/alpha/model/model.go +++ b/alpha/model/model.go @@ -332,32 +332,30 @@ type Bundle struct { // These fields are used to compare bundles in a diff. PropertiesP *property.Properties Version semver.Version - Release property.Release + Release semver.PRVersion } func (b *Bundle) VersionString() string { - if b.Release.Label != "" || (b.Release.Version.Major != 0 || b.Release.Version.Minor != 0 || b.Release.Version.Patch != 0) { - return strings.Join([]string{b.Version.String(), b.Release.String()}, "-") - } else { - return b.Version.String() + relString := b.Release.String() + if relString != "" { + 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: - // -v-- + // -v- // if no release versioning exists, then just return the bundle name - if b.Release.Label != "" || (b.Release.Version.Major != 0 || b.Release.Version.Minor != 0 || b.Release.Version.Patch != 0) { + relString := b.Release.String() + if relString != "" { return strings.Join([]string{b.Package.Name, "v" + b.Version.String(), b.Release.String()}, "-") - } else { - return b.Name } + return b.Name } // order by version, then // release, if present -// - label first, if present -// - then version, if present func (b *Bundle) Compare(other *Bundle) int { if b.Name == other.Name { return 0 @@ -365,13 +363,7 @@ func (b *Bundle) Compare(other *Bundle) int { if b.Version.NE(other.Version) { return b.Version.Compare(other.Version) } - if b.Release.Label != other.Release.Label { - return strings.Compare(b.Release.Label, other.Release.Label) - } - if b.Release.Version.NE(other.Release.Version) { - return b.Release.Version.Compare(other.Release.Version) - } - return 0 + return b.Release.Compare(other.Release) } func (b *Bundle) Validate() error { diff --git a/alpha/model/model_test.go b/alpha/model/model_test.go index 4ca588523..54b217a91 100644 --- a/alpha/model/model_test.go +++ b/alpha/model/model_test.go @@ -297,15 +297,15 @@ func TestValidators(t *testing.T) { 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-alpha-0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1"), Release: property.Release{Label: "alpha", Version: semver.MustParse("0.0.1")}, Package: pkg}, - "anakin-v0.0.2-alpha-0.0.1": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.1"), Release: property.Release{Label: "alpha", Version: semver.MustParse("0.0.1")}, Package: pkg}, + "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: property.MustBuildPackageReleaseVersion("100"), Package: pkg}, + "anakin-v0.0.2-hotfix.0.0.1": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.1"), Release: property.MustBuildPackageReleaseVersion("100"), Package: pkg}, }, }, }, }, - assertion: hasError(`duplicate versions found in bundles: [{0.0.1: [anakin.v0.0.1, anakin.v0.0.2]} {0.0.1-alpha-0.0.1: [anakin.v0.0.1, anakin.v0.0.2]}]`), + 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", @@ -316,12 +316,12 @@ func TestValidators(t *testing.T) { Package: pkg, Name: "light", Bundles: map[string]*Bundle{ - "anakin.v0.0.1.alpha.0.0.1": {Name: "anakin.v0.0.1.alpha.0.0.1", Version: semver.MustParse("0.0.1"), Release: property.Release{Label: "alpha", Version: semver.MustParse("0.0.1")}, Package: pkg}, + "anakin.v0.0.1.alpha1": {Name: "anakin.v0.0.1.alpha1", Version: semver.MustParse("0.0.1"), Release: property.MustBuildPackageReleaseVersion("alpha1"), Package: pkg}, }, }, }, }, - assertion: hasError(`name "anakin.v0.0.1.alpha.0.0.1" does not match normalized name "anakin-v0.0.1-alpha-0.0.1"`), + assertion: hasError(`name "anakin.v0.0.1.alpha1" does not match normalized name "anakin-v0.0.1-alpha1"`), }, { name: "Package/Error/NoDefaultChannel", diff --git a/alpha/property/property.go b/alpha/property/property.go index e026fd026..069ed917c 100644 --- a/alpha/property/property.go +++ b/alpha/property/property.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "reflect" - "strings" "github.com/blang/semver/v4" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -37,15 +36,10 @@ func (p Property) String() string { return fmt.Sprintf("type: %q, value: %q", p.Type, p.Value) } -type Release struct { - Label string `json:"label"` - Version semver.Version `json:"version"` -} - type Package struct { - PackageName string `json:"packageName"` - Version string `json:"version"` - Release *Release `json:"release,omitempty"` + PackageName string `json:"packageName"` + Version string `json:"version"` + Release semver.PRVersion `json:"release,omitzero"` } // NOTICE: The Channel properties are for internal use only. @@ -255,8 +249,15 @@ func jsonMarshal(p interface{}) ([]byte, error) { func MustBuildPackage(name, version string) Property { return MustBuild(&Package{PackageName: name, Version: version}) } -func MustBuildPackageRelease(name, version, relLabel, relVersion string) Property { - return MustBuild(&Package{PackageName: name, Version: version, Release: &Release{Label: relLabel, Version: semver.MustParse(relVersion)}}) +func MustBuildPackageReleaseVersion(relVersion string) semver.PRVersion { + val, err := semver.NewPRVersion(relVersion) + if err != nil { + panic(err) + } + return val +} +func MustBuildPackageRelease(name, version, relVersion string) Property { + return MustBuild(&Package{PackageName: name, Version: version, Release: MustBuildPackageReleaseVersion(relVersion)}) } func MustBuildPackageRequired(name, versionRange string) Property { return MustBuild(&PackageRequired{name, versionRange}) @@ -297,14 +298,3 @@ func MustBuildCSVMetadata(csv v1alpha1.ClusterServiceVersion) Property { func MustBuildChannelPriority(name string, priority int) Property { return MustBuild(&Channel{ChannelName: name, Priority: priority}) } - -func (r *Release) String() string { - segments := []string{} - if r.Label != "" { - segments = append(segments, r.Label) - } - if r.Version.Major != 0 || r.Version.Minor != 0 || r.Version.Patch != 0 { - segments = append(segments, r.Version.String()) - } - return strings.Join(segments, "-") -} diff --git a/alpha/property/property_test.go b/alpha/property/property_test.go index d55aebecc..3394e2e53 100644 --- a/alpha/property/property_test.go +++ b/alpha/property/property_test.go @@ -4,7 +4,6 @@ import ( "encoding/json" "testing" - "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -212,10 +211,22 @@ func TestBuild(t *testing.T) { expectedProperty: propPtr(MustBuildPackage("name", "0.1.0")), }, { - name: "Success/Package-ReleaseVersion", - input: &Package{PackageName: "name", Version: "0.1.0", Release: &Release{Label: "alpha-whatsit", Version: semver.MustParse("1.1.0-bluefoot")}}, + name: "Success/Package-ReleaseVersionNumber", + input: &Package{PackageName: "name", Version: "0.1.0", Release: MustBuildPackageReleaseVersion("1")}, assertion: require.NoError, - expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "alpha-whatsit", "1.1.0-bluefoot")), + expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "1")), + }, + { + name: "Success/Package-ReleaseVersionAlpha", + input: &Package{PackageName: "name", Version: "0.1.0", Release: MustBuildPackageReleaseVersion("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: MustBuildPackageReleaseVersion("gamma1")}, + assertion: require.NoError, + expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "gamma1")), }, { name: "Success/PackageRequired", From 1eb6993ef0b227b5347f1b3ea2be7477d9baec06 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Wed, 15 Oct 2025 15:45:07 -0500 Subject: [PATCH 6/8] convert to semver.Version, using only prerelease Signed-off-by: grokspawn --- alpha/declcfg/declcfg_to_model.go | 14 +++++- alpha/declcfg/declcfg_to_model_test.go | 64 ++++++++++++++++++++++++++ alpha/model/model.go | 19 +++++--- alpha/model/model_test.go | 7 +-- alpha/property/property.go | 16 ++----- alpha/property/property_test.go | 6 +-- 6 files changed, 101 insertions(+), 25 deletions(-) diff --git a/alpha/declcfg/declcfg_to_model.go b/alpha/declcfg/declcfg_to_model.go index be94f9056..d2c5a79bb 100644 --- a/alpha/declcfg/declcfg_to_model.go +++ b/alpha/declcfg/declcfg_to_model.go @@ -135,6 +135,18 @@ 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)) + if err != nil { + return nil, fmt.Errorf("error parsing bundle %q release version %q: %v", b.Name, props.Packages[0].Release, err) + } + if relver.Major != 0 || relver.Minor != 0 || relver.Patch != 0 || len(relver.Build) != 0 { + return nil, fmt.Errorf("bundle %q release version %q must only contain prerelease", b.Name, props.Packages[0].Release) + } + } + channelDefinedEntries[b.Package] = channelDefinedEntries[b.Package].Delete(b.Name) found := false for _, mch := range mpkg.Channels { @@ -147,7 +159,7 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) { mb.Objects = b.Objects mb.PropertiesP = props mb.Version = ver - mb.Release = props.Packages[0].Release + mb.Release = relver } } if !found { diff --git a/alpha/declcfg/declcfg_to_model_test.go b/alpha/declcfg/declcfg_to_model_test.go index de8639c1b..40b45009e 100644 --- a/alpha/declcfg/declcfg_to_model_test.go +++ b/alpha/declcfg/declcfg_to_model_test.go @@ -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 { diff --git a/alpha/model/model.go b/alpha/model/model.go index 992d92e40..5457fe595 100644 --- a/alpha/model/model.go +++ b/alpha/model/model.go @@ -332,12 +332,16 @@ type Bundle struct { // These fields are used to compare bundles in a diff. PropertiesP *property.Properties Version semver.Version - Release semver.PRVersion + Release semver.Version } func (b *Bundle) VersionString() string { - relString := b.Release.String() - if relString != "" { + 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() @@ -347,9 +351,8 @@ func (b *Bundle) normalizeName() string { // if the bundle has release versioning, then the name must include this in standard form: // -v- // if no release versioning exists, then just return the bundle name - relString := b.Release.String() - if relString != "" { - return strings.Join([]string{b.Package.Name, "v" + b.Version.String(), b.Release.String()}, "-") + if len(b.Release.Pre) > 0 { + return strings.Join([]string{b.Package.Name, "v" + b.VersionString()}, "-") } return b.Name } @@ -415,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() } diff --git a/alpha/model/model_test.go b/alpha/model/model_test.go index 54b217a91..a125d7978 100644 --- a/alpha/model/model_test.go +++ b/alpha/model/model_test.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "encoding/json" "errors" + "fmt" "testing" "github.com/blang/semver/v4" @@ -299,8 +300,8 @@ func TestValidators(t *testing.T) { 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: property.MustBuildPackageReleaseVersion("100"), Package: pkg}, - "anakin-v0.0.2-hotfix.0.0.1": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.1"), Release: property.MustBuildPackageReleaseVersion("100"), Package: pkg}, + "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}, }, }, }, @@ -316,7 +317,7 @@ func TestValidators(t *testing.T) { 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: property.MustBuildPackageReleaseVersion("alpha1"), Package: pkg}, + "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}, }, }, }, diff --git a/alpha/property/property.go b/alpha/property/property.go index 069ed917c..dcca33ce5 100644 --- a/alpha/property/property.go +++ b/alpha/property/property.go @@ -7,7 +7,6 @@ import ( "fmt" "reflect" - "github.com/blang/semver/v4" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -37,9 +36,9 @@ func (p Property) String() string { } type Package struct { - PackageName string `json:"packageName"` - Version string `json:"version"` - Release semver.PRVersion `json:"release,omitzero"` + PackageName string `json:"packageName"` + Version string `json:"version"` + Release string `json:"release,omitzero"` } // NOTICE: The Channel properties are for internal use only. @@ -249,15 +248,8 @@ func jsonMarshal(p interface{}) ([]byte, error) { func MustBuildPackage(name, version string) Property { return MustBuild(&Package{PackageName: name, Version: version}) } -func MustBuildPackageReleaseVersion(relVersion string) semver.PRVersion { - val, err := semver.NewPRVersion(relVersion) - if err != nil { - panic(err) - } - return val -} func MustBuildPackageRelease(name, version, relVersion string) Property { - return MustBuild(&Package{PackageName: name, Version: version, Release: MustBuildPackageReleaseVersion(relVersion)}) + return MustBuild(&Package{PackageName: name, Version: version, Release: relVersion}) } func MustBuildPackageRequired(name, versionRange string) Property { return MustBuild(&PackageRequired{name, versionRange}) diff --git a/alpha/property/property_test.go b/alpha/property/property_test.go index 3394e2e53..bb67d5264 100644 --- a/alpha/property/property_test.go +++ b/alpha/property/property_test.go @@ -212,19 +212,19 @@ func TestBuild(t *testing.T) { }, { name: "Success/Package-ReleaseVersionNumber", - input: &Package{PackageName: "name", Version: "0.1.0", Release: MustBuildPackageReleaseVersion("1")}, + 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: MustBuildPackageReleaseVersion("gamma")}, + 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: MustBuildPackageReleaseVersion("gamma1")}, + input: &Package{PackageName: "name", Version: "0.1.0", Release: "gamma1"}, assertion: require.NoError, expectedProperty: propPtr(MustBuildPackageRelease("name", "0.1.0", "gamma1")), }, From 31ef9611eaa9e77e3a1bf79352a974fff20f7d91 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Fri, 17 Oct 2025 13:51:01 -0500 Subject: [PATCH 7/8] implement template expansion logic, tests Signed-off-by: grokspawn Helped-by: Claude-LLM --- alpha/template/substitutes/substitutes.go | 110 +- .../template/substitutes/substitutes_test.go | 1231 +++++++++++++++++ 2 files changed, 1336 insertions(+), 5 deletions(-) create mode 100644 alpha/template/substitutes/substitutes_test.go diff --git a/alpha/template/substitutes/substitutes.go b/alpha/template/substitutes/substitutes.go index c41286011..82522752a 100644 --- a/alpha/template/substitutes/substitutes.go +++ b/alpha/template/substitutes/substitutes.go @@ -16,8 +16,8 @@ type Template struct { } type Substitute struct { - Name string `json:"name"` - Base string `json:"base"` + Name string `json:"name"` // the bundle image pullspec to substitute + Base string `json:"base"` // the bundle name to substitute for } type SubstitutesForTemplate struct { @@ -54,7 +54,107 @@ func (t Template) Render(ctx context.Context, reader io.Reader) (*declcfg.Declar return nil, fmt.Errorf("render: unable to parse template: %v", err) } - // TODO: Implement the actual rendering logic using st.Entries and st.Substitutes - _ = st - return nil, nil + // Create DeclarativeConfig from template entries + cfg, err := declcfg.LoadSlice(st.Entries) + if err != nil { + return nil, fmt.Errorf("render: unable to create declarative config from entries: %v", err) + } + + _, err = declcfg.ConvertToModel(*cfg) + if err != nil { + return nil, fmt.Errorf("render: entries are not valid FBC: %v", err) + } + + // Process each substitution + for _, substitution := range st.Substitutions { + err := t.processSubstitution(ctx, cfg, substitution) + if err != nil { + return nil, fmt.Errorf("render: error processing substitution %s->%s: %v", substitution.Base, substitution.Name, err) + } + } + + return cfg, nil +} + +// processSubstitution handles the complex logic for processing a single substitution +func (t Template) processSubstitution(ctx context.Context, cfg *declcfg.DeclarativeConfig, substitution Substitute) error { + // Validate substitution fields - all are required + if substitution.Name == "" { + return fmt.Errorf("substitution name cannot be empty") + } + if substitution.Base == "" { + return fmt.Errorf("substitution base cannot be empty") + } + if substitution.Name == substitution.Base { + return fmt.Errorf("substitution name and base cannot be the same") + } + + substituteCfg, err := t.RenderBundle(ctx, substitution.Name) + if err != nil { + return fmt.Errorf("failed to render bundle image reference %q: %v", substitution.Name, err) + } + + substituteBundle := &substituteCfg.Bundles[0] + + // Iterate over all channels + for i := range cfg.Channels { + channel := &cfg.Channels[i] + + // First pass: find entries that have substitution.base as their name + // Only process original entries, not substitution entries (they have empty replaces after clearing) + var entriesToSubstitute []int + for j := range channel.Entries { + entry := &channel.Entries[j] + if entry.Name == substitution.Base { + entriesToSubstitute = append(entriesToSubstitute, j) + } + } + + // Create new entries for each substitution (process in reverse order to avoid index issues) + for i := len(entriesToSubstitute) - 1; i >= 0; i-- { + entryIndex := entriesToSubstitute[i] + // Create a new channel entry for substitution.name + newEntry := declcfg.ChannelEntry{ + Name: substituteBundle.Name, + Replaces: channel.Entries[entryIndex].Replaces, + Skips: channel.Entries[entryIndex].Skips, + SkipRange: channel.Entries[entryIndex].SkipRange, + } + + // Add skip relationship to substitution.base + newEntry.Skips = append(newEntry.Skips, substitution.Base) + + // Add the new entry to the channel + channel.Entries = append(channel.Entries, newEntry) + + // Clear the original entry's replaces/skips/skipRange since they moved to the new entry + channel.Entries[entryIndex].Replaces = "" + channel.Entries[entryIndex].Skips = nil + channel.Entries[entryIndex].SkipRange = "" + } + + // Second pass: update all references to substitution.base to point to substitution.name + // Skip the newly created substitution entries (they are at the end) + originalEntryCount := len(channel.Entries) - len(entriesToSubstitute) + for j := 0; j < originalEntryCount; j++ { + entry := &channel.Entries[j] + + // If this entry replaces substitution.base, update it to replace substitution.name + if entry.Replaces == substitution.Base { + entry.Replaces = substituteBundle.Name + entry.Skips = append(entry.Skips, substitution.Base) + } + } + } + + // Add the substitute bundle to the config (only once) + cfg.Bundles = append(cfg.Bundles, *substituteBundle) + + // now validate the resulting config + _, err = declcfg.ConvertToModel(*cfg) + if err != nil { + return fmt.Errorf("resulting config is not valid FBC: %v", err) + } + + return nil } diff --git a/alpha/template/substitutes/substitutes_test.go b/alpha/template/substitutes/substitutes_test.go new file mode 100644 index 000000000..432d40302 --- /dev/null +++ b/alpha/template/substitutes/substitutes_test.go @@ -0,0 +1,1231 @@ +package substitutes + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" +) + +// Helper function to create a mock template for testing +func createMockTemplate() Template { + return Template{ + RenderBundle: func(ctx context.Context, imageRef string) (*declcfg.DeclarativeConfig, error) { + // Extract package and version from image reference (simplified for testing) + packageName := "testoperator" + version := "1.2.0" + if strings.Contains(imageRef, "test-bundle") { + packageName = "test" + version = "1.0.0" + } + // Extract version from image reference if it contains a version + if strings.Contains(imageRef, ":v") { + parts := strings.Split(imageRef, ":v") + if len(parts) == 2 { + version = parts[1] + } + } + + // Create bundle name based on version for predictable naming + bundleName := packageName + "-v" + version + "-alpha" + + return &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: packageName, + DefaultChannel: "stable", + }, + }, + Bundles: []declcfg.Bundle{ + { + Schema: "olm.bundle", + Name: bundleName, + Package: packageName, + Image: imageRef, + Properties: []property.Property{ + property.MustBuildPackage(packageName, version), + property.MustBuildBundleObject([]byte(fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, imageRef))), + property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)), + }, + RelatedImages: []declcfg.RelatedImage{ + {Name: "bundle", Image: imageRef}, + }, + CsvJSON: fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, imageRef), + Objects: []string{ + fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, imageRef), + `{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`, + }, + }, + }, + }, nil + }, + } +} + +// Helper function to create a test DeclarativeConfig +func createTestDeclarativeConfig() *declcfg.DeclarativeConfig { + return &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "testoperator", + DefaultChannel: "stable", + }, + }, + Channels: []declcfg.Channel{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Entries: []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.2.0-alpha", Replaces: "testoperator-v1.1.0-alpha", Skips: []string{"testoperator-v1.0.0-alpha"}}, + }, + }, + }, + Bundles: []declcfg.Bundle{ + { + Schema: "olm.bundle", + Name: "testoperator-v1.0.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.0.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.0.0", "alpha"), + }, + }, + { + Schema: "olm.bundle", + Name: "testoperator-v1.1.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.1.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.1.0", "alpha"), + }, + }, + { + Schema: "olm.bundle", + Name: "testoperator-v1.2.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.2.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.2.0", "alpha"), + }, + }, + }, + } +} + +// Helper function to create a valid test package Meta entry +// nolint: unparam +func createValidTestPackageMeta(name, defaultChannel string) *declcfg.Meta { + pkg := declcfg.Package{ + Schema: "olm.package", + Name: name, + DefaultChannel: defaultChannel, + Description: fmt.Sprintf("%s operator", name), + } + + blob, err := json.Marshal(pkg) + if err != nil { + panic(err) + } + + return &declcfg.Meta{ + Schema: "olm.package", + Name: name, + Package: name, + Blob: json.RawMessage(blob), + } +} + +// Helper function to create a valid test bundle Meta entry with proper naming convention +// nolint: unparam +func createValidTestBundleMeta(name, packageName, version, release string) *declcfg.Meta { + var bundleName string + var properties []property.Property + + if release != "" { + // Create bundle name following the normalizeName convention: package-vversion-release + bundleName = fmt.Sprintf("%s-v%s-%s", packageName, version, release) + properties = []property.Property{ + property.MustBuildPackageRelease(packageName, version, release), + property.MustBuildBundleObject([]byte(fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, bundleName))), + property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)), + } + } else { + // Use simple naming convention for bundles without release version + bundleName = name + properties = []property.Property{ + property.MustBuildPackage(packageName, version), + property.MustBuildBundleObject([]byte(fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, bundleName))), + property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)), + } + } + + bundle := declcfg.Bundle{ + Schema: "olm.bundle", + Name: bundleName, + Package: packageName, + Image: fmt.Sprintf("quay.io/test/%s-bundle:v%s", packageName, version), + Properties: properties, + RelatedImages: []declcfg.RelatedImage{ + { + Name: "bundle", + Image: fmt.Sprintf("quay.io/test/%s-bundle:v%s", packageName, version), + }, + }, + CsvJSON: fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, bundleName), + Objects: []string{ + fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, bundleName), + `{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`, + }, + } + + blob, err := json.Marshal(bundle) + if err != nil { + panic(err) + } + + return &declcfg.Meta{ + Schema: "olm.bundle", + Name: bundleName, + Package: packageName, + Blob: json.RawMessage(blob), + } +} + +// Helper function to create a valid test channel Meta entry with proper bundle names +// nolint: unparam +func createValidTestChannelMeta(name, packageName string, entries []declcfg.ChannelEntry) *declcfg.Meta { + channel := declcfg.Channel{ + Schema: "olm.channel", + Name: name, + Package: packageName, + Entries: entries, + } + + blob, err := json.Marshal(channel) + if err != nil { + panic(err) + } + + return &declcfg.Meta{ + Schema: "olm.channel", + Name: name, + Package: packageName, + Blob: json.RawMessage(blob), + } +} + +func TestParseSpec(t *testing.T) { + tests := []struct { + name string + input string + expected *SubstitutesForTemplate + expectError bool + errorMsg string + }{ + { + name: "Success/valid template with substitutions", + input: ` +schema: olm.template.substitutes +entries: + - schema: olm.channel + name: stable + package: testoperator + blob: '{"schema":"olm.channel","name":"stable","package":"testoperator","entries":[{"name":"testoperator.v1.0.0"}]}' +substitutions: + - name: testoperator.v1.1.0 + base: testoperator.v1.0.0 +`, + expected: &SubstitutesForTemplate{ + Schema: "olm.template.substitutes", + Entries: []*declcfg.Meta{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Blob: json.RawMessage(`{"schema":"olm.channel","name":"stable","package":"testoperator","entries":[{"name":"testoperator.v1.0.0"}]}`), + }, + }, + Substitutions: []Substitute{ + {Name: "testoperator.v1.1.0", Base: "testoperator.v1.0.0"}, + }, + }, + expectError: false, + }, + { + name: "Error/invalid schema", + input: ` +schema: olm.template.invalid +entries: [] +substitutions: [] +`, + expectError: true, + errorMsg: "template has unknown schema", + }, + { + name: "Error/missing schema", + input: ` +entries: [] +substitutions: [] +`, + expectError: true, + errorMsg: "template has unknown schema", + }, + { + name: "Error/invalid YAML", + input: ` +schema: olm.template.substitutes +entries: [ +substitutions: [] +`, + expectError: true, + errorMsg: "decoding template schema", + }, + { + name: "Success/empty template", + input: ` +schema: olm.template.substitutes +entries: [] +substitutions: [] +`, + expected: &SubstitutesForTemplate{ + Schema: "olm.template.substitutes", + Entries: []*declcfg.Meta{}, + Substitutions: []Substitute{}, + }, + expectError: false, + }, + { + name: "Success/multiple substitutions", + input: ` +schema: olm.template.substitutes +entries: + - schema: olm.channel + name: stable + package: testoperator + blob: '{"schema":"olm.channel","name":"stable","package":"testoperator","entries":[{"name":"testoperator.v1.0.0"}]}' +substitutions: + - name: testoperator.v1.1.0 + base: testoperator.v1.0.0 + - name: testoperator.v1.2.0 + base: testoperator.v1.1.0 +`, + expected: &SubstitutesForTemplate{ + Schema: "olm.template.substitutes", + Entries: []*declcfg.Meta{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Blob: json.RawMessage(`{"schema":"olm.channel","name":"stable","package":"testoperator","entries":[{"name":"testoperator.v1.0.0"}]}`), + }, + }, + Substitutions: []Substitute{ + {Name: "testoperator.v1.1.0", Base: "testoperator.v1.0.0"}, + {Name: "testoperator.v1.2.0", Base: "testoperator.v1.1.0"}, + }, + }, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reader := strings.NewReader(tt.input) + result, err := parseSpec(reader) + + if tt.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errorMsg) + require.Nil(t, result) + } else { + require.NoError(t, err) + require.Equal(t, tt.expected.Schema, result.Schema) + require.Equal(t, len(tt.expected.Entries), len(result.Entries)) + require.Equal(t, len(tt.expected.Substitutions), len(result.Substitutions)) + + // Check substitutions + for i, expectedSub := range tt.expected.Substitutions { + require.Equal(t, expectedSub.Name, result.Substitutions[i].Name) + require.Equal(t, expectedSub.Base, result.Substitutions[i].Base) + } + } + }) + } +} + +func TestRender(t *testing.T) { + tests := []struct { + name string + entries []*declcfg.Meta + substitutions []Substitute + expectError bool + errorContains string + validate func(t *testing.T, cfg *declcfg.DeclarativeConfig) + }{ + { + name: "Success/render with single substitution", + entries: []*declcfg.Meta{ + createValidTestPackageMeta("testoperator", "stable"), + createValidTestChannelMeta("stable", "testoperator", []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, // Base bundle must be in channel entries + }), + createValidTestBundleMeta("testoperator-v1.0.0-alpha", "testoperator", "1.0.0", "alpha"), + createValidTestBundleMeta("testoperator-v1.1.0-alpha", "testoperator", "1.1.0", "alpha"), // Base bundle must be defined as bundle + // Substitute.name bundle (testoperator.v1.2.0) must NOT be in template entries + }, + substitutions: []Substitute{ + {Name: "quay.io/test/testoperator-bundle:v1.2.0", Base: "testoperator-v1.1.0-alpha"}, // Use bundle image reference + }, + expectError: false, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + require.Len(t, cfg.Channels, 1) + channel := cfg.Channels[0] + require.Len(t, channel.Entries, 3) // Original 2 + 1 new substitution + + // Find the new substitution entry + var substituteEntry *declcfg.ChannelEntry + for i := range channel.Entries { + if channel.Entries[i].Name == "testoperator-v1.2.0-alpha" { + substituteEntry = &channel.Entries[i] + break + } + } + require.NotNil(t, substituteEntry) + require.Equal(t, "testoperator-v1.0.0-alpha", substituteEntry.Replaces) + require.Contains(t, substituteEntry.Skips, "testoperator-v1.1.0-alpha") + }, + }, + { + name: "Success/render with multiple substitutions", + entries: []*declcfg.Meta{ + createValidTestPackageMeta("testoperator", "stable"), + createValidTestChannelMeta("stable", "testoperator", []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + // Don't include substitution bundles in channel entries initially - they will be added by the substitution process + }), + createValidTestBundleMeta("testoperator-v1.0.0-alpha", "testoperator", "1.0.0", "alpha"), + createValidTestBundleMeta("testoperator-v1.1.0-alpha", "testoperator", "1.1.0", "alpha"), + // Don't include substitution bundles in entries - they will be added by the substitution process + }, + substitutions: []Substitute{ + {Name: "quay.io/test/testoperator-bundle:v1.2.0", Base: "testoperator-v1.1.0-alpha"}, + {Name: "quay.io/test/testoperator-bundle:v1.3.0", Base: "testoperator-v1.2.0-alpha"}, + }, + expectError: false, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + require.Len(t, cfg.Channels, 1) + channel := cfg.Channels[0] + require.Len(t, channel.Entries, 4) // Original 2 + 2 new substitutions + + // Check first substitution (it gets cleared by the second substitution) + var firstSub *declcfg.ChannelEntry + for i := range channel.Entries { + if channel.Entries[i].Name == "testoperator-v1.2.0-alpha" { + firstSub = &channel.Entries[i] + break + } + } + require.NotNil(t, firstSub) + require.Equal(t, "", firstSub.Replaces) // Cleared by second substitution + require.Nil(t, firstSub.Skips) // Cleared by second substitution + + // Check second substitution + var secondSub *declcfg.ChannelEntry + for i := range channel.Entries { + if channel.Entries[i].Name == "testoperator-v1.3.0-alpha" { + secondSub = &channel.Entries[i] + break + } + } + require.NotNil(t, secondSub) + require.Equal(t, "testoperator-v1.0.0-alpha", secondSub.Replaces) + require.Contains(t, secondSub.Skips, "testoperator-v1.2.0-alpha") + }, + }, + { + name: "Success/render with no substitutions", + entries: []*declcfg.Meta{ + createValidTestPackageMeta("testoperator", "stable"), + createValidTestChannelMeta("stable", "testoperator", []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + }), + createValidTestBundleMeta("testoperator-v1.0.0-alpha", "testoperator", "1.0.0", "alpha"), + }, + substitutions: []Substitute{}, + expectError: false, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + require.Len(t, cfg.Channels, 1) + channel := cfg.Channels[0] + require.Len(t, channel.Entries, 1) + require.Equal(t, "testoperator-v1.0.0-alpha", channel.Entries[0].Name) + }, + }, + { + name: "Error/render with substitution that has no matching base", + entries: []*declcfg.Meta{ + createValidTestPackageMeta("testoperator", "stable"), + createValidTestChannelMeta("stable", "testoperator", []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + }), + createValidTestBundleMeta("testoperator-v1.0.0-alpha", "testoperator", "1.0.0", "alpha"), + }, + substitutions: []Substitute{ + {Name: "quay.io/test/testoperator-bundle:v1.2.0", Base: "nonexistent-v1.0.0-alpha"}, + }, + expectError: true, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + require.Len(t, cfg.Channels, 1) + channel := cfg.Channels[0] + require.Len(t, channel.Entries, 1) // No new entries added + require.Equal(t, "testoperator-v1.0.0-alpha", channel.Entries[0].Name) + }, + }, + { + name: "Error/render with invalid substitution (empty name)", + entries: []*declcfg.Meta{ + createValidTestPackageMeta("testoperator", "stable"), + createValidTestChannelMeta("stable", "testoperator", []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + }), + createValidTestBundleMeta("testoperator-v1.0.0-alpha", "testoperator", "1.0.0", "alpha"), + createValidTestBundleMeta("testoperator-v1.1.0-alpha", "testoperator", "1.1.0", "alpha"), + }, + substitutions: []Substitute{ + {Name: "", Base: "testoperator-v1.1.0-alpha"}, // Invalid: empty name + }, + expectError: true, + errorContains: "substitution name cannot be empty", + }, + { + name: "Error/render with invalid substitution (empty base)", + entries: []*declcfg.Meta{ + createValidTestPackageMeta("testoperator", "stable"), + createValidTestChannelMeta("stable", "testoperator", []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + }), + createValidTestBundleMeta("testoperator-v1.0.0-alpha", "testoperator", "1.0.0", "alpha"), + createValidTestBundleMeta("testoperator-v1.1.0-alpha", "testoperator", "1.1.0", "alpha"), + }, + substitutions: []Substitute{ + {Name: "testoperator-v1.2.0-alpha", Base: ""}, // Invalid: empty base + }, + expectError: true, + errorContains: "substitution base cannot be empty", + }, + { + name: "Error/render with invalid substitution (same name and base)", + entries: []*declcfg.Meta{ + createValidTestPackageMeta("testoperator", "stable"), + createValidTestChannelMeta("stable", "testoperator", []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + }), + createValidTestBundleMeta("testoperator-v1.0.0-alpha", "testoperator", "1.0.0", "alpha"), + createValidTestBundleMeta("testoperator-v1.1.0-alpha", "testoperator", "1.1.0", "alpha"), + }, + substitutions: []Substitute{ + {Name: "testoperator-v1.1.0-alpha", Base: "testoperator-v1.1.0-alpha"}, // Invalid: same name and base + }, + expectError: true, + errorContains: "substitution name and base cannot be the same", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create template with test data + template := SubstitutesForTemplate{ + Schema: "olm.template.substitutes", + Entries: tt.entries, + Substitutions: tt.substitutions, + } + + // Convert to JSON and create reader + templateJSON, err := json.Marshal(template) + require.NoError(t, err) + + reader := strings.NewReader(string(templateJSON)) + templateInstance := Template{ + RenderBundle: func(ctx context.Context, imageRef string) (*declcfg.DeclarativeConfig, error) { + // Mock implementation that creates a bundle from the image reference + // Extract version from image reference (simplified for testing) + version := "1.2.0" + if strings.Contains(imageRef, ":v") { + parts := strings.Split(imageRef, ":v") + if len(parts) == 2 { + version = parts[1] + } + } + + // Create bundle name based on version for predictable naming + bundleName := "testoperator-v" + version + "-alpha" + + return &declcfg.DeclarativeConfig{ + Bundles: []declcfg.Bundle{ + { + Schema: "olm.bundle", + Name: bundleName, + Package: "testoperator", + Image: imageRef, + Properties: []property.Property{ + property.MustBuildPackage("testoperator", version), + property.MustBuildBundleObject([]byte(fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, imageRef))), + property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)), + }, + RelatedImages: []declcfg.RelatedImage{ + {Name: "bundle", Image: imageRef}, + }, + CsvJSON: fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, imageRef), + Objects: []string{ + fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, imageRef), + `{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`, + }, + }, + }, + }, nil + }, + } + ctx := context.Background() + + result, err := templateInstance.Render(ctx, reader) + + if tt.expectError { + require.Error(t, err) + if tt.errorContains != "" { + require.Contains(t, err.Error(), tt.errorContains) + } + require.Nil(t, result) + } else { + require.NoError(t, err) + require.NotNil(t, result) + if tt.validate != nil { + tt.validate(t, result) + } + } + }) + } +} + +func TestProcessSubstitution(t *testing.T) { + tests := []struct { + name string + cfg *declcfg.DeclarativeConfig + substitution Substitute + validate func(t *testing.T, cfg *declcfg.DeclarativeConfig) + }{ + { + name: "Success/substitution with replaces relationship", + cfg: &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "testoperator", + DefaultChannel: "stable", + Description: "testoperator operator", + }, + }, + Channels: []declcfg.Channel{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Entries: []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + }, + }, + }, + Bundles: []declcfg.Bundle{ + { + Schema: "olm.bundle", + Name: "testoperator-v1.0.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.0.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.0.0", "alpha"), + property.MustBuildBundleObject([]byte(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.0.0-alpha"}}`)), + property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)), + }, + RelatedImages: []declcfg.RelatedImage{ + {Name: "bundle", Image: "quay.io/test/testoperator-bundle:v1.0.0"}, + }, + CsvJSON: `{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.0.0-alpha"}}`, + Objects: []string{ + `{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.0.0-alpha"}}`, + `{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`, + }, + }, + { + Schema: "olm.bundle", + Name: "testoperator-v1.1.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.1.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.1.0", "alpha"), + property.MustBuildBundleObject([]byte(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.1.0-alpha"}}`)), + property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)), + }, + RelatedImages: []declcfg.RelatedImage{ + {Name: "bundle", Image: "quay.io/test/testoperator-bundle:v1.1.0"}, + }, + CsvJSON: `{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.1.0-alpha"}}`, + Objects: []string{ + `{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.1.0-alpha"}}`, + `{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`, + }, + }, + }, + }, + substitution: Substitute{Name: "testoperator-v1.2.0-alpha", Base: "testoperator-v1.1.0-alpha"}, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + channel := cfg.Channels[0] + require.Len(t, channel.Entries, 3) + + // Find the new substitution entry + var substituteEntry *declcfg.ChannelEntry + for i := range channel.Entries { + if channel.Entries[i].Name == "testoperator-v1.2.0-alpha" { + substituteEntry = &channel.Entries[i] + break + } + } + require.NotNil(t, substituteEntry) + require.Equal(t, "testoperator-v1.0.0-alpha", substituteEntry.Replaces) + require.Contains(t, substituteEntry.Skips, "testoperator-v1.1.0-alpha") + + // Check that original entry was cleared + var originalEntry *declcfg.ChannelEntry + for i := range channel.Entries { + if channel.Entries[i].Name == "testoperator-v1.1.0-alpha" { + originalEntry = &channel.Entries[i] + break + } + } + require.NotNil(t, originalEntry) + require.Empty(t, originalEntry.Replaces) + require.Empty(t, originalEntry.Skips) + require.Empty(t, originalEntry.SkipRange) + }, + }, + { + name: "Success/substitution with skips and skipRange", + cfg: &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "testoperator", + DefaultChannel: "stable", + Description: "testoperator operator", + }, + }, + Channels: []declcfg.Channel{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Entries: []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha", Skips: []string{"testoperator-v0.9.0-alpha"}, SkipRange: ">=0.9.0 <1.1.0"}, + }, + }, + }, + Bundles: []declcfg.Bundle{ + { + Schema: "olm.bundle", + Name: "testoperator-v1.0.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.0.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.0.0", "alpha"), + property.MustBuildBundleObject([]byte(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.0.0-alpha"}}`)), + property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)), + }, + RelatedImages: []declcfg.RelatedImage{ + {Name: "bundle", Image: "quay.io/test/testoperator-bundle:v1.0.0"}, + }, + CsvJSON: `{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.0.0-alpha"}}`, + Objects: []string{ + `{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.0.0-alpha"}}`, + `{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`, + }, + }, + { + Schema: "olm.bundle", + Name: "testoperator-v1.1.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.1.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.1.0", "alpha"), + property.MustBuildBundleObject([]byte(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.1.0-alpha"}}`)), + property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)), + }, + RelatedImages: []declcfg.RelatedImage{ + {Name: "bundle", Image: "quay.io/test/testoperator-bundle:v1.1.0"}, + }, + CsvJSON: `{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.1.0-alpha"}}`, + Objects: []string{ + `{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":"testoperator-v1.1.0-alpha"}}`, + `{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`, + }, + }, + }, + }, + substitution: Substitute{Name: "testoperator-v1.2.0-alpha", Base: "testoperator-v1.1.0-alpha"}, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + channel := cfg.Channels[0] + require.Len(t, channel.Entries, 3) + + // Find the new substitution entry + var substituteEntry *declcfg.ChannelEntry + for i := range channel.Entries { + if channel.Entries[i].Name == "testoperator-v1.2.0-alpha" { + substituteEntry = &channel.Entries[i] + break + } + } + require.NotNil(t, substituteEntry) + require.Equal(t, "testoperator-v1.0.0-alpha", substituteEntry.Replaces) + require.Contains(t, substituteEntry.Skips, "testoperator-v0.9.0-alpha") + require.Contains(t, substituteEntry.Skips, "testoperator-v1.1.0-alpha") + require.Equal(t, ">=0.9.0 <1.1.0", substituteEntry.SkipRange) + }, + }, + { + name: "Error/substitution with no matching base", + cfg: &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "testoperator", + DefaultChannel: "stable", + }, + }, + Channels: []declcfg.Channel{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Entries: []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + }, + }, + }, + Bundles: []declcfg.Bundle{ + { + Schema: "olm.bundle", + Name: "testoperator-v1.0.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.0.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.0.0", "alpha"), + }, + }, + }, + }, + substitution: Substitute{Name: "quay.io/test/testoperator-bundle:v1.2.0", Base: "nonexistent.v1.0.0"}, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + // This test should fail, so this validation should not be called + t.Fatal("This test should have failed") + }, + }, + { + name: "Success/substitution with multiple channels", + cfg: &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "testoperator", + DefaultChannel: "stable", + }, + }, + Channels: []declcfg.Channel{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Entries: []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + }, + }, + { + Schema: "olm.channel", + Name: "beta", + Package: "testoperator", + Entries: []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + }, + }, + }, + Bundles: []declcfg.Bundle{ + { + Schema: "olm.bundle", + Name: "testoperator-v1.0.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.0.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.0.0", "alpha"), + }, + }, + { + Schema: "olm.bundle", + Name: "testoperator-v1.1.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.1.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.1.0", "alpha"), + }, + }, + }, + }, + substitution: Substitute{Name: "quay.io/test/testoperator-bundle:v1.2.0", Base: "testoperator-v1.1.0-alpha"}, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + require.Len(t, cfg.Channels, 2) + + // Check stable channel + stableChannel := cfg.Channels[0] + require.Len(t, stableChannel.Entries, 3) + + // Check beta channel + betaChannel := cfg.Channels[1] + require.Len(t, betaChannel.Entries, 3) + + // Both channels should have the substitution + for _, channel := range cfg.Channels { + var substituteEntry *declcfg.ChannelEntry + for i := range channel.Entries { + if channel.Entries[i].Name == "testoperator-v1.2.0-alpha" { + substituteEntry = &channel.Entries[i] + break + } + } + require.NotNil(t, substituteEntry) + require.Equal(t, "testoperator-v1.0.0-alpha", substituteEntry.Replaces) + require.Contains(t, substituteEntry.Skips, "testoperator-v1.1.0-alpha") + } + }, + }, + { + name: "Success/substitution updates existing references", + cfg: &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "testoperator", + DefaultChannel: "stable", + }, + }, + Channels: []declcfg.Channel{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Entries: []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.1.0-alpha", Replaces: "testoperator-v1.0.0-alpha"}, + {Name: "testoperator-v1.2.0-alpha", Replaces: "testoperator-v1.1.0-alpha"}, + }, + }, + }, + Bundles: []declcfg.Bundle{ + { + Schema: "olm.bundle", + Name: "testoperator-v1.0.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.0.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.0.0", "alpha"), + }, + }, + { + Schema: "olm.bundle", + Name: "testoperator-v1.1.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.1.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.1.0", "alpha"), + }, + }, + { + Schema: "olm.bundle", + Name: "testoperator-v1.2.0-alpha", + Package: "testoperator", + Image: "quay.io/test/testoperator-bundle:v1.2.0", + Properties: []property.Property{ + property.MustBuildPackageRelease("testoperator", "1.2.0", "alpha"), + }, + }, + }, + }, + substitution: Substitute{Name: "quay.io/test/testoperator-bundle:v1.1.5", Base: "testoperator-v1.1.0-alpha"}, + validate: func(t *testing.T, cfg *declcfg.DeclarativeConfig) { + channel := cfg.Channels[0] + require.Len(t, channel.Entries, 4) // Original 3 + 1 new substitution + + // Find the entry that originally replaced testoperator-v1.1.0-alpha + var updatedEntry *declcfg.ChannelEntry + for i := range channel.Entries { + if channel.Entries[i].Name == "testoperator-v1.2.0-alpha" { + updatedEntry = &channel.Entries[i] + break + } + } + require.NotNil(t, updatedEntry) + require.Equal(t, "testoperator-v1.1.5-alpha", updatedEntry.Replaces) // Should now reference the substitute + require.Contains(t, updatedEntry.Skips, "testoperator-v1.1.0-alpha") // Should skip the original base + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, tt.cfg, tt.substitution) + if strings.Contains(tt.name, "Error/") { + require.Error(t, err) + } else { + require.NoError(t, err) + tt.validate(t, tt.cfg) + } + }) + } +} + +func TestBoundaryCases(t *testing.T) { + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Error/empty DeclarativeConfig", + testFunc: func(t *testing.T) { + cfg := &declcfg.DeclarativeConfig{} + substitution := Substitute{Name: "quay.io/test/test-bundle:v1.0.0", Base: "test.v0.9.0"} + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, cfg, substitution) + require.Error(t, err) + require.Contains(t, err.Error(), "unknown package") + }, + }, + { + name: "Error/DeclarativeConfig with empty channels", + testFunc: func(t *testing.T) { + cfg := &declcfg.DeclarativeConfig{ + Channels: []declcfg.Channel{}, + } + substitution := Substitute{Name: "quay.io/test/test-bundle:v1.0.0", Base: "test.v0.9.0"} + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, cfg, substitution) + require.Error(t, err) + require.Contains(t, err.Error(), "unknown package") + }, + }, + { + name: "Error/channel with empty entries", + testFunc: func(t *testing.T) { + cfg := &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "testoperator", + DefaultChannel: "stable", + }, + }, + Channels: []declcfg.Channel{ + { + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Entries: []declcfg.ChannelEntry{}, + }, + }, + } + substitution := Substitute{Name: "quay.io/test/test-bundle:v1.0.0", Base: "test.v0.9.0"} + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, cfg, substitution) + require.Error(t, err) + require.Contains(t, err.Error(), "unknown package") + }, + }, + { + name: "Error/substitution with empty name", + testFunc: func(t *testing.T) { + cfg := createTestDeclarativeConfig() + substitution := Substitute{Name: "", Base: "testoperator.v1.1.0"} + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, cfg, substitution) + require.Error(t, err) + require.Contains(t, err.Error(), "substitution name cannot be empty") + // Should not create any new entries with empty name + require.Len(t, cfg.Channels[0].Entries, 3) // Original entries unchanged + }, + }, + { + name: "Error/substitution with empty base", + testFunc: func(t *testing.T) { + cfg := createTestDeclarativeConfig() + substitution := Substitute{Name: "quay.io/test/testoperator-bundle:v1.2.0", Base: ""} + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, cfg, substitution) + require.Error(t, err) + require.Contains(t, err.Error(), "substitution base cannot be empty") + // Should not create any new entries with empty base + require.Len(t, cfg.Channels[0].Entries, 3) // Original entries unchanged + }, + }, + { + name: "Error/substitution with same name and base", + testFunc: func(t *testing.T) { + cfg := createTestDeclarativeConfig() + substitution := Substitute{Name: "quay.io/test/testoperator-bundle:v1.1.0", Base: "quay.io/test/testoperator-bundle:v1.1.0"} + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, cfg, substitution) + require.Error(t, err) + require.Contains(t, err.Error(), "substitution name and base cannot be the same") + // Should not create any new entries when name equals base + require.Len(t, cfg.Channels[0].Entries, 3) // Original entries unchanged + }, + }, + { + name: "Error/template with malformed JSON in blob", + testFunc: func(t *testing.T) { + // Create a template with invalid JSON in the blob + invalidMeta := &declcfg.Meta{ + Schema: "olm.channel", + Name: "stable", + Package: "testoperator", + Blob: json.RawMessage(`{"invalid": json, "missing": quote}`), + } + + template := SubstitutesForTemplate{ + Schema: "olm.template.substitutes", + Entries: []*declcfg.Meta{invalidMeta}, + Substitutions: []Substitute{}, + } + + _, err := json.Marshal(template) + // The malformed JSON should cause an error during marshaling + require.Error(t, err) + require.Contains(t, err.Error(), "invalid character") + }, + }, + { + name: "Success/template with nil context", + testFunc: func(t *testing.T) { + entries := []*declcfg.Meta{ + createValidTestPackageMeta("testoperator", "stable"), + createValidTestChannelMeta("stable", "testoperator", []declcfg.ChannelEntry{ + {Name: "testoperator-v1.0.0-alpha"}, + }), + createValidTestBundleMeta("testoperator-v1.0.0-alpha", "testoperator", "1.0.0", "alpha"), + } + + template := SubstitutesForTemplate{ + Schema: "olm.template.substitutes", + Entries: entries, + Substitutions: []Substitute{}, + } + + templateJSON, err := json.Marshal(template) + require.NoError(t, err) + + reader := strings.NewReader(string(templateJSON)) + templateInstance := Template{} + + result, err := templateInstance.Render(context.TODO(), reader) + require.NoError(t, err) // Context is not used in current implementation + require.NotNil(t, result) + }, + }, + { + name: "Error/substitution with invalid declarative config - missing package", + testFunc: func(t *testing.T) { + // Create a config with a bundle that references a non-existent package + cfg := &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "nonexistent", + DefaultChannel: "stable", + }, + }, + Bundles: []declcfg.Bundle{ + { + Name: "testoperator.v1.1.0", // This is the substitution name we're testing + Package: "nonexistent", // This package exists but bundle name doesn't match + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"packageName":"nonexistent","version":"1.1.0"}`), + }, + }, + }, + }, + } + substitution := Substitute{Name: "quay.io/test/testoperator-bundle:v1.1.0", Base: "testoperator.v1.0.0"} + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, cfg, substitution) + require.Error(t, err) + require.Contains(t, err.Error(), "not found in any channel entries") + }, + }, + { + name: "Error/substitution with invalid declarative config - bundle missing olm.package property", + testFunc: func(t *testing.T) { + // Create a config with a bundle that has no olm.package property + cfg := &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{ + { + Schema: "olm.package", + Name: "testoperator", + DefaultChannel: "stable", + }, + }, + Bundles: []declcfg.Bundle{ + { + Name: "testoperator.v1.1.0", // This is the substitution name we're testing + Package: "testoperator", + Properties: []property.Property{}, // No olm.package property + }, + }, + } + substitution := Substitute{Name: "quay.io/test/testoperator-bundle:v1.1.0", Base: "testoperator.v1.0.0"} + template := createMockTemplate() + ctx := context.Background() + err := template.processSubstitution(ctx, cfg, substitution) + require.Error(t, err) + require.Contains(t, err.Error(), "must have exactly 1 \"olm.package\" property") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.testFunc(t) + }) + } +} From c74666469fc5ab28e2c87d9ec348c5eb8f71e805 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Wed, 22 Oct 2025 14:38:59 -0500 Subject: [PATCH 8/8] added substitutes template generation from FBC Signed-off-by: grokspawn --- alpha/template/converter/converter.go | 26 ++++++++++---- alpha/template/substitutes/substitutes.go | 23 ++++++++++++ cmd/opm/alpha/convert-template/convert.go | 44 +++++++++++++++++++++++ 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/alpha/template/converter/converter.go b/alpha/template/converter/converter.go index 03e3e0a97..c68f47696 100644 --- a/alpha/template/converter/converter.go +++ b/alpha/template/converter/converter.go @@ -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 { diff --git a/alpha/template/substitutes/substitutes.go b/alpha/template/substitutes/substitutes.go index 82522752a..cd1553cbc 100644 --- a/alpha/template/substitutes/substitutes.go +++ b/alpha/template/substitutes/substitutes.go @@ -158,3 +158,26 @@ func (t Template) processSubstitution(ctx context.Context, cfg *declcfg.Declarat return nil } + +// FromReader reads FBC from a reader and generates a BasicTemplate from it +func FromReader(r io.Reader) (*SubstitutesForTemplate, error) { + var entries []*declcfg.Meta + if err := declcfg.WalkMetasReader(r, func(meta *declcfg.Meta, err error) error { + if err != nil { + return err + } + + entries = append(entries, meta) + return nil + }); err != nil { + return nil, err + } + + bt := &SubstitutesForTemplate{ + Schema: schema, + Entries: entries, + Substitutions: []Substitute{}, + } + + return bt, nil +} diff --git a/cmd/opm/alpha/convert-template/convert.go b/cmd/opm/alpha/convert-template/convert.go index a5587c004..4b5567e9c 100644 --- a/cmd/opm/alpha/convert-template/convert.go +++ b/cmd/opm/alpha/convert-template/convert.go @@ -17,6 +17,7 @@ func NewCmd() *cobra.Command { } cmd.AddCommand( newBasicConvertCmd(), + newSubstitutesConvertCmd(), ) return cmd } @@ -49,6 +50,49 @@ If no argument is specified or is '-' input is assumed from STDIN. } converter.FbcReader = reader + converter.DestinationTemplateType = "basic" + err = converter.Convert() + if err != nil { + return fmt.Errorf("converting: %v", err) + } + + return nil + }, + } + cmd.Flags().StringVarP(&output, "output", "o", "json", "Output format (json|yaml)") + + return cmd +} + +func newSubstitutesConvertCmd() *cobra.Command { + var ( + converter converter.Converter + output string + ) + cmd := &cobra.Command{ + Use: "substitutes [ | -]", + Args: cobra.MaximumNArgs(1), + Short: "Generate a substitutes template from existing FBC", + Long: `Generate a substitutes template from existing FBC. + +This command outputs a substitutes catalog template to STDOUT from input FBC. +If no argument is specified or is '-' input is assumed from STDIN. +`, + RunE: func(c *cobra.Command, args []string) error { + switch output { + case "yaml", "json": + converter.OutputFormat = output + default: + log.Fatalf("invalid --output value %q, expected (json|yaml)", output) + } + + reader, name, err := util.OpenFileOrStdin(c, args) + if err != nil { + return fmt.Errorf("unable to open input: %q", name) + } + + converter.FbcReader = reader + converter.DestinationTemplateType = "substitutes" err = converter.Convert() if err != nil { return fmt.Errorf("converting: %v", err)