Skip to content

Commit b264baa

Browse files
committed
Fix: installed bundle provider
The installed bundle provider, which is necessary for safely handling package upgrades, was too strict. It required the currently installed bundle to exist in the catalog in order to "find" the installed bundle. This is problematic in several situations: - If a user receives a catalog update that no longer contains their installed bundle (perhaps its was pulled from the catalog for security or policy reasons) - If a user is trying to transition to a different catalog that provides that package - If a bundle was directly installed, and the user is trying to have a catalog-backed ClusterExtension adopt it. This change simply returns the name and version of the installed bundle, and makes some minor changes in the successors functions to handle the simplified installed bundle metadata.
1 parent 35e5087 commit b264baa

File tree

8 files changed

+128
-205
lines changed

8 files changed

+128
-205
lines changed

internal/catalogmetadata/filter/bundle_predicates.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/operator-framework/operator-registry/alpha/declcfg"
88

9+
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
910
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1011
)
1112

@@ -66,7 +67,7 @@ func WithBundleName(bundleName string) Predicate[catalogmetadata.Bundle] {
6667
}
6768
}
6869

69-
func LegacySuccessor(installedBundle *catalogmetadata.Bundle) Predicate[catalogmetadata.Bundle] {
70+
func LegacySuccessor(installedBundle *ocv1alpha1.BundleMetadata) Predicate[catalogmetadata.Bundle] {
7071
isSuccessor := func(candidateBundleEntry declcfg.ChannelEntry) bool {
7172
if candidateBundleEntry.Replaces == installedBundle.Name {
7273
return true
@@ -77,9 +78,9 @@ func LegacySuccessor(installedBundle *catalogmetadata.Bundle) Predicate[catalogm
7778
}
7879
}
7980
if candidateBundleEntry.SkipRange != "" {
80-
installedBundleVersion, _ := installedBundle.Version()
81-
skipRange, _ := bsemver.ParseRange(candidateBundleEntry.SkipRange)
82-
if installedBundleVersion != nil && skipRange != nil && skipRange(*installedBundleVersion) {
81+
installedBundleVersion, vErr := bsemver.Parse(installedBundle.Version)
82+
skipRange, srErr := bsemver.ParseRange(candidateBundleEntry.SkipRange)
83+
if vErr == nil && srErr == nil && skipRange(installedBundleVersion) {
8384
return true
8485
}
8586
}

internal/catalogmetadata/filter/bundle_predicates_test.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/operator-framework/operator-registry/alpha/declcfg"
1212
"github.com/operator-framework/operator-registry/alpha/property"
1313

14+
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
1415
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1516
"github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
1617
)
@@ -150,13 +151,9 @@ func TestLegacySuccessor(t *testing.T) {
150151
},
151152
},
152153
}
153-
installedBundle := &catalogmetadata.Bundle{
154-
Bundle: declcfg.Bundle{
155-
Name: "package1.v0.0.1",
156-
Properties: []property.Property{
157-
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package1", "version": "0.0.1"}`)},
158-
},
159-
},
154+
installedBundle := &ocv1alpha1.BundleMetadata{
155+
Name: "package1.v0.0.1",
156+
Version: "0.0.1",
160157
}
161158

162159
b2 := &catalogmetadata.Bundle{

internal/controllers/clusterextension_controller.go

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ type ClusterExtensionReconciler struct {
9494
}
9595

9696
type InstalledBundleGetter interface {
97-
GetInstalledBundle(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error)
97+
GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error)
9898
}
9999

100100
//+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
370370
channelName := ext.Spec.Channel
371371
versionRange := ext.Spec.Version
372372

373-
installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, r.ActionClientGetter, allBundles, &ext)
373+
installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, &ext)
374374
if err != nil {
375375
return nil, err
376376
}
@@ -392,7 +392,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1
392392
}
393393

394394
if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore && installedBundle != nil {
395-
upgradePredicate, err := SuccessorsPredicate(installedBundle)
395+
upgradePredicate, err := SuccessorsPredicate(ext.Spec.PackageName, installedBundle)
396396
if err != nil {
397397
return nil, err
398398
}
@@ -404,7 +404,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1
404404

405405
var upgradeErrorPrefix string
406406
if installedBundle != nil {
407-
installedBundleVersion, err := installedBundle.Version()
407+
installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version)
408408
if err != nil {
409409
return nil, err
410410
}
@@ -568,6 +568,11 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error {
568568
r.controller = controller
569569
r.cache = mgr.GetCache()
570570
r.dynamicWatchGVKs = map[schema.GroupVersionKind]struct{}{}
571+
572+
if r.InstalledBundleGetter == nil {
573+
r.InstalledBundleGetter = &DefaultInstalledBundleGetter{r.ActionClientGetter}
574+
}
575+
571576
return nil
572577
}
573578

@@ -663,10 +668,12 @@ func (r *ClusterExtensionReconciler) getReleaseState(cl helmclient.ActionInterfa
663668
return currentRelease, stateUnchanged, nil
664669
}
665670

666-
type DefaultInstalledBundleGetter struct{}
671+
type DefaultInstalledBundleGetter struct {
672+
helmclient.ActionClientGetter
673+
}
667674

668-
func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) {
669-
cl, err := acg.ActionClientFor(ctx, ext)
675+
func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error) {
676+
cl, err := d.ActionClientFor(ctx, ext)
670677
if err != nil {
671678
return nil, err
672679
}
@@ -679,27 +686,10 @@ func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, a
679686
return nil, nil
680687
}
681688

682-
// Bundle must match installed version exactly
683-
vr, err := mmsemver.NewConstraint(release.Labels[labels.BundleVersionKey])
684-
if err != nil {
685-
return nil, err
686-
}
687-
688-
// find corresponding bundle for the installed content
689-
resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(
690-
catalogfilter.WithPackageName(release.Labels[labels.PackageNameKey]),
691-
catalogfilter.WithBundleName(release.Labels[labels.BundleNameKey]),
692-
catalogfilter.InMastermindsSemverRange(vr),
693-
))
694-
if len(resultSet) == 0 {
695-
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)
696-
}
697-
698-
sort.SliceStable(resultSet, func(i, j int) bool {
699-
return catalogsort.ByVersion(resultSet[i], resultSet[j])
700-
})
701-
702-
return resultSet[0], nil
689+
return &ocv1alpha1.BundleMetadata{
690+
Name: release.Labels[labels.BundleNameKey],
691+
Version: release.Labels[labels.BundleVersionKey],
692+
}, nil
703693
}
704694

705695
type postrenderer struct {
@@ -749,7 +739,7 @@ func bundleMetadataFor(bundle *catalogmetadata.Bundle) *ocv1alpha1.BundleMetadat
749739
}
750740

751741
func (r *ClusterExtensionReconciler) validateBundle(bundle *catalogmetadata.Bundle) error {
752-
unsupportedProps := sets.New(
742+
unsupportedProps := sets.New[string](
753743
property.TypePackageRequired,
754744
property.TypeGVKRequired,
755745
property.TypeConstraint,

0 commit comments

Comments
 (0)