diff --git a/internal/catalogmetadata/filter/successors.go b/internal/catalogmetadata/filter/successors.go index 13467614a..cd2ea4f5b 100644 --- a/internal/catalogmetadata/filter/successors.go +++ b/internal/catalogmetadata/filter/successors.go @@ -9,15 +9,9 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/features" ) func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) { - var successors successorsPredicateFunc = legacySuccessor - if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) { - successors = semverSuccessor - } - installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) if err != nil { return nil, fmt.Errorf("parsing installed bundle %q version %q: %w", installedBundle.Name, installedBundle.Version, err) @@ -28,7 +22,7 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann return nil, fmt.Errorf("parsing installed version constraint %q: %w", installedBundleVersion.String(), err) } - successorsPredicate, err := successors(installedBundle, channels...) + successorsPredicate, err := legacySuccessor(installedBundle, channels...) if err != nil { return nil, fmt.Errorf("getting successorsPredicate: %w", err) } @@ -40,10 +34,6 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann ), nil } -// successorsPredicateFunc returns a predicate to find successors -// for a bundle. Predicate must not include the current version. -type successorsPredicateFunc func(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) - func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) { installedBundleVersion, err := bsemver.Parse(installedBundle.Version) if err != nil { @@ -84,28 +74,3 @@ func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Ch return false }, nil } - -// semverSuccessor returns a predicate to find 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. -// -// NOTE: semverSuccessor does not consider channels since there is no information -// in a channel entry that is necessary to determine if a bundle is a successor. -// A semver range check is the only necessary element. If filtering by channel -// membership is necessary, an additional filter for that purpose should be applied. -func semverSuccessor(installedBundle ocv1.BundleMetadata, _ ...declcfg.Channel) (Predicate[declcfg.Bundle], error) { - currentVersion, err := mmsemver.NewVersion(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("^%[1]s, != %[1]s", currentVersion.String()) - wantedVersionRangeConstraint, err := mmsemver.NewConstraint(constraintStr) - if err != nil { - return nil, err - } - - return InMastermindsSemverRange(wantedVersionRangeConstraint), nil -} diff --git a/internal/catalogmetadata/filter/successors_test.go b/internal/catalogmetadata/filter/successors_test.go index caec01509..2a0a53348 100644 --- a/internal/catalogmetadata/filter/successors_test.go +++ b/internal/catalogmetadata/filter/successors_test.go @@ -9,7 +9,6 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - featuregatetesting "k8s.io/component-base/featuregate/testing" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" @@ -17,182 +16,9 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/bundleutil" "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" - "github.com/operator-framework/operator-controller/internal/features" ) -func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true) - - const testPackageName = "test-package" - channelSet := map[string]declcfg.Channel{ - testPackageName: { - Package: testPackageName, - Name: "stable", - }, - } - - bundleSet := map[string]declcfg.Bundle{ - // Major version zero is for initial development and - // has different update behaviour than versions >= 1.0.0: - // - In versions 0.0.y updates are not allowed when using semver constraints - // - In versions 0.x.y only patch updates are allowed (>= 0.x.y and < 0.x+1.0) - // This means that we need in test data bundles that cover these three version ranges. - "test-package.v0.0.1": { - Name: "test-package.v0.0.1", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.0.1", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "0.0.1"), - }, - }, - "test-package.v0.0.2": { - Name: "test-package.v0.0.2", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.0.2", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "0.0.2"), - }, - }, - "test-package.v0.1.0": { - Name: "test-package.v0.1.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.1.0", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "0.1.0"), - }, - }, - "test-package.v0.1.1": { - Name: "test-package.v0.1.1", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.1.1", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "0.1.1"), - }, - }, - "test-package.v0.1.2": { - Name: "test-package.v0.1.2", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.1.2", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "0.1.2"), - }, - }, - "test-package.v0.2.0": { - Name: "test-package.v0.2.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.2.0", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "0.2.0"), - }, - }, - "test-package.v2.0.0": { - Name: "test-package.v2.0.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.0.0", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "2.0.0"), - }, - }, - "test-package.v2.1.0": { - Name: "test-package.v2.1.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.1.0", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "2.1.0"), - }, - }, - "test-package.v2.2.0": { - Name: "test-package.v2.2.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.2.0", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "2.2.0"), - }, - }, - // We need a bundle with a different major version to ensure - // that we do not allow upgrades from one major version to another - "test-package.v3.0.0": { - Name: "test-package.v3.0.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v3.0.0", - Properties: []property.Property{ - property.MustBuildPackage(testPackageName, "3.0.0"), - }, - }, - } - - for _, b := range bundleSet { - ch := channelSet[b.Package] - ch.Entries = append(ch.Entries, declcfg.ChannelEntry{Name: b.Name}) - channelSet[b.Package] = ch - } - - for _, tt := range []struct { - name string - installedBundle ocv1.BundleMetadata - expectedResult []declcfg.Bundle - }{ - { - name: "with non-zero major version", - installedBundle: bundleutil.MetadataFor("test-package.v2.0.0", bsemver.MustParse("2.0.0")), - expectedResult: []declcfg.Bundle{ - // Updates are allowed within the major version - bundleSet["test-package.v2.2.0"], - bundleSet["test-package.v2.1.0"], - bundleSet["test-package.v2.0.0"], - }, - }, - { - name: "with zero major and zero minor version", - installedBundle: bundleutil.MetadataFor("test-package.v0.0.1", bsemver.MustParse("0.0.1")), - expectedResult: []declcfg.Bundle{ - // No updates are allowed in major version zero when minor version is also zero - bundleSet["test-package.v0.0.1"], - }, - }, - { - name: "with zero major and non-zero minor version", - installedBundle: bundleutil.MetadataFor("test-package.v0.1.0", bsemver.MustParse("0.1.0")), - expectedResult: []declcfg.Bundle{ - // Patch version updates are allowed within the minor version - bundleSet["test-package.v0.1.2"], - bundleSet["test-package.v0.1.1"], - bundleSet["test-package.v0.1.0"], - }, - }, - { - name: "installed bundle not found", - installedBundle: ocv1.BundleMetadata{ - Name: "test-package.v9.0.0", - Version: "9.0.0", - }, - expectedResult: []declcfg.Bundle{}, - }, - } { - t.Run(tt.name, func(t *testing.T) { - successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName]) - require.NoError(t, err) - - allBundles := make([]declcfg.Bundle, 0, len(bundleSet)) - for _, bundle := range bundleSet { - allBundles = append(allBundles, bundle) - } - result := Filter(allBundles, successors) - - // sort before comparison for stable order - slices.SortFunc(result, compare.ByVersion) - - gocmpopts := []cmp.Option{ - cmpopts.IgnoreUnexported(declcfg.Bundle{}), - } - require.Empty(t, cmp.Diff(result, tt.expectedResult, gocmpopts...)) - }) - } -} - -func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false) - +func TestSuccessorsPredicate(t *testing.T) { const testPackageName = "test-package" channelSet := map[string]declcfg.Channel{ testPackageName: { diff --git a/internal/features/features.go b/internal/features/features.go index 329737a96..399bc7356 100644 --- a/internal/features/features.go +++ b/internal/features/features.go @@ -6,17 +6,13 @@ import ( ) const ( - // Add new feature gates constants (strings) - // Ex: SomeFeature featuregate.Feature = "SomeFeature" - - ForceSemverUpgradeConstraints featuregate.Feature = "ForceSemverUpgradeConstraints" +// Add new feature gates constants (strings) +// Ex: SomeFeature featuregate.Feature = "SomeFeature" ) var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ // Add new feature gate definitions // Ex: SomeFeature: {...} - - ForceSemverUpgradeConstraints: {Default: false, PreRelease: featuregate.Alpha}, } var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate() diff --git a/internal/resolve/catalog_test.go b/internal/resolve/catalog_test.go index 1054a1fcd..505cbb790 100644 --- a/internal/resolve/catalog_test.go +++ b/internal/resolve/catalog_test.go @@ -12,7 +12,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/rand" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -21,7 +20,6 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" catalogd "github.com/operator-framework/operator-controller/catalogd/api/v1" - "github.com/operator-framework/operator-controller/internal/features" ) func TestInvalidClusterExtensionVersionRange(t *testing.T) { @@ -426,7 +424,6 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) { } func TestUpgradeFoundLegacy(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false) pkgName := randPkg() w := staticCatalogWalker{ "a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { @@ -454,7 +451,6 @@ func TestUpgradeFoundLegacy(t *testing.T) { } func TestUpgradeNotFoundLegacy(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false) pkgName := randPkg() w := staticCatalogWalker{ "a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { @@ -478,62 +474,6 @@ func TestUpgradeNotFoundLegacy(t *testing.T) { assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "0.1.0": no bundles found for package %q matching version "<1.0.0 >=2.0.0"`, pkgName)) } -func TestUpgradeFoundSemver(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true) - pkgName := randPkg() - w := staticCatalogWalker{ - "a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { - return &declcfg.DeclarativeConfig{}, nil, nil - }, - "b": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { - return &declcfg.DeclarativeConfig{}, nil, nil - }, - "c": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { - return genPackage(pkgName), nil, nil - }, - } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} - ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - installedBundle := &ocv1.BundleMetadata{ - Name: bundleName(pkgName, "1.0.0"), - Version: "1.0.0", - } - // there is a legacy upgrade edge from 1.0.0 to 2.0.0, but we are using semver semantics here. - // therefore: - // 1.0.0 => 1.0.2 is what we expect - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, installedBundle) - require.NoError(t, err) - assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle) - assert.Equal(t, bsemver.MustParse("1.0.2"), *gotVersion) - assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) -} - -func TestUpgradeNotFoundSemver(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true) - pkgName := randPkg() - w := staticCatalogWalker{ - "a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { - return &declcfg.DeclarativeConfig{}, nil, nil - }, - "b": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { - return &declcfg.DeclarativeConfig{}, nil, nil - }, - "c": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { - return genPackage(pkgName), nil, nil - }, - } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} - ce := buildFooClusterExtension(pkgName, []string{}, "!=0.1.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - installedBundle := &ocv1.BundleMetadata{ - Name: bundleName(pkgName, "0.1.0"), - Version: "0.1.0", - } - // there are legacy upgrade edges from 0.1.0 to 1.0.x, but we are using semver semantics here. - // therefore, we expect to fail because there are no semver-compatible upgrade edges from 0.1.0. - _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) - assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "0.1.0": no bundles found for package %q matching version "!=0.1.0"`, pkgName)) -} - func TestDowngradeFound(t *testing.T) { pkgName := randPkg() w := staticCatalogWalker{ @@ -838,7 +778,6 @@ func TestInvalidClusterExtensionCatalogMatchLabelsValue(t *testing.T) { } func TestClusterExtensionMatchLabel(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false) pkgName := randPkg() w := staticCatalogWalker{ "a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) { @@ -859,7 +798,6 @@ func TestClusterExtensionMatchLabel(t *testing.T) { } func TestClusterExtensionNoMatchLabel(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false) pkgName := randPkg() w := staticCatalogWalker{ "a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {