fixup! fixup! PoC: refactor variable sources

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
operator-framework · Oct 20, 2023 · 4a0d099 · 4a0d099
1 parent cd2ab54
commit 4a0d099
Show file tree

Hide file tree

Showing 2 changed files with 149 additions and 2 deletions.
diff --git a/internal/resolution/variablesources/olm.go b/internal/resolution/variablesources/olm.go
@@ -18,6 +18,7 @@ import (
 	catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
 	catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
 	olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
+	"github.com/operator-framework/operator-controller/pkg/features"
 )
 
 var _ input.VariableSource = &OLMVariableSource{}
@@ -142,8 +143,10 @@ func InstalledPackageVariables(
 	allBundles []*catalogmetadata.Bundle,
 	bundleDeployments []rukpakv1alpha1.BundleDeployment,
 ) ([]*olmvariables.InstalledPackageVariable, error) {
-	// TODO: Switch between legacy and semver based on feature flag once semver implemented
 	var successors successorsFunc = legacySemanticsSuccessors
+	if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
+		successors = semverSuccessors
+	}
 
 	result := make([]*olmvariables.InstalledPackageVariable, 0, len(bundleDeployments))
 	processed := sets.Set[string]{}
@@ -205,6 +208,31 @@ func legacySemanticsSuccessors(allBundles []*catalogmetadata.Bundle, installedBu
 	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 minor 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
+}
+
 func BundleVariables(
 	allBundles []*catalogmetadata.Bundle,
 	requiredPackages []*olmvariables.RequiredPackageVariable,

diff --git a/internal/resolution/variablesources/olm_test.go b/internal/resolution/variablesources/olm_test.go
@@ -140,7 +140,27 @@ func TestInstalledPackageVariables(t *testing.T) {
 		Name: "stable",
 		Entries: []declcfg.ChannelEntry{
 			{
-				Name: "test-package.v1.0.0",
+				Name: "test-package.v0.0.1",
+			},
+			{
+				Name:     "test-package.v0.0.2",
+				Replaces: "test-package.v0.0.1",
+			},
+			{
+				Name:     "test-package.v0.1.0",
+				Replaces: "test-package.v0.0.2",
+			},
+			{
+				Name:     "test-package.v0.1.1",
+				Replaces: "test-package.v0.1.0",
+			},
+			{
+				Name:     "test-package.v0.2.0",
+				Replaces: "test-package.v0.1.1",
+			},
+			{
+				Name:     "test-package.v1.0.0",
+				Replaces: "test-package.v0.2.0",
 			},
 			{
 				Name:     "test-package.v2.0.0",
@@ -169,6 +189,51 @@ func TestInstalledPackageVariables(t *testing.T) {
 		},
 	}}
 	allBundles := []*catalogmetadata.Bundle{
+		{Bundle: declcfg.Bundle{
+			Name:    "test-package.v0.0.1",
+			Package: "test-package",
+			Image:   "registry.io/repo/test-package@v0.0.1",
+			Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.0.1"}`)},
+			}},
+			InChannels: []*catalogmetadata.Channel{&channel},
+		},
+		{Bundle: declcfg.Bundle{
+			Name:    "test-package.v0.0.2",
+			Package: "test-package",
+			Image:   "registry.io/repo/test-package@v0.0.2",
+			Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.0.2"}`)},
+			}},
+			InChannels: []*catalogmetadata.Channel{&channel},
+		},
+		{Bundle: declcfg.Bundle{
+			Name:    "test-package.v0.1.0",
+			Package: "test-package",
+			Image:   "registry.io/repo/test-package@v0.1.0",
+			Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.1.0"}`)},
+			}},
+			InChannels: []*catalogmetadata.Channel{&channel},
+		},
+		{Bundle: declcfg.Bundle{
+			Name:    "test-package.v0.1.1",
+			Package: "test-package",
+			Image:   "registry.io/repo/test-package@v0.1.1",
+			Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.1.1"}`)},
+			}},
+			InChannels: []*catalogmetadata.Channel{&channel},
+		},
+		{Bundle: declcfg.Bundle{
+			Name:    "test-package.v0.2.0",
+			Package: "test-package",
+			Image:   "registry.io/repo/test-package@v0.2.0",
+			Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.2.0"}`)},
+			}},
+			InChannels: []*catalogmetadata.Channel{&channel},
+		},
 		{Bundle: declcfg.Bundle{
 			Name:    "test-package.v1.0.0",
 			Package: "test-package",
@@ -259,6 +324,60 @@ func TestInstalledPackageVariables(t *testing.T) {
 		return bundleDeployments
 	}
 
+	t.Run("with ForceSemverUpgradeConstraints feature gate enabled", func(t *testing.T) {
+		defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)()
+
+		t.Run("with non-zero major version", func(t *testing.T) {
+			const bundleImage = "registry.io/repo/test-package@v2.0.0"
+			installedPackages, err := variablesources.InstalledPackageVariables(allBundles, fakeBundleDeployments(bundleImage))
+			require.NoError(t, err)
+
+			require.Len(t, installedPackages, 1)
+			packageVariable := installedPackages[0]
+			assert.Equal(t, deppy.IdentifierFromString("installed package test-package"), packageVariable.Identifier())
+
+			// ensure bundles are in version order (high to low)
+			bundles := packageVariable.Bundles()
+			require.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)
+		})
+
+		t.Run("with zero major version", func(t *testing.T) {
+			t.Run("with zero minor version", func(t *testing.T) {
+				const bundleImage = "registry.io/repo/test-package@v0.0.1"
+				installedPackages, err := variablesources.InstalledPackageVariables(allBundles, fakeBundleDeployments(bundleImage))
+				require.NoError(t, err)
+
+				require.Len(t, installedPackages, 1)
+				packageVariable := installedPackages[0]
+				assert.Equal(t, deppy.IdentifierFromString("installed package test-package"), packageVariable.Identifier())
+
+				// No upgrades are allowed in major version zero when minor version is also zero
+				bundles := packageVariable.Bundles()
+				require.Len(t, bundles, 1)
+				assert.Equal(t, "test-package.v0.0.1", packageVariable.Bundles()[0].Name)
+			})
+
+			t.Run("with non-zero minor version", func(t *testing.T) {
+				const bundleImage = "registry.io/repo/test-package@v0.1.0"
+				installedPackages, err := variablesources.InstalledPackageVariables(allBundles, fakeBundleDeployments(bundleImage))
+				require.NoError(t, err)
+
+				require.Len(t, installedPackages, 1)
+				packageVariable := installedPackages[0]
+				assert.Equal(t, deppy.IdentifierFromString("installed package test-package"), packageVariable.Identifier())
+
+				// Patch version upgrades are allowed, but not minor upgrades
+				bundles := packageVariable.Bundles()
+				require.Len(t, bundles, 2)
+				assert.Equal(t, "test-package.v0.1.1", packageVariable.Bundles()[0].Name)
+				assert.Equal(t, "test-package.v0.1.0", packageVariable.Bundles()[1].Name)
+			})
+		})
+	})
+
 	t.Run("with ForceSemverUpgradeConstraints feature gate disabled", func(t *testing.T) {
 		defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)()