diff --git a/internal/resolution/variablesources/bundle_deployment.go b/internal/resolution/variablesources/bundle_deployment.go index 95fe55d59..671513bba 100644 --- a/internal/resolution/variablesources/bundle_deployment.go +++ b/internal/resolution/variablesources/bundle_deployment.go @@ -6,7 +6,6 @@ import ( "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" - "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -37,21 +36,23 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context) ([]de return nil, err } - processed := sets.Set[string]{} - for _, bundleDeployment := range bundleDeployments.Items { - sourceImage := bundleDeployment.Spec.Template.Spec.Source.Image - if sourceImage != nil && sourceImage.Ref != "" { - if processed.Has(sourceImage.Ref) { - continue - } - processed.Insert(sourceImage.Ref) - ips, err := NewInstalledPackageVariableSource(o.catalogClient, bundleDeployment.Spec.Template.Spec.Source.Image.Ref) - if err != nil { - return nil, err - } - variableSources = append(variableSources, ips) - } + allBundles, err := o.catalogClient.Bundles(ctx) + if err != nil { + return nil, err + } + + installedPackages, err := MakeInstalledPackageVariables(allBundles, bundleDeployments.Items) + if err != nil { + return nil, err } - return variableSources.GetVariables(ctx) + variables, err := variableSources.GetVariables(ctx) + if err != nil { + return nil, err + } + + for _, v := range installedPackages { + variables = append(variables, v) + } + return variables, nil } diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index 071afaa73..74df66e01 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -1,76 +1,71 @@ package variablesources import ( - "context" "fmt" "sort" mmsemver "github.com/Masterminds/semver/v3" - "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" + "k8s.io/apimachinery/pkg/util/sets" + + rukpakv1alpha1 "github.com/operator-framework/rukpak/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" - "github.com/operator-framework/operator-controller/internal/resolution/variables" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/pkg/features" ) -var _ input.VariableSource = &InstalledPackageVariableSource{} - -type InstalledPackageVariableSource struct { - catalogClient BundleProvider - successors successorsFunc - bundleImage string -} - -func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { - allBundles, err := r.catalogClient.Bundles(ctx) - if err != nil { - return nil, err - } - - // find corresponding bundle for the installed content - resultSet := catalogfilter.Filter(allBundles, catalogfilter.WithBundleImage(r.bundleImage)) - if len(resultSet) == 0 { - return nil, r.notFoundError() - } - - // TODO: fast follow - we should check whether we are already supporting the channel attribute in the operator spec. - // if so, we should take the value from spec of the operator CR in the owner ref of the bundle deployment. - // If that channel is set, we need to update the filter above to filter by channel as well. - sort.SliceStable(resultSet, func(i, j int) bool { - return catalogsort.ByVersion(resultSet[i], resultSet[j]) - }) - installedBundle := resultSet[0] - - upgradeEdges, err := r.successors(allBundles, installedBundle) - if err != nil { - return nil, err - } - - // you can always upgrade to yourself, i.e. not upgrade - upgradeEdges = append(upgradeEdges, installedBundle) - return []deppy.Variable{ - variables.NewInstalledPackageVariable(installedBundle.Package, upgradeEdges), - }, nil -} - -func (r *InstalledPackageVariableSource) notFoundError() error { - return fmt.Errorf("bundleImage %q not found", r.bundleImage) -} - -func NewInstalledPackageVariableSource(catalogClient BundleProvider, bundleImage string) (*InstalledPackageVariableSource, error) { - successors := legacySemanticsSuccessors +// MakeInstalledPackageVariables returns variables representing packages +// already installed in the system. +// Meaning that each BundleDeployment managed by operator-controller +// has own variable. +func MakeInstalledPackageVariables( + allBundles []*catalogmetadata.Bundle, + bundleDeployments []rukpakv1alpha1.BundleDeployment, +) ([]*olmvariables.InstalledPackageVariable, error) { + var successors successorsFunc = legacySemanticsSuccessors if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) { successors = semverSuccessors } - return &InstalledPackageVariableSource{ - catalogClient: catalogClient, - bundleImage: bundleImage, - successors: successors, - }, nil + result := make([]*olmvariables.InstalledPackageVariable, 0, len(bundleDeployments)) + processed := sets.Set[string]{} + for _, bundleDeployment := range bundleDeployments { + sourceImage := bundleDeployment.Spec.Template.Spec.Source.Image + if sourceImage == nil || sourceImage.Ref == "" { + continue + } + + if processed.Has(sourceImage.Ref) { + continue + } + processed.Insert(sourceImage.Ref) + + bundleImage := sourceImage.Ref + + // find corresponding bundle for the installed content + resultSet := catalogfilter.Filter(allBundles, catalogfilter.WithBundleImage(bundleImage)) + if len(resultSet) == 0 { + return nil, fmt.Errorf("bundleImage %q not found", bundleImage) + } + + sort.SliceStable(resultSet, func(i, j int) bool { + return catalogsort.ByVersion(resultSet[i], resultSet[j]) + }) + installedBundle := resultSet[0] + + upgradeEdges, err := successors(allBundles, installedBundle) + if err != nil { + return nil, err + } + + // you can always upgrade to yourself, i.e. not upgrade + upgradeEdges = append(upgradeEdges, installedBundle) + result = append(result, olmvariables.NewInstalledPackageVariable(installedBundle.Package, upgradeEdges)) + } + + return result, nil } // successorsFunc must return successors of a currently installed bundle diff --git a/internal/resolution/variablesources/installed_package_test.go b/internal/resolution/variablesources/installed_package_test.go index 23b785126..26bb92643 100644 --- a/internal/resolution/variablesources/installed_package_test.go +++ b/internal/resolution/variablesources/installed_package_test.go @@ -1,25 +1,26 @@ package variablesources_test import ( - "context" "encoding/json" + "fmt" "testing" - "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" featuregatetesting "k8s.io/component-base/featuregate/testing" + "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" - olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" "github.com/operator-framework/operator-controller/pkg/features" - testutil "github.com/operator-framework/operator-controller/test/util" ) -func TestInstalledPackageVariableSource(t *testing.T) { +func TestMakeInstalledPackageVariables(t *testing.T) { someOtherPackageChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ Name: "stable", Package: "some-other-package", @@ -82,7 +83,7 @@ func TestInstalledPackageVariableSource(t *testing.T) { }, }, }} - bundleList := []*catalogmetadata.Bundle{ + allBundles := []*catalogmetadata.Bundle{ {Bundle: declcfg.Bundle{ Name: "test-package.v0.0.1", Package: "test-package", @@ -202,21 +203,41 @@ func TestInstalledPackageVariableSource(t *testing.T) { }, } - fakeCatalogClient := testutil.NewFakeCatalogClient(bundleList) + fakeBundleDeployments := func(bundleImages ...string) []rukpakv1alpha1.BundleDeployment { + bundleDeployments := []rukpakv1alpha1.BundleDeployment{} + for idx, bundleImage := range bundleImages { + bd := rukpakv1alpha1.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("bd-%d", idx), + }, + Spec: rukpakv1alpha1.BundleDeploymentSpec{ + Template: &rukpakv1alpha1.BundleTemplate{ + Spec: rukpakv1alpha1.BundleSpec{ + Source: rukpakv1alpha1.BundleSource{ + Image: &rukpakv1alpha1.ImageSource{ + Ref: bundleImage, + }, + }, + }, + }, + }, + } + bundleDeployments = append(bundleDeployments, bd) + } + + 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" - ipvs, err := variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage) + installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, fakeBundleDeployments(bundleImage)) require.NoError(t, err) - variables, err := ipvs.GetVariables(context.TODO()) - require.NoError(t, err) - require.Len(t, variables, 1) - packageVariable, ok := variables[0].(*olmvariables.InstalledPackageVariable) - assert.True(t, ok) + 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) @@ -230,14 +251,11 @@ func TestInstalledPackageVariableSource(t *testing.T) { 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" - ipvs, err := variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage) + installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, fakeBundleDeployments(bundleImage)) require.NoError(t, err) - variables, err := ipvs.GetVariables(context.TODO()) - require.NoError(t, err) - require.Len(t, variables, 1) - packageVariable, ok := variables[0].(*olmvariables.InstalledPackageVariable) - assert.True(t, ok) + 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 @@ -248,14 +266,11 @@ func TestInstalledPackageVariableSource(t *testing.T) { t.Run("with non-zero minor version", func(t *testing.T) { const bundleImage = "registry.io/repo/test-package@v0.1.0" - ipvs, err := variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage) + installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, fakeBundleDeployments(bundleImage)) require.NoError(t, err) - variables, err := ipvs.GetVariables(context.TODO()) - require.NoError(t, err) - require.Len(t, variables, 1) - packageVariable, ok := variables[0].(*olmvariables.InstalledPackageVariable) - assert.True(t, ok) + 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 @@ -271,14 +286,11 @@ func TestInstalledPackageVariableSource(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() const bundleImage = "registry.io/repo/test-package@v2.0.0" - ipvs, err := variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage) + installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, fakeBundleDeployments(bundleImage)) require.NoError(t, err) - variables, err := ipvs.GetVariables(context.TODO()) - require.NoError(t, err) - require.Len(t, variables, 1) - packageVariable, ok := variables[0].(*olmvariables.InstalledPackageVariable) - assert.True(t, ok) + 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) @@ -287,4 +299,11 @@ func TestInstalledPackageVariableSource(t *testing.T) { assert.Equal(t, "test-package.v2.1.0", packageVariable.Bundles()[0].Name) assert.Equal(t, "test-package.v2.0.0", packageVariable.Bundles()[1].Name) }) + + t.Run("installed bundle not found", func(t *testing.T) { + const bundleImage = "registry.io/repo/test-package@v9.0.0" + installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, fakeBundleDeployments(bundleImage)) + assert.Nil(t, installedPackages) + assert.ErrorContains(t, err, `bundleImage "registry.io/repo/test-package@v9.0.0" not found`) + }) }