use claude to resolve unit tests after updates

Signed-off-by: grokspawn <jordan@nimblewidget.com>

Author: grokspawn

alpha/declcfg/declcfg.go

@@ -10,6 +10,7 @@ import (
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 
+	"github.com/blang/semver/v4"
 	"github.com/operator-framework/operator-registry/alpha/property"
 	prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler"
 )
@@ -206,3 +207,71 @@ func (destination *DeclarativeConfig) Merge(src *DeclarativeConfig) {
 	destination.Others = append(destination.Others, src.Others...)
 	destination.Deprecations = append(destination.Deprecations, src.Deprecations...)
 }
+
+type CompositeVersion struct {
+	Version semver.Version
+	Release semver.Version
+}
+
+func (cv *CompositeVersion) Compare(other *CompositeVersion) int {
+	if cv.Version.NE(other.Version) {
+		return cv.Version.Compare(other.Version)
+	}
+	hasrelease := len(cv.Release.Pre) > 0
+	otherhasrelease := len(other.Release.Pre) > 0
+	if hasrelease && !otherhasrelease {
+		return 1
+	}
+	if !hasrelease && otherhasrelease {
+		return -1
+	}
+	return cv.Release.Compare(other.Release)
+}
+
+// order by version, then
+// release, if present
+func (b *Bundle) Compare(other *Bundle) int {
+	if b.Name == other.Name {
+		return 0
+	}
+	acv, err := b.CompositeVersion()
+	if err != nil {
+		return 0
+	}
+	otherCv, err := other.CompositeVersion()
+	if err != nil {
+		return 0
+	}
+	return acv.Compare(otherCv)
+}
+
+func (b *Bundle) CompositeVersion() (*CompositeVersion, error) {
+	props, err := property.Parse(b.Properties)
+	if err != nil {
+		return nil, fmt.Errorf("parse properties for bundle %q: %v", b.Name, err)
+	}
+	if len(props.Packages) != 1 {
+		return nil, fmt.Errorf("bundle %q must have exactly 1 \"olm.package\" property, found %v", b.Name, len(props.Packages))
+	}
+	v, err := semver.Parse(props.Packages[0].Version)
+	if err != nil {
+		return nil, fmt.Errorf("bundle %q has invalid version %q: %v", b.Name, props.Packages[0].Version, err)
+	}
+
+	var r semver.Version
+	if props.Packages[0].Release != "" {
+		r, 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)
+		}
+		// only need to check for build metadata since we are using explicit zero major, minor, and patch versions above
+		if len(r.Build) != 0 {
+			return nil, fmt.Errorf("bundle %q release version %q cannot contain build metadata", b.Name, props.Packages[0].Release)
+		}
+	}
+
+	return &CompositeVersion{
+		Version: v,
+		Release: r,
+	}, nil
+}

alpha/model/model.go

@@ -366,6 +366,14 @@ func (b *Bundle) Compare(other *Bundle) int {
 	if b.Version.NE(other.Version) {
 		return b.Version.Compare(other.Version)
 	}
+	bhasrelease := len(b.Release.Pre) > 0
+	otherhasrelease := len(other.Release.Pre) > 0
+	if bhasrelease && !otherhasrelease {
+		return 1
+	}
+	if !bhasrelease && otherhasrelease {
+		return -1
+	}
 	return b.Release.Compare(other.Release)
 }
 

alpha/template/substitutes/substitutes.go

@@ -106,8 +106,8 @@ func parseSpec(reader io.Reader) (*SubstitutesTemplateData, error) {
 	return st, nil
 }
 
-// processSubstitution handles the complex logic for processing a single substitution
-func (t *template) processSubstitution(ctx context.Context, cfg *declcfg.DeclarativeConfig, substitution Substitute) error {
+// validateSubstitution validates the substitution references
+func (t *template) validateSubstitution(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")
@@ -119,6 +119,53 @@ func (t *template) processSubstitution(ctx context.Context, cfg *declcfg.Declara
 		return fmt.Errorf("substitution name and base cannot be the same")
 	}
 
+	// determine the versions of the base and substitute bundles and ensure that
+	// the composite version of the substitute bundle is greater than the composite version of the base bundle
+
+	// 1. Render the pullspec represented by substitution.Name
+	substituteCfg, err := t.renderBundle(ctx, substitution.Name)
+	if err != nil {
+		return fmt.Errorf("failed to render bundle image reference %q: %v", substitution.Name, err)
+	}
+	if substituteCfg == nil || len(substituteCfg.Bundles) == 0 {
+		return fmt.Errorf("rendered bundle image reference %q contains no bundles", substitution.Name)
+	}
+	substituteBundle := &substituteCfg.Bundles[0]
+	substituteCv, err := substituteBundle.CompositeVersion()
+	if err != nil {
+		return fmt.Errorf("failed to get composite version for substitute bundle %q: %v", substitution.Name, err)
+	}
+
+	// 2. Examine cfg to find the bundle which has matching name to substitution.Base
+	var baseBundle *declcfg.Bundle
+	for i := range cfg.Bundles {
+		if cfg.Bundles[i].Name == substitution.Base {
+			baseBundle = &cfg.Bundles[i]
+			break
+		}
+	}
+	if baseBundle == nil {
+		return fmt.Errorf("base bundle %q does not exist in catalog", substitution.Base)
+	}
+	baseCv, err := baseBundle.CompositeVersion()
+	if err != nil {
+		return fmt.Errorf("failed to get composite version for base bundle %q: %v", substitution.Base, err)
+	}
+
+	// 3. Ensure that the base bundle composite version is less than the substitute bundle composite version
+	if baseCv.Compare(substituteCv) >= 0 {
+		return fmt.Errorf("base bundle %q is not less than substitute bundle %q", substitution.Base, substitution.Name)
+	}
+
+	return nil
+}
+
+// processSubstitution handles the complex logic for processing a single substitution
+func (t *template) processSubstitution(ctx context.Context, cfg *declcfg.DeclarativeConfig, substitution Substitute) error {
+	if err := t.validateSubstitution(ctx, cfg, substitution); err != nil {
+		return err
+	}
+
 	substituteCfg, err := t.RenderBundle(ctx, substitution.Name)
 	if err != nil {
 		return fmt.Errorf("failed to render bundle image reference %q: %v", substitution.Name, err)

alpha/template/substitutes/substitutes_test.go

@@ -1066,7 +1066,19 @@ func TestBoundaryCases(t *testing.T) {
 		{
 			name: "Error/empty DeclarativeConfig",
 			testFunc: func(t *testing.T) {
-				cfg := &declcfg.DeclarativeConfig{}
+				cfg := &declcfg.DeclarativeConfig{
+					Bundles: []declcfg.Bundle{
+						{
+							Schema:  "olm.bundle",
+							Name:    "test.v0.9.0",
+							Package: "test",
+							Image:   "quay.io/test/test-bundle:v0.9.0",
+							Properties: []property.Property{
+								property.MustBuildPackage("test", "0.9.0"),
+							},
+						},
+					},
+				}
 				substitution := Substitute{Name: "quay.io/test/test-bundle:v1.0.0-alpha", Base: "test.v0.9.0"}
 				template := createMockTemplate()
 				ctx := context.Background()
@@ -1080,6 +1092,17 @@ func TestBoundaryCases(t *testing.T) {
 			testFunc: func(t *testing.T) {
 				cfg := &declcfg.DeclarativeConfig{
 					Channels: []declcfg.Channel{},
+					Bundles: []declcfg.Bundle{
+						{
+							Schema:  "olm.bundle",
+							Name:    "test.v0.9.0",
+							Package: "test",
+							Image:   "quay.io/test/test-bundle:v0.9.0",
+							Properties: []property.Property{
+								property.MustBuildPackage("test", "0.9.0"),
+							},
+						},
+					},
 				}
 				substitution := Substitute{Name: "quay.io/test/test-bundle:v1.0.0-alpha", Base: "test.v0.9.0"}
 				template := createMockTemplate()
@@ -1108,6 +1131,17 @@ func TestBoundaryCases(t *testing.T) {
 							Entries: []declcfg.ChannelEntry{},
 						},
 					},
+					Bundles: []declcfg.Bundle{
+						{
+							Schema:  "olm.bundle",
+							Name:    "test.v0.9.0",
+							Package: "test",
+							Image:   "quay.io/test/test-bundle:v0.9.0",
+							Properties: []property.Property{
+								property.MustBuildPackage("test", "0.9.0"),
+							},
+						},
+					},
 				}
 				substitution := Substitute{Name: "quay.io/test/test-bundle:v1.0.0-alpha", Base: "test.v0.9.0"}
 				template := createMockTemplate()
@@ -1213,24 +1247,38 @@ func TestBoundaryCases(t *testing.T) {
 		{
 			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
+				// Create a config with a bundle in the catalog that is not referenced in any channels
 				cfg := &declcfg.DeclarativeConfig{
 					Packages: []declcfg.Package{
 						{
 							Schema:         "olm.package",
-							Name:           "nonexistent",
+							Name:           "testoperator",
 							DefaultChannel: "stable",
 						},
 					},
+					Channels: []declcfg.Channel{
+						{
+							Schema:  "olm.channel",
+							Name:    "stable",
+							Package: "testoperator",
+							Entries: []declcfg.ChannelEntry{
+								{Name: "testoperator.v1.0.0"},
+							},
+						},
+					},
 					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
+							Name:    "testoperator.v1.0.0", // Base bundle for substitution
+							Package: "testoperator",
 							Properties: []property.Property{
-								{
-									Type:  property.TypePackage,
-									Value: json.RawMessage(`{"packageName":"nonexistent","version":"1.1.0"}`),
-								},
+								property.MustBuildPackage("testoperator", "1.0.0"),
+							},
+						},
+						{
+							Name:    "testoperator.v1.1.0", // Extra bundle not in any channel
+							Package: "testoperator",
+							Properties: []property.Property{
+								property.MustBuildPackage("testoperator", "1.1.0"),
 							},
 						},
 					},
@@ -1246,7 +1294,7 @@ func TestBoundaryCases(t *testing.T) {
 		{
 			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
+				// Create a config where the base bundle has no olm.package property
 				cfg := &declcfg.DeclarativeConfig{
 					Packages: []declcfg.Package{
 						{
@@ -1257,7 +1305,7 @@ func TestBoundaryCases(t *testing.T) {
 					},
 					Bundles: []declcfg.Bundle{
 						{
-							Name:       "testoperator.v1.1.0", // This is the substitution name we're testing
+							Name:       "testoperator.v1.0.0", // Base bundle for substitution, missing olm.package property
 							Package:    "testoperator",
 							Properties: []property.Property{}, // No olm.package property
 						},
@@ -1271,6 +1319,65 @@ func TestBoundaryCases(t *testing.T) {
 				require.Contains(t, err.Error(), "must have exactly 1 \"olm.package\" property")
 			},
 		},
+		{
+			name: "Error/base bundle does not exist in catalog",
+			testFunc: func(t *testing.T) {
+				cfg := &declcfg.DeclarativeConfig{
+					Packages: []declcfg.Package{
+						{
+							Schema:         "olm.package",
+							Name:           "testoperator",
+							DefaultChannel: "stable",
+						},
+					},
+					Bundles: []declcfg.Bundle{
+						{
+							Name:    "testoperator.v1.1.0",
+							Package: "testoperator",
+							Properties: []property.Property{
+								property.MustBuildPackage("testoperator", "1.1.0"),
+							},
+						},
+					},
+				}
+				substitution := Substitute{Name: "quay.io/test/testoperator-bundle:v1.2.0-alpha", 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(), "does not exist in catalog")
+			},
+		},
+		{
+			name: "Error/substitute bundle version not greater than base bundle",
+			testFunc: func(t *testing.T) {
+				cfg := &declcfg.DeclarativeConfig{
+					Packages: []declcfg.Package{
+						{
+							Schema:         "olm.package",
+							Name:           "testoperator",
+							DefaultChannel: "stable",
+						},
+					},
+					Bundles: []declcfg.Bundle{
+						{
+							Name:    "testoperator-v1.2.0-beta",
+							Package: "testoperator",
+							Properties: []property.Property{
+								property.MustBuildPackageRelease("testoperator", "1.2.0", "beta"),
+							},
+						},
+					},
+				}
+				// Trying to substitute with an alpha release (lower than beta)
+				substitution := Substitute{Name: "quay.io/test/testoperator-bundle:v1.2.0-alpha", Base: "testoperator-v1.2.0-beta"}
+				template := createMockTemplate()
+				ctx := context.Background()
+				err := template.processSubstitution(ctx, cfg, substitution)
+				require.Error(t, err)
+				require.Contains(t, err.Error(), "is not less than")
+			},
+		},
 	}
 
 	for _, tt := range tests {