diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index 3687c04e5..7f73ec495 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -5,18 +5,22 @@ import ( "fmt" "sort" + mmsemver "github.com/Masterminds/semver/v3" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" "github.com/operator-framework/operator-controller/internal/resolution/variables" + "github.com/operator-framework/operator-controller/pkg/features" ) var _ input.VariableSource = &InstalledPackageVariableSource{} type InstalledPackageVariableSource struct { catalogClient BundleProvider + successors successorsFunc bundleImage string } @@ -40,13 +44,10 @@ func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context) ([]de }) installedBundle := resultSet[0] - // now find the bundles that replace the installed bundle - // TODO: this algorithm does not yet consider skips and skipRange - // we simplify the process here by just searching for the bundle that replaces the installed bundle - upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.Replaces(installedBundle.Name)) - sort.SliceStable(upgradeEdges, func(i, j int) bool { - return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j]) - }) + upgradeEdges, err := r.successors(allBundles, installedBundle) + if err != nil { + return nil, err + } // you can always upgrade to yourself, i.e. not upgrade upgradeEdges = append(upgradeEdges, installedBundle) @@ -60,8 +61,57 @@ func (r *InstalledPackageVariableSource) notFoundError() error { } func NewInstalledPackageVariableSource(catalogClient BundleProvider, bundleImage string) (*InstalledPackageVariableSource, error) { + successors := legacySemanticsSuccessors + if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) { + successors = semverSuccessors + } + return &InstalledPackageVariableSource{ catalogClient: catalogClient, bundleImage: bundleImage, + successors: successors, }, nil } + +// successorsFunc must return successors of a currently installed bundle +// from a list of all bundles provided to the function. +// Must not return installed bundle as a successor +type successorsFunc func(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) + +// legacySemanticsSuccessors returns successors based on legacy OLMv0 semantics +// which rely on Replaces, Skips and skipRange. +func legacySemanticsSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { + // find the bundles that replace the bundle provided + // TODO: this algorithm does not yet consider skips and skipRange + upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.Replaces(installedBundle.Name)) + sort.SliceStable(upgradeEdges, func(i, j int) bool { + return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j]) + }) + + return upgradeEdges, nil +} + +// semverSuccessors returns successors based on Semver. +// Successors will not include versions outside the major version of the +// installed bundle as major version is intended to indicate breaking changes. +func semverSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { + currentVersion, err := installedBundle.Version() + if err != nil { + return nil, err + } + + // Based on current version create a caret range comparison constraint + // to allow only major and patch version as successors and exclude current version. + constraintStr := fmt.Sprintf("^%s, != %s", currentVersion.String(), currentVersion.String()) + wantedVersionRangeConstraint, err := mmsemver.NewConstraint(constraintStr) + if err != nil { + return nil, err + } + + upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.InMastermindsSemverRange(wantedVersionRangeConstraint)) + sort.SliceStable(upgradeEdges, func(i, j int) bool { + return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j]) + }) + + return upgradeEdges, nil +} diff --git a/internal/resolution/variablesources/installed_package_test.go b/internal/resolution/variablesources/installed_package_test.go index 1f435f908..5fb68b069 100644 --- a/internal/resolution/variablesources/installed_package_test.go +++ b/internal/resolution/variablesources/installed_package_test.go @@ -3,116 +3,161 @@ package variablesources_test import ( "context" "encoding/json" + "testing" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" + "github.com/stretchr/testify/assert" + featuregatetesting "k8s.io/component-base/featuregate/testing" "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" + "github.com/operator-framework/operator-controller/pkg/features" testutil "github.com/operator-framework/operator-controller/test/util" ) -var _ = Describe("InstalledPackageVariableSource", func() { - var ( - ipvs *variablesources.InstalledPackageVariableSource - fakeCatalogClient testutil.FakeCatalogClient - bundleImage string - ) - - BeforeEach(func() { - channel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "stable", - Entries: []declcfg.ChannelEntry{ - { - Name: "test-package.v1.0.0", - }, - { - Name: "test-package.v2.0.0", - Replaces: "test-package.v1.0.0", - }, - { - Name: "test-package.v3.0.0", - Replaces: "test-package.v2.0.0", - }, - { - Name: "test-package.v4.0.0", - Replaces: "test-package.v3.0.0", - }, - { - Name: "test-package.v5.0.0", - Replaces: "test-package.v4.0.0", - }, +func TestInstalledPackageVariableSource(t *testing.T) { + channel := catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "stable", + Entries: []declcfg.ChannelEntry{ + { + Name: "test-package.v1.0.0", + }, + { + Name: "test-package.v2.0.0", + Replaces: "test-package.v1.0.0", }, - }} - bundleList := []*catalogmetadata.Bundle{ - {Bundle: declcfg.Bundle{ - Name: "test-package.v1.0.0", - Package: "test-package", - Image: "registry.io/repo/test-package@v1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, + { + Name: "test-package.v2.1.0", + Replaces: "test-package.v2.0.0", }, - {Bundle: declcfg.Bundle{ - Name: "test-package.v3.0.0", - Package: "test-package", - Image: "registry.io/repo/test-package@v3.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, + { + Name: "test-package.v2.2.0", + Replaces: "test-package.v2.1.0", }, - {Bundle: declcfg.Bundle{ - Name: "test-package.v2.0.0", - Package: "test-package", - Image: "registry.io/repo/test-package@v2.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, + { + Name: "test-package.v3.0.0", + Replaces: "test-package.v2.2.0", }, - {Bundle: declcfg.Bundle{ - Name: "test-package.v4.0.0", - Package: "test-package", - Image: "registry.io/repo/test-package@v4.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "4.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, + { + Name: "test-package.v4.0.0", + Replaces: "test-package.v3.0.0", }, - {Bundle: declcfg.Bundle{ - Name: "test-package.v5.0.0", - Package: "test-package", - Image: "registry.io/repo/test-package@v5.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "5-0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, + { + Name: "test-package.v5.0.0", + Replaces: "test-package.v4.0.0", }, - } - var err error - bundleImage = "registry.io/repo/test-package@v2.0.0" - fakeCatalogClient = testutil.NewFakeCatalogClient(bundleList) - ipvs, err = variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage) - Expect(err).NotTo(HaveOccurred()) + }, + }} + bundleList := []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{ + Name: "test-package.v1.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v3.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v3.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v2.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v2.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v2.1.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v2.1.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.1.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v2.2.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v2.2.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.2.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v4.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v4.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "4.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v5.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v5.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "5-0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + } + + const bundleImage = "registry.io/repo/test-package@v2.0.0" + fakeCatalogClient := testutil.NewFakeCatalogClient(bundleList) + + t.Run("with ForceSemverUpgradeConstraints feature gate enabled", func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() + + ipvs, err := variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage) + assert.NoError(t, err) + + variables, err := ipvs.GetVariables(context.TODO()) + assert.NoError(t, err) + assert.Len(t, variables, 1) + packageVariable, ok := variables[0].(*olmvariables.InstalledPackageVariable) + assert.True(t, ok) + assert.Equal(t, deppy.IdentifierFromString("installed package test-package"), packageVariable.Identifier()) + + // ensure bundles are in version order (high to low) + bundles := packageVariable.Bundles() + assert.Len(t, bundles, 3) + assert.Equal(t, "test-package.v2.2.0", packageVariable.Bundles()[0].Name) + assert.Equal(t, "test-package.v2.1.0", packageVariable.Bundles()[1].Name) + assert.Equal(t, "test-package.v2.0.0", packageVariable.Bundles()[2].Name) }) - It("should return the correct package variable", func() { + t.Run("with ForceSemverUpgradeConstraints feature gate disabled", func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() + + ipvs, err := variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage) + assert.NoError(t, err) + variables, err := ipvs.GetVariables(context.TODO()) - Expect(err).NotTo(HaveOccurred()) - Expect(variables).To(HaveLen(1)) - reqPackageVar, ok := variables[0].(*olmvariables.InstalledPackageVariable) - Expect(ok).To(BeTrue()) - Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString("installed package test-package"))) + assert.NoError(t, err) + assert.Len(t, variables, 1) + packageVariable, ok := variables[0].(*olmvariables.InstalledPackageVariable) + assert.True(t, ok) + assert.Equal(t, deppy.IdentifierFromString("installed package test-package"), packageVariable.Identifier()) // ensure bundles are in version order (high to low) - Expect(reqPackageVar.Bundles()).To(HaveLen(2)) - Expect(reqPackageVar.Bundles()[0].Name).To(Equal("test-package.v3.0.0")) - Expect(reqPackageVar.Bundles()[1].Name).To(Equal("test-package.v2.0.0")) + bundles := packageVariable.Bundles() + assert.Len(t, bundles, 2) + assert.Equal(t, "test-package.v2.1.0", packageVariable.Bundles()[0].Name) + assert.Equal(t, "test-package.v2.0.0", packageVariable.Bundles()[1].Name) }) -}) +}