diff --git a/cmd/manager/main.go b/cmd/manager/main.go index ab677dc38..bc5f89724 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -188,7 +188,7 @@ func main() { Unpacker: unpacker, Storage: localStorage, Handler: handler.HandlerFunc(handler.HandleClusterExtension), - InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{}, + InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg}, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") os.Exit(1) diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index 4dd7559fe..8f16c6c5e 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -6,6 +6,7 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) @@ -66,7 +67,7 @@ func WithBundleName(bundleName string) Predicate[catalogmetadata.Bundle] { } } -func LegacySuccessor(installedBundle *catalogmetadata.Bundle) Predicate[catalogmetadata.Bundle] { +func LegacySuccessor(installedBundle *ocv1alpha1.BundleMetadata) Predicate[catalogmetadata.Bundle] { isSuccessor := func(candidateBundleEntry declcfg.ChannelEntry) bool { if candidateBundleEntry.Replaces == installedBundle.Name { return true @@ -77,9 +78,9 @@ func LegacySuccessor(installedBundle *catalogmetadata.Bundle) Predicate[catalogm } } if candidateBundleEntry.SkipRange != "" { - installedBundleVersion, _ := installedBundle.Version() - skipRange, _ := bsemver.ParseRange(candidateBundleEntry.SkipRange) - if installedBundleVersion != nil && skipRange != nil && skipRange(*installedBundleVersion) { + installedBundleVersion, vErr := bsemver.Parse(installedBundle.Version) + skipRange, srErr := bsemver.ParseRange(candidateBundleEntry.SkipRange) + if vErr == nil && srErr == nil && skipRange(installedBundleVersion) { return true } } diff --git a/internal/catalogmetadata/filter/bundle_predicates_test.go b/internal/catalogmetadata/filter/bundle_predicates_test.go index 6392a0154..51bb5746a 100644 --- a/internal/catalogmetadata/filter/bundle_predicates_test.go +++ b/internal/catalogmetadata/filter/bundle_predicates_test.go @@ -11,6 +11,7 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" ) @@ -150,13 +151,9 @@ func TestLegacySuccessor(t *testing.T) { }, }, } - 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"}`)}, - }, - }, + installedBundle := &ocv1alpha1.BundleMetadata{ + Name: "package1.v0.0.1", + Version: "0.0.1", } b2 := &catalogmetadata.Bundle{ diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 7b9ff0996..f78715741 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -94,7 +94,7 @@ type ClusterExtensionReconciler struct { } type InstalledBundleGetter interface { - GetInstalledBundle(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) + GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error) } //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch @@ -370,7 +370,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1 channelName := ext.Spec.Channel versionRange := ext.Spec.Version - installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, r.ActionClientGetter, allBundles, &ext) + installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, &ext) if err != nil { return nil, err } @@ -392,7 +392,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1 } if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore && installedBundle != nil { - upgradePredicate, err := SuccessorsPredicate(installedBundle) + upgradePredicate, err := SuccessorsPredicate(ext.Spec.PackageName, installedBundle) if err != nil { return nil, err } @@ -404,7 +404,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1 var upgradeErrorPrefix string if installedBundle != nil { - installedBundleVersion, err := installedBundle.Version() + installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) if err != nil { return nil, err } @@ -568,6 +568,7 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error { r.controller = controller r.cache = mgr.GetCache() r.dynamicWatchGVKs = map[schema.GroupVersionKind]struct{}{} + return nil } @@ -663,10 +664,12 @@ func (r *ClusterExtensionReconciler) getReleaseState(cl helmclient.ActionInterfa return currentRelease, stateUnchanged, nil } -type DefaultInstalledBundleGetter struct{} +type DefaultInstalledBundleGetter struct { + helmclient.ActionClientGetter +} -func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - cl, err := acg.ActionClientFor(ctx, ext) +func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error) { + cl, err := d.ActionClientFor(ctx, ext) if err != nil { return nil, err } @@ -679,27 +682,10 @@ func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, a return nil, nil } - // Bundle must match installed version exactly - vr, err := mmsemver.NewConstraint(release.Labels[labels.BundleVersionKey]) - if err != nil { - return nil, err - } - - // find corresponding bundle for the installed content - resultSet := catalogfilter.Filter(allBundles, catalogfilter.And( - catalogfilter.WithPackageName(release.Labels[labels.PackageNameKey]), - catalogfilter.WithBundleName(release.Labels[labels.BundleNameKey]), - catalogfilter.InMastermindsSemverRange(vr), - )) - if len(resultSet) == 0 { - return nil, fmt.Errorf("bundle %q for package %q not found in available catalogs but is currently installed in namespace %q", release.Labels[labels.BundleNameKey], ext.Spec.PackageName, release.Namespace) - } - - sort.SliceStable(resultSet, func(i, j int) bool { - return catalogsort.ByVersion(resultSet[i], resultSet[j]) - }) - - return resultSet[0], nil + return &ocv1alpha1.BundleMetadata{ + Name: release.Labels[labels.BundleNameKey], + Version: release.Labels[labels.BundleVersionKey], + }, nil } type postrenderer struct { @@ -749,7 +735,7 @@ func bundleMetadataFor(bundle *catalogmetadata.Bundle) *ocv1alpha1.BundleMetadat } func (r *ClusterExtensionReconciler) validateBundle(bundle *catalogmetadata.Bundle) error { - unsupportedProps := sets.New( + unsupportedProps := sets.New[string]( property.TypePackageRequired, property.TypeGVKRequired, property.TypeConstraint, diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index efa1191c8..ebacd924b 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -169,7 +169,7 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) { require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) t.Log("By checking the status fields") - require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) + require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) require.Empty(t, clusterExtension.Status.InstalledBundle) t.Log("By checking the expected conditions") @@ -227,7 +227,7 @@ func TestClusterExtensionChannelExistsNoVersion(t *testing.T) { require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) t.Log("By checking the status fields") - require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) + require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) require.Empty(t, clusterExtension.Status.InstalledBundle) t.Log("By checking the expected conditions") @@ -418,22 +418,6 @@ func verifyConditionsInvariants(t *testing.T, ext *ocv1alpha1.ClusterExtension) } func TestClusterExtensionUpgrade(t *testing.T) { - bundle := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - } - - cl, reconciler := newClientAndReconciler(t, bundle) - mockUnpacker := unpacker.(*MockUnpacker) // Set up the Unpack method to return a result with StateUnpackPending mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ @@ -442,6 +426,13 @@ func TestClusterExtensionUpgrade(t *testing.T) { ctx := context.Background() t.Run("semver upgrade constraints enforcement of upgrades within major version", func(t *testing.T) { + bundle := &ocv1alpha1.BundleMetadata{ + Name: "prometheus.v1.0.0", + Version: "1.0.0", + } + + cl, reconciler := newClientAndReconciler(t, bundle) + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) @@ -475,7 +466,7 @@ func TestClusterExtensionUpgrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -489,21 +480,6 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - bundle := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - } - - cl, reconciler := newClientAndReconciler(t, bundle) // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -539,7 +515,7 @@ func TestClusterExtensionUpgrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/1.2.0", Version: "1.2.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.2.0", Version: "1.2.0"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -550,6 +526,13 @@ func TestClusterExtensionUpgrade(t *testing.T) { }) t.Run("legacy semantics upgrade constraints enforcement", func(t *testing.T) { + bundle := &ocv1alpha1.BundleMetadata{ + Name: "prometheus.v1.0.0", + Version: "1.0.0", + } + + cl, reconciler := newClientAndReconciler(t, bundle) + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) @@ -583,7 +566,7 @@ func TestClusterExtensionUpgrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -597,22 +580,6 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - bundle := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - } - - cl, reconciler := newClientAndReconciler(t, bundle) - // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -648,7 +615,7 @@ func TestClusterExtensionUpgrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/1.0.1", Version: "1.0.1"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.1", Version: "1.0.1"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -673,6 +640,13 @@ func TestClusterExtensionUpgrade(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { + bundle := &ocv1alpha1.BundleMetadata{ + Name: "prometheus.v1.0.0", + Version: "1.0.0", + } + + cl, reconciler := newClientAndReconciler(t, bundle) + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, tt.flagState)() defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) @@ -704,7 +678,7 @@ func TestClusterExtensionUpgrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -720,22 +694,6 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - bundle := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - } - - cl, reconciler := newClientAndReconciler(t, bundle) - // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.NoError(t, err) @@ -746,7 +704,7 @@ func TestClusterExtensionUpgrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -760,7 +718,6 @@ func TestClusterExtensionUpgrade(t *testing.T) { } func TestClusterExtensionDowngrade(t *testing.T) { - cl, reconciler := newClientAndReconciler(t, nil) mockUnpacker := unpacker.(*MockUnpacker) // Set up the Unpack method to return a result with StateUnpacked mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ @@ -783,6 +740,13 @@ func TestClusterExtensionDowngrade(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { + bundle := &ocv1alpha1.BundleMetadata{ + Name: "prometheus.v1.0.1", + Version: "1.0.1", + } + + cl, reconciler := newClientAndReconciler(t, bundle) + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, tt.flagState)() defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) @@ -813,7 +777,7 @@ func TestClusterExtensionDowngrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/1.0.1", Version: "1.0.1"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.1", Version: "1.0.1"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -827,22 +791,6 @@ func TestClusterExtensionDowngrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - bundle := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.1", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.1", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.1"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - } - - cl, reconciler := newClientAndReconciler(t, bundle) - // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -881,6 +829,12 @@ func TestClusterExtensionDowngrade(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { + bundle := &ocv1alpha1.BundleMetadata{ + Name: "prometheus.v2.0.0", + Version: "2.0.0", + } + + cl, reconciler := newClientAndReconciler(t, bundle) defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, tt.flagState)() defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) @@ -912,7 +866,7 @@ func TestClusterExtensionDowngrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -926,22 +880,6 @@ func TestClusterExtensionDowngrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - bundle := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/2.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake2.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"2.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - } - - cl, reconciler := newClientAndReconciler(t, bundle) - // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.NoError(t, err) @@ -952,7 +890,7 @@ func TestClusterExtensionDowngrade(t *testing.T) { require.NoError(t, err) // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) // checking the expected conditions cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -1422,19 +1360,19 @@ var ( Package: "prometheus", Entries: []declcfg.ChannelEntry{ { - Name: "operatorhub/prometheus/beta/1.0.0", + Name: "prometheus.v1.0.0", }, { - Name: "operatorhub/prometheus/beta/1.0.1", - Replaces: "operatorhub/prometheus/beta/1.0.0", + Name: "prometheus.v1.0.1", + Replaces: "prometheus.v1.0.0", }, { - Name: "operatorhub/prometheus/beta/1.2.0", - Replaces: "operatorhub/prometheus/beta/1.0.1", + Name: "prometheus.v1.2.0", + Replaces: "prometheus.v1.0.1", }, { - Name: "operatorhub/prometheus/beta/2.0.0", - Replaces: "operatorhub/prometheus/beta/1.2.0", + Name: "prometheus.v2.0.0", + Replaces: "prometheus.v1.2.0", }, }, }, @@ -1444,7 +1382,7 @@ var ( var testBundleList = []*catalogmetadata.Bundle{ { Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/alpha/0.37.0", + Name: "prometheus.v0.37.0", Package: "prometheus", Image: "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", Properties: []property.Property{ @@ -1457,7 +1395,7 @@ var testBundleList = []*catalogmetadata.Bundle{ }, { Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.0", + Name: "prometheus.v1.0.0", Package: "prometheus", Image: "quay.io/operatorhubio/prometheus@fake1.0.0", Properties: []property.Property{ @@ -1470,7 +1408,7 @@ var testBundleList = []*catalogmetadata.Bundle{ }, { Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.1", + Name: "prometheus.v1.0.1", Package: "prometheus", Image: "quay.io/operatorhubio/prometheus@fake1.0.1", Properties: []property.Property{ @@ -1483,7 +1421,7 @@ var testBundleList = []*catalogmetadata.Bundle{ }, { Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.2.0", + Name: "prometheus.v1.2.0", Package: "prometheus", Image: "quay.io/operatorhubio/prometheus@fake1.2.0", Properties: []property.Property{ @@ -1496,7 +1434,7 @@ var testBundleList = []*catalogmetadata.Bundle{ }, { Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/2.0.0", + Name: "prometheus.v2.0.0", Package: "prometheus", Image: "quay.io/operatorhubio/prometheus@fake2.0.0", Properties: []property.Property{ diff --git a/internal/controllers/clusterextension_registryv1_validation_test.go b/internal/controllers/clusterextension_registryv1_validation_test.go index a828a3e05..298bd3def 100644 --- a/internal/controllers/clusterextension_registryv1_validation_test.go +++ b/internal/controllers/clusterextension_registryv1_validation_test.go @@ -109,7 +109,6 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { }, nil) // Create and configure the mock InstalledBundleGetter mockInstalledBundleGetter := &MockInstalledBundleGetter{} - mockInstalledBundleGetter.SetBundle(tt.bundle) reconciler := &controllers.ClusterExtensionReconciler{ Client: cl, diff --git a/internal/controllers/successors.go b/internal/controllers/successors.go index 04283d70f..0076b7c20 100644 --- a/internal/controllers/successors.go +++ b/internal/controllers/successors.go @@ -5,18 +5,19 @@ import ( mmsemver "github.com/Masterminds/semver/v3" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" "github.com/operator-framework/operator-controller/pkg/features" ) -func SuccessorsPredicate(installedBundle *catalogmetadata.Bundle) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { +func SuccessorsPredicate(packageName string, installedBundle *ocv1alpha1.BundleMetadata) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { var successors successorsPredicateFunc = legacySemanticsSuccessorsPredicate if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) { successors = semverSuccessorsPredicate } - installedBundleVersion, err := installedBundle.Version() + installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) if err != nil { return nil, err } @@ -26,7 +27,7 @@ func SuccessorsPredicate(installedBundle *catalogmetadata.Bundle) (catalogfilter return nil, err } - successorsPredicate, err := successors(installedBundle) + successorsPredicate, err := successors(packageName, installedBundle) if err != nil { return nil, err } @@ -35,7 +36,7 @@ func SuccessorsPredicate(installedBundle *catalogmetadata.Bundle) (catalogfilter return catalogfilter.Or( successorsPredicate, catalogfilter.And( - catalogfilter.WithPackageName(installedBundle.Package), + catalogfilter.WithPackageName(packageName), catalogfilter.InMastermindsSemverRange(installedVersionConstraint), ), ), nil @@ -43,14 +44,14 @@ func SuccessorsPredicate(installedBundle *catalogmetadata.Bundle) (catalogfilter // successorsPredicateFunc returns a predicate to find successors // for a bundle. Predicate must not include the current version. -type successorsPredicateFunc func(bundle *catalogmetadata.Bundle) (catalogfilter.Predicate[catalogmetadata.Bundle], error) +type successorsPredicateFunc func(packageName string, bundle *ocv1alpha1.BundleMetadata) (catalogfilter.Predicate[catalogmetadata.Bundle], error) // legacySemanticsSuccessorsPredicate returns a predicate to find successors // based on legacy OLMv0 semantics which rely on Replaces, Skips and skipRange. -func legacySemanticsSuccessorsPredicate(bundle *catalogmetadata.Bundle) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { +func legacySemanticsSuccessorsPredicate(packageName string, bundle *ocv1alpha1.BundleMetadata) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { // find the bundles that replace, skip, or skipRange the bundle provided return catalogfilter.And( - catalogfilter.WithPackageName(bundle.Package), + catalogfilter.WithPackageName(packageName), catalogfilter.LegacySuccessor(bundle), ), nil } @@ -58,8 +59,8 @@ func legacySemanticsSuccessorsPredicate(bundle *catalogmetadata.Bundle) (catalog // semverSuccessorsPredicate 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. -func semverSuccessorsPredicate(bundle *catalogmetadata.Bundle) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { - currentVersion, err := bundle.Version() +func semverSuccessorsPredicate(packageName string, bundle *ocv1alpha1.BundleMetadata) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { + currentVersion, err := mmsemver.NewVersion(bundle.Version) if err != nil { return nil, err } @@ -73,7 +74,7 @@ func semverSuccessorsPredicate(bundle *catalogmetadata.Bundle) (catalogfilter.Pr } return catalogfilter.And( - catalogfilter.WithPackageName(bundle.Package), + catalogfilter.WithPackageName(packageName), catalogfilter.InMastermindsSemverRange(wantedVersionRangeConstraint), ), nil } diff --git a/internal/controllers/successors_test.go b/internal/controllers/successors_test.go index cda5261ad..946dc2a19 100644 --- a/internal/controllers/successors_test.go +++ b/internal/controllers/successors_test.go @@ -14,6 +14,7 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "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" @@ -172,12 +173,12 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing. for _, tt := range []struct { name string - installedBundle *catalogmetadata.Bundle + installedBundle *ocv1alpha1.BundleMetadata expectedResult []*catalogmetadata.Bundle }{ { name: "with non-zero major version", - installedBundle: bundleSet["test-package.v2.0.0"], + installedBundle: bundleMetadataFor(bundleSet["test-package.v2.0.0"]), expectedResult: []*catalogmetadata.Bundle{ // Updates are allowed within the major version bundleSet["test-package.v2.2.0"], @@ -187,7 +188,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing. }, { name: "with zero major and zero minor version", - installedBundle: bundleSet["test-package.v0.0.1"], + installedBundle: bundleMetadataFor(bundleSet["test-package.v0.0.1"]), expectedResult: []*catalogmetadata.Bundle{ // No updates are allowed in major version zero when minor version is also zero bundleSet["test-package.v0.0.1"], @@ -195,7 +196,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing. }, { name: "with zero major and non-zero minor version", - installedBundle: bundleSet["test-package.v0.1.0"], + installedBundle: bundleMetadataFor(bundleSet["test-package.v0.1.0"]), expectedResult: []*catalogmetadata.Bundle{ // Patch version updates are allowed within the minor version bundleSet["test-package.v0.1.2"], @@ -205,22 +206,15 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing. }, { name: "installed bundle not found", - 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}, + installedBundle: &ocv1alpha1.BundleMetadata{ + Name: "test-package.v9.0.0", + Version: "9.0.0", }, expectedResult: []*catalogmetadata.Bundle{}, }, } { t.Run(tt.name, func(t *testing.T) { - successors, err := controllers.SuccessorsPredicate(tt.installedBundle) + successors, err := controllers.SuccessorsPredicate("test-package", tt.installedBundle) assert.NoError(t, err) result := catalogfilter.Filter(allBundles, successors) @@ -369,12 +363,12 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing for _, tt := range []struct { name string - installedBundle *catalogmetadata.Bundle + installedBundle *ocv1alpha1.BundleMetadata expectedResult []*catalogmetadata.Bundle }{ { name: "respect replaces directive from catalog", - installedBundle: bundleSet["test-package.v2.0.0"], + installedBundle: bundleMetadataFor(bundleSet["test-package.v2.0.0"]), expectedResult: []*catalogmetadata.Bundle{ // Must only have two bundle: // - the one which replaces the current version @@ -385,7 +379,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing }, { name: "respect skips directive from catalog", - installedBundle: bundleSet["test-package.v2.2.1"], + installedBundle: bundleMetadataFor(bundleSet["test-package.v2.2.1"]), expectedResult: []*catalogmetadata.Bundle{ // Must only have two bundle: // - the one which skips the current version @@ -396,7 +390,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing }, { name: "respect skipRange directive from catalog", - installedBundle: bundleSet["test-package.v2.3.0"], + installedBundle: bundleMetadataFor(bundleSet["test-package.v2.3.0"]), expectedResult: []*catalogmetadata.Bundle{ // Must only have two bundle: // - the one which is skipRanges the current version @@ -407,22 +401,15 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing }, { name: "installed bundle not found", - 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}, + installedBundle: &ocv1alpha1.BundleMetadata{ + Name: "test-package.v9.0.0", + Version: "9.0.0", }, expectedResult: []*catalogmetadata.Bundle{}, }, } { t.Run(tt.name, func(t *testing.T) { - successors, err := controllers.SuccessorsPredicate(tt.installedBundle) + successors, err := controllers.SuccessorsPredicate("test-package", tt.installedBundle) assert.NoError(t, err) result := catalogfilter.Filter(allBundles, successors) @@ -438,3 +425,14 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing }) } } + +func bundleMetadataFor(bundle *catalogmetadata.Bundle) *ocv1alpha1.BundleMetadata { + if bundle == nil { + return nil + } + v, _ := bundle.Version() + return &ocv1alpha1.BundleMetadata{ + Name: bundle.Name, + Version: v.String(), + } +} diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 6de9560ef..acb325acd 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -39,7 +39,6 @@ import ( "github.com/operator-framework/rukpak/pkg/storage" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/controllers" "github.com/operator-framework/operator-controller/pkg/scheme" testutil "github.com/operator-framework/operator-controller/test/util" @@ -97,18 +96,18 @@ func newClient(t *testing.T) client.Client { } type MockInstalledBundleGetter struct { - bundle *catalogmetadata.Bundle + bundle *ocv1alpha1.BundleMetadata } -func (m *MockInstalledBundleGetter) SetBundle(bundle *catalogmetadata.Bundle) { +func (m *MockInstalledBundleGetter) SetBundle(bundle *ocv1alpha1.BundleMetadata) { m.bundle = bundle } -func (m *MockInstalledBundleGetter) GetInstalledBundle(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { +func (m *MockInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error) { return m.bundle, nil } -func newClientAndReconciler(t *testing.T, bundle *catalogmetadata.Bundle) (client.Client, *controllers.ClusterExtensionReconciler) { +func newClientAndReconciler(t *testing.T, bundle *ocv1alpha1.BundleMetadata) (client.Client, *controllers.ClusterExtensionReconciler) { cl := newClient(t) fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList)