diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 50722c8c4..bde0f91a7 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -55,7 +55,6 @@ spec: - "--health-probe-bind-address=:8081" - "--metrics-bind-address=127.0.0.1:8080" - "--leader-elect" - - "--feature-gates=ForceSemverUpgradeConstraints=true" image: controller:latest imagePullPolicy: IfNotPresent name: manager diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index 1d5761a95..c73870ddc 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -3,6 +3,7 @@ package filter import ( mmsemver "github.com/Masterminds/semver/v3" bsemver "github.com/blang/semver/v4" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) @@ -58,11 +59,30 @@ func WithBundleImage(bundleImage string) Predicate[catalogmetadata.Bundle] { } } -func Replaces(bundleName string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - for _, ch := range bundle.InChannels { +func LegacySuccessor(installedBundle *catalogmetadata.Bundle) Predicate[catalogmetadata.Bundle] { + isSuccessor := func(candidateBundleEntry declcfg.ChannelEntry) bool { + if candidateBundleEntry.Replaces == installedBundle.Name { + return true + } + for _, skip := range candidateBundleEntry.Skips { + if skip == installedBundle.Name { + return true + } + } + if candidateBundleEntry.SkipRange != "" { + installedBundleVersion, _ := installedBundle.Version() + skipRange, _ := bsemver.ParseRange(candidateBundleEntry.SkipRange) + if installedBundleVersion != nil && skipRange != nil && skipRange(*installedBundleVersion) { + return true + } + } + return false + } + + return func(candidateBundle *catalogmetadata.Bundle) bool { + for _, ch := range candidateBundle.InChannels { for _, chEntry := range ch.Entries { - if bundle.Name == chEntry.Name && chEntry.Replaces == bundleName { + if candidateBundle.Name == chEntry.Name && isSuccessor(chEntry) { return true } } diff --git a/internal/catalogmetadata/filter/bundle_predicates_test.go b/internal/catalogmetadata/filter/bundle_predicates_test.go index 3617f47ce..c9770b03e 100644 --- a/internal/catalogmetadata/filter/bundle_predicates_test.go +++ b/internal/catalogmetadata/filter/bundle_predicates_test.go @@ -126,7 +126,7 @@ func TestWithBundleImage(t *testing.T) { assert.False(t, f(b3)) } -func TestReplaces(t *testing.T) { +func TestLegacySuccessor(t *testing.T) { fakeChannel := &catalogmetadata.Channel{ Channel: declcfg.Channel{ Entries: []declcfg.ChannelEntry{ @@ -138,25 +138,51 @@ func TestReplaces(t *testing.T) { Name: "package1.v0.0.3", Replaces: "package1.v0.0.2", }, + { + Name: "package1.v0.0.4", + Skips: []string{"package1.v0.0.1"}, + }, + { + Name: "package1.v0.0.5", + SkipRange: "<=0.0.1", + }, + }, + }, + } + installedBundle := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "package1.v0.0.1", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package1", "version": "0.0.1"}`)}, }, }, } - b1 := &catalogmetadata.Bundle{ + b2 := &catalogmetadata.Bundle{ Bundle: declcfg.Bundle{Name: "package1.v0.0.2"}, InChannels: []*catalogmetadata.Channel{fakeChannel}, } - b2 := &catalogmetadata.Bundle{ + b3 := &catalogmetadata.Bundle{ Bundle: declcfg.Bundle{Name: "package1.v0.0.3"}, InChannels: []*catalogmetadata.Channel{fakeChannel}, } - b3 := &catalogmetadata.Bundle{} + b4 := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{Name: "package1.v0.0.4"}, + InChannels: []*catalogmetadata.Channel{fakeChannel}, + } + b5 := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{Name: "package1.v0.0.5"}, + InChannels: []*catalogmetadata.Channel{fakeChannel}, + } + emptyBundle := &catalogmetadata.Bundle{} - f := filter.Replaces("package1.v0.0.1") + f := filter.LegacySuccessor(installedBundle) - assert.True(t, f(b1)) - assert.False(t, f(b2)) + assert.True(t, f(b2)) assert.False(t, f(b3)) + assert.True(t, f(b4)) + assert.True(t, f(b5)) + assert.False(t, f(emptyBundle)) } func TestWithDeprecation(t *testing.T) { diff --git a/internal/controllers/common_controller.go b/internal/controllers/common_controller.go index 98067824d..fd721f469 100644 --- a/internal/controllers/common_controller.go +++ b/internal/controllers/common_controller.go @@ -22,9 +22,8 @@ import ( apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" - ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) // BundleProvider provides the way to retrieve a list of Bundles from a source, @@ -99,7 +98,7 @@ func setInstalledStatusConditionFailed(conditions *[]metav1.Condition, message s }) } -// setDEprecationStatusesUnknown sets the deprecation status conditions to unknown. +// setDeprecationStatusesUnknown sets the deprecation status conditions to unknown. func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message string, generation int64) { conditionTypes := []string{ ocv1alpha1.TypeDeprecated, diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index 339bded3b..46b314831 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -94,11 +94,10 @@ type successorsFunc func(allBundles []*catalogmetadata.Bundle, installedBundle * // 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 + // find the bundles that replace, skip, or skipRange the bundle provided upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.And( catalogfilter.WithPackageName(installedBundle.Package), - catalogfilter.Replaces(installedBundle.Name), + catalogfilter.LegacySuccessor(installedBundle), )) sort.SliceStable(upgradeEdges, func(i, j int) bool { return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j]) diff --git a/internal/resolution/variablesources/installed_package_test.go b/internal/resolution/variablesources/installed_package_test.go index 8dec706ee..2668b450e 100644 --- a/internal/resolution/variablesources/installed_package_test.go +++ b/internal/resolution/variablesources/installed_package_test.go @@ -306,6 +306,20 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( Name: "test-package.v2.2.0", Replaces: "test-package.v2.1.0", }, + { + Name: "test-package.v2.2.1", + }, + { + Name: "test-package.v2.3.0", + Replaces: "test-package.v2.2.0", + Skips: []string{ + "test-package.v2.2.1", + }, + }, + { + Name: "test-package.v2.4.0", + SkipRange: ">=2.3.0 <2.4.0", + }, }, }} bundleSet := map[string]*catalogmetadata.Bundle{ @@ -342,6 +356,39 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( }, InChannels: []*catalogmetadata.Channel{&testPackageChannel}, }, + "test-package.v2.2.1": { + Bundle: declcfg.Bundle{ + Name: "test-package.v2.2.1", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.2.1", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.2.1"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&testPackageChannel}, + }, + "test-package.v2.3.0": { + Bundle: declcfg.Bundle{ + Name: "test-package.v2.3.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.3.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.3.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&testPackageChannel}, + }, + "test-package.v2.4.0": { + Bundle: declcfg.Bundle{ + Name: "test-package.v2.4.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.4.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.4.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&testPackageChannel}, + }, // We need a bundle from different package to ensure that // we filter out bundles certain bundle image "some-other-package.v2.3.0": { @@ -382,6 +429,34 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( }), }, }, + { + name: "respect skips directive from catalog", + upgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyEnforce, + installedBundle: bundleSet["test-package.v2.2.1"], + expectedResult: []*olmvariables.InstalledPackageVariable{ + olmvariables.NewInstalledPackageVariable(testPackageName, []*catalogmetadata.Bundle{ + // Must only have two bundle: + // - the one which skips the current version + // - the current version (to allow to stay on the current version) + bundleSet["test-package.v2.3.0"], + bundleSet["test-package.v2.2.1"], + }), + }, + }, + { + name: "respect skipRange directive from catalog", + upgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyEnforce, + installedBundle: bundleSet["test-package.v2.3.0"], + expectedResult: []*olmvariables.InstalledPackageVariable{ + olmvariables.NewInstalledPackageVariable(testPackageName, []*catalogmetadata.Bundle{ + // Must only have two bundle: + // - the one which is skipRanges the current version + // - the current version (to allow to stay on the current version) + bundleSet["test-package.v2.4.0"], + bundleSet["test-package.v2.3.0"], + }), + }, + }, { name: "UpgradeConstraintPolicy is set to Ignore", upgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyIgnore, @@ -409,6 +484,22 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( }, expectedError: `bundle with image "registry.io/repo/test-package@v9.0.0" for package "test-package" not found in available catalogs but is currently installed via BundleDeployment "test-package-bd"`, }, + { + name: "installed bundle not found, but UpgradeConstraintPolicy is set to Ignore", + upgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyIgnore, + installedBundle: &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "test-package.v9.0.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v9.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "9.0.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&testPackageChannel}, + }, + expectedResult: []*olmvariables.InstalledPackageVariable{}, + }, } { t.Run(tt.name, func(t *testing.T) { fakeOwnerClusterExtension := fakeClusterExtension("test-extension-legacy", testPackageName, tt.upgradeConstraintPolicy) diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index 2c3e0f49e..4b7aa4a17 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -236,7 +236,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) { }, pollDuration, pollInterval) } -func TestClusterExtensionInstallNonSuccessorVersion(t *testing.T) { +func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) { t.Log("When a cluster extension is installed from a catalog") t.Log("When resolving upgrade edges") @@ -264,8 +264,8 @@ func TestClusterExtensionInstallNonSuccessorVersion(t *testing.T) { t.Log("It does not allow to upgrade the ClusterExtension to a non-successor version") t.Log("By updating the ClusterExtension resource to a non-successor version") - // Semver only allows upgrades within major version at the moment. - clusterExtension.Spec.Version = "2.0.0" + // 1.2.0 does not replace/skip/skipRange 1.0.0. + clusterExtension.Spec.Version = "1.2.0" require.NoError(t, c.Update(context.Background(), clusterExtension)) t.Log("By eventually reporting an unsatisfiable resolution") require.EventuallyWithT(t, func(ct *assert.CollectT) { @@ -276,11 +276,56 @@ func TestClusterExtensionInstallNonSuccessorVersion(t *testing.T) { } assert.Equal(ct, ocv1alpha1.ReasonResolutionFailed, cond.Reason) assert.Contains(ct, cond.Message, "constraints not satisfiable") - assert.Contains(ct, cond.Message, "installed package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.1.2.0, test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0") + assert.Contains(ct, cond.Message, "installed package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0") assert.Empty(ct, clusterExtension.Status.ResolvedBundle) }, pollDuration, pollInterval) } +func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) { + t.Log("When a cluster extension is installed from a catalog") + t.Log("When resolving upgrade edges") + + clusterExtension, _, extensionCatalog := testInit(t) + defer testCleanup(t, extensionCatalog, clusterExtension) + defer getArtifactsOutput(t) + + t.Log("By creating an ClusterExtension at a specified version") + clusterExtension.Spec = ocv1alpha1.ClusterExtensionSpec{ + PackageName: "prometheus", + Version: "1.0.0", + } + require.NoError(t, c.Create(context.Background(), clusterExtension)) + t.Log("By eventually reporting a successful resolution") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) + if !assert.NotNil(ct, cond) { + return + } + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) + assert.Contains(ct, cond.Message, "resolved to") + assert.Equal(ct, &ocv1alpha1.BundleMetadata{Name: "prometheus-operator.1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) + }, pollDuration, pollInterval) + + t.Log("It does not allow to upgrade the ClusterExtension to a non-successor version") + t.Log("By updating the ClusterExtension resource to a non-successor version") + // 1.2.0 does not replace/skip/skipRange 1.0.0. + clusterExtension.Spec.Version = "1.2.0" + clusterExtension.Spec.UpgradeConstraintPolicy = ocv1alpha1.UpgradeConstraintPolicyIgnore + require.NoError(t, c.Update(context.Background(), clusterExtension)) + t.Log("By eventually reporting an unsatisfiable resolution") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) + if !assert.NotNil(ct, cond) { + return + } + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) + assert.Contains(ct, cond.Message, "resolved to") + assert.Equal(ct, &ocv1alpha1.BundleMetadata{Name: "prometheus-operator.1.2.0", Version: "1.2.0"}, clusterExtension.Status.ResolvedBundle) + }, pollDuration, pollInterval) +} + func TestClusterExtensionInstallSuccessorVersion(t *testing.T) { t.Log("When a cluster extension is installed from a catalog") t.Log("When resolving upgrade edges") @@ -308,8 +353,8 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) { t.Log("It does allow to upgrade the ClusterExtension to any of the successor versions within non-zero major version") t.Log("By updating the ClusterExtension resource by skipping versions") - // Test catalog has versions between the initial version and new version - clusterExtension.Spec.Version = "1.2.0" + // 1.0.1 replaces 1.0.0 in the test catalog + clusterExtension.Spec.Version = "1.0.1" require.NoError(t, c.Update(context.Background(), clusterExtension)) t.Log("By eventually reporting a successful resolution and bundle path") require.EventuallyWithT(t, func(ct *assert.CollectT) { @@ -320,7 +365,7 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) { } assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) assert.Contains(ct, cond.Message, "resolved to") - assert.Equal(ct, &ocv1alpha1.BundleMetadata{Name: "prometheus-operator.1.2.0", Version: "1.2.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(ct, &ocv1alpha1.BundleMetadata{Name: "prometheus-operator.1.0.1", Version: "1.0.1"}, clusterExtension.Status.ResolvedBundle) }, pollDuration, pollInterval) }