diff --git a/internal/controllers/variable_source.go b/internal/controllers/variable_source.go index 88608cdfb..c1976a1cc 100644 --- a/internal/controllers/variable_source.go +++ b/internal/controllers/variable_source.go @@ -17,26 +17,68 @@ limitations under the License. package controllers import ( + "context" + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" + rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" + operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func NewVariableSource(cl client.Client, catalogClient variablesources.BundleProvider) variablesources.NestedVariableSource { - return variablesources.NestedVariableSource{ +// BundleProvider provides the way to retrieve a list of Bundles from a source, +// generally from a catalog client of some kind. +type BundleProvider interface { + Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) +} + +type VariableSource struct { + client client.Client + catalogClient BundleProvider +} + +func NewVariableSource(cl client.Client, catalogClient BundleProvider) *VariableSource { + return &VariableSource{ + client: cl, + catalogClient: catalogClient, + } +} + +func (v *VariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { + operatorList := operatorsv1alpha1.OperatorList{} + if err := v.client.List(ctx, &operatorList); err != nil { + return nil, err + } + + bundleDeploymentList := rukpakv1alpha1.BundleDeploymentList{} + if err := v.client.List(ctx, &bundleDeploymentList); err != nil { + return nil, err + } + + allBundles, err := v.catalogClient.Bundles(ctx) + if err != nil { + return nil, err + } + + // We are in process of getting rid of extra variable sources. + // See this for progress: https://github.com/operator-framework/operator-controller/issues/437 + vs := variablesources.NestedVariableSource{ func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewOperatorVariableSource(cl, catalogClient, inputVariableSource), nil + return variablesources.NewOperatorVariableSource(operatorList.Items, allBundles, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewBundleDeploymentVariableSource(cl, catalogClient, inputVariableSource), nil + return variablesources.NewBundleDeploymentVariableSource(bundleDeploymentList.Items, allBundles, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewBundlesAndDepsVariableSource(catalogClient, inputVariableSource), nil + return variablesources.NewBundlesAndDepsVariableSource(allBundles, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { return variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource), nil }, } + return vs.GetVariables(ctx) } diff --git a/internal/resolution/variablesources/bundle_deployment.go b/internal/resolution/variablesources/bundle_deployment.go index 95fe55d59..1cb4502ae 100644 --- a/internal/resolution/variablesources/bundle_deployment.go +++ b/internal/resolution/variablesources/bundle_deployment.go @@ -5,23 +5,23 @@ import ( "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" "k8s.io/apimachinery/pkg/util/sets" - "sigs.k8s.io/controller-runtime/pkg/client" ) var _ input.VariableSource = &BundleDeploymentVariableSource{} type BundleDeploymentVariableSource struct { - client client.Client - catalogClient BundleProvider + bundleDeployments []rukpakv1alpha1.BundleDeployment + allBundles []*catalogmetadata.Bundle inputVariableSource input.VariableSource } -func NewBundleDeploymentVariableSource(cl client.Client, catalogClient BundleProvider, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource { +func NewBundleDeploymentVariableSource(bundleDeployments []rukpakv1alpha1.BundleDeployment, allBundles []*catalogmetadata.Bundle, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource { return &BundleDeploymentVariableSource{ - client: cl, - catalogClient: catalogClient, + bundleDeployments: bundleDeployments, + allBundles: allBundles, inputVariableSource: inputVariableSource, } } @@ -32,20 +32,15 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context) ([]de variableSources = append(variableSources, o.inputVariableSource) } - bundleDeployments := rukpakv1alpha1.BundleDeploymentList{} - if err := o.client.List(ctx, &bundleDeployments); err != nil { - return nil, err - } - processed := sets.Set[string]{} - for _, bundleDeployment := range bundleDeployments.Items { + for _, bundleDeployment := range o.bundleDeployments { 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) + ips, err := NewInstalledPackageVariableSource(o.allBundles, bundleDeployment.Spec.Template.Spec.Source.Image.Ref) if err != nil { return nil, err } diff --git a/internal/resolution/variablesources/bundle_deployment_test.go b/internal/resolution/variablesources/bundle_deployment_test.go index 0e640210e..1010d0743 100644 --- a/internal/resolution/variablesources/bundle_deployment_test.go +++ b/internal/resolution/variablesources/bundle_deployment_test.go @@ -13,26 +13,15 @@ import ( "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" - testutil "github.com/operator-framework/operator-controller/test/util" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/operator-framework/deppy/pkg/deppy" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" ) -func BundleDeploymentFakeClient(objects ...client.Object) client.Client { - scheme := runtime.NewScheme() - utilruntime.Must(rukpakv1alpha1.AddToScheme(scheme)) - return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() -} - -func bundleDeployment(name, image string) *rukpakv1alpha1.BundleDeployment { - return &rukpakv1alpha1.BundleDeployment{ +func bundleDeployment(name, image string) rukpakv1alpha1.BundleDeployment { + return rukpakv1alpha1.BundleDeployment{ ObjectMeta: metav1.ObjectMeta{ Name: name, }, @@ -53,7 +42,6 @@ func bundleDeployment(name, image string) *rukpakv1alpha1.BundleDeployment { } var _ = Describe("BundleDeploymentVariableSource", func() { - var fakeCatalogClient testutil.FakeCatalogClient var betaChannel catalogmetadata.Channel var stableChannel catalogmetadata.Channel var testBundleList []*catalogmetadata.Bundle @@ -111,14 +99,14 @@ var _ = Describe("BundleDeploymentVariableSource", func() { }, }, InChannels: []*catalogmetadata.Channel{&stableChannel}}, } - - fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList) }) It("should produce RequiredPackage variables", func() { - cl := BundleDeploymentFakeClient(bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35")) + bundleDeployments := []rukpakv1alpha1.BundleDeployment{ + bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"), + } - bdVariableSource := variablesources.NewBundleDeploymentVariableSource(cl, &fakeCatalogClient, &MockRequiredPackageSource{}) + bdVariableSource := variablesources.NewBundleDeploymentVariableSource(bundleDeployments, testBundleList, &MockRequiredPackageSource{}) variables, err := bdVariableSource.GetVariables(context.Background()) Expect(err).ToNot(HaveOccurred()) @@ -137,9 +125,11 @@ var _ = Describe("BundleDeploymentVariableSource", func() { }))) }) It("should return an error if the bundleDeployment image doesn't match any operator resource", func() { - cl := BundleDeploymentFakeClient(bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent")) + bundleDeployments := []rukpakv1alpha1.BundleDeployment{ + bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent"), + } - bdVariableSource := variablesources.NewBundleDeploymentVariableSource(cl, &fakeCatalogClient, &MockRequiredPackageSource{}) + bdVariableSource := variablesources.NewBundleDeploymentVariableSource(bundleDeployments, testBundleList, &MockRequiredPackageSource{}) _, err := bdVariableSource.GetVariables(context.Background()) Expect(err.Error()).To(Equal("bundleImage \"quay.io/operatorhubio/prometheus@sha256:nonexistent\" not found")) }) diff --git a/internal/resolution/variablesources/bundle_provider.go b/internal/resolution/variablesources/bundle_provider.go deleted file mode 100644 index c6517069a..000000000 --- a/internal/resolution/variablesources/bundle_provider.go +++ /dev/null @@ -1,14 +0,0 @@ -package variablesources - -import ( - "context" - - "github.com/operator-framework/operator-controller/internal/catalogmetadata" -) - -// BundleProvider provides the Bundles method through which we can retrieve -// a list of Bundles from any source, generally from a catalog client of -// some kind. -type BundleProvider interface { - Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) -} diff --git a/internal/resolution/variablesources/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go index 88eaafc94..0288537a9 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies.go +++ b/internal/resolution/variablesources/bundles_and_dependencies.go @@ -18,13 +18,13 @@ import ( var _ input.VariableSource = &BundlesAndDepsVariableSource{} type BundlesAndDepsVariableSource struct { - catalogClient BundleProvider + allBundles []*catalogmetadata.Bundle variableSources []input.VariableSource } -func NewBundlesAndDepsVariableSource(catalogClient BundleProvider, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource { +func NewBundlesAndDepsVariableSource(allBundles []*catalogmetadata.Bundle, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource { return &BundlesAndDepsVariableSource{ - catalogClient: catalogClient, + allBundles: allBundles, variableSources: inputVariableSources, } } @@ -52,11 +52,6 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp } } - allBundles, err := b.catalogClient.Bundles(ctx) - if err != nil { - return nil, err - } - // build bundle and dependency variables visited := sets.Set[deppy.Identifier]{} for len(bundleQueue) > 0 { @@ -73,7 +68,7 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp visited.Insert(id) // get bundle dependencies - dependencies, err := b.filterBundleDependencies(allBundles, head) + dependencies, err := b.filterBundleDependencies(b.allBundles, head) if err != nil { return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err) } diff --git a/internal/resolution/variablesources/bundles_and_dependencies_test.go b/internal/resolution/variablesources/bundles_and_dependencies_test.go index 1c71a8ada..b3d4a6c5a 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies_test.go +++ b/internal/resolution/variablesources/bundles_and_dependencies_test.go @@ -14,14 +14,12 @@ import ( "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" - testutil "github.com/operator-framework/operator-controller/test/util" ) var _ = Describe("BundlesAndDepsVariableSource", func() { var ( - bdvs *variablesources.BundlesAndDepsVariableSource - testBundleList []*catalogmetadata.Bundle - fakeCatalogClient testutil.FakeCatalogClient + bdvs *variablesources.BundlesAndDepsVariableSource + testBundleList []*catalogmetadata.Bundle ) BeforeEach(func() { @@ -222,12 +220,11 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { InChannels: []*catalogmetadata.Channel{&channel}, }, } - fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList) bdvs = variablesources.NewBundlesAndDepsVariableSource( - &fakeCatalogClient, + testBundleList, &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ - // must match data in fakeCatalogClient + // must match data in testBundleList olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{ { CatalogName: "fake-catalog", @@ -259,7 +256,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { }, &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ - // must match data in fakeCatalogClient + // must match data in testBundleList olmvariables.NewRequiredPackageVariable("test-package-2", []*catalogmetadata.Bundle{ // test-package-2 required package - no dependencies { @@ -335,13 +332,10 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { }) It("should return error if dependencies not found", func() { - emptyCatalogClient := testutil.NewFakeCatalogClient(make([]*catalogmetadata.Bundle, 0)) - bdvs = variablesources.NewBundlesAndDepsVariableSource( - &emptyCatalogClient, + []*catalogmetadata.Bundle{}, &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ - // must match data in fakeCatalogClient olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{ { CatalogName: "fake-catalog", @@ -379,7 +373,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { It("should return error if an inner variable source returns an error", func() { bdvs = variablesources.NewBundlesAndDepsVariableSource( - &fakeCatalogClient, + testBundleList, &MockRequiredPackageSource{Error: errors.New("fake error")}, ) _, err := bdvs.GetVariables(context.TODO()) diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index 071afaa73..f2f272f2b 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -19,19 +19,14 @@ import ( var _ input.VariableSource = &InstalledPackageVariableSource{} type InstalledPackageVariableSource struct { - catalogClient BundleProvider - successors successorsFunc - bundleImage string + allBundles []*catalogmetadata.Bundle + 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)) + resultSet := catalogfilter.Filter(r.allBundles, catalogfilter.WithBundleImage(r.bundleImage)) if len(resultSet) == 0 { return nil, r.notFoundError() } @@ -44,7 +39,7 @@ func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context) ([]de }) installedBundle := resultSet[0] - upgradeEdges, err := r.successors(allBundles, installedBundle) + upgradeEdges, err := r.successors(r.allBundles, installedBundle) if err != nil { return nil, err } @@ -60,16 +55,16 @@ func (r *InstalledPackageVariableSource) notFoundError() error { return fmt.Errorf("bundleImage %q not found", r.bundleImage) } -func NewInstalledPackageVariableSource(catalogClient BundleProvider, bundleImage string) (*InstalledPackageVariableSource, error) { +func NewInstalledPackageVariableSource(allBundles []*catalogmetadata.Bundle, bundleImage string) (*InstalledPackageVariableSource, error) { successors := legacySemanticsSuccessors if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) { successors = semverSuccessors } return &InstalledPackageVariableSource{ - catalogClient: catalogClient, - bundleImage: bundleImage, - successors: successors, + allBundles: allBundles, + bundleImage: bundleImage, + successors: successors, }, nil } diff --git a/internal/resolution/variablesources/installed_package_test.go b/internal/resolution/variablesources/installed_package_test.go index 23b785126..77e76e59f 100644 --- a/internal/resolution/variablesources/installed_package_test.go +++ b/internal/resolution/variablesources/installed_package_test.go @@ -16,7 +16,6 @@ import ( 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) { @@ -202,14 +201,12 @@ func TestInstalledPackageVariableSource(t *testing.T) { }, } - fakeCatalogClient := testutil.NewFakeCatalogClient(bundleList) - 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) + ipvs, err := variablesources.NewInstalledPackageVariableSource(bundleList, bundleImage) require.NoError(t, err) variables, err := ipvs.GetVariables(context.TODO()) @@ -230,7 +227,7 @@ 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) + ipvs, err := variablesources.NewInstalledPackageVariableSource(bundleList, bundleImage) require.NoError(t, err) variables, err := ipvs.GetVariables(context.TODO()) @@ -248,7 +245,7 @@ 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) + ipvs, err := variablesources.NewInstalledPackageVariableSource(bundleList, bundleImage) require.NoError(t, err) variables, err := ipvs.GetVariables(context.TODO()) @@ -271,7 +268,7 @@ 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) + ipvs, err := variablesources.NewInstalledPackageVariableSource(bundleList, bundleImage) require.NoError(t, err) variables, err := ipvs.GetVariables(context.TODO()) diff --git a/internal/resolution/variablesources/operator.go b/internal/resolution/variablesources/operator.go index 19cc88443..06de4ec34 100644 --- a/internal/resolution/variablesources/operator.go +++ b/internal/resolution/variablesources/operator.go @@ -4,24 +4,24 @@ import ( "context" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - "sigs.k8s.io/controller-runtime/pkg/client" ) var _ input.VariableSource = &OperatorVariableSource{} type OperatorVariableSource struct { - client client.Client - catalogClient BundleProvider + operators []operatorsv1alpha1.Operator + allBundles []*catalogmetadata.Bundle inputVariableSource input.VariableSource } -func NewOperatorVariableSource(cl client.Client, catalogClient BundleProvider, inputVariableSource input.VariableSource) *OperatorVariableSource { +func NewOperatorVariableSource(operators []operatorsv1alpha1.Operator, allBundles []*catalogmetadata.Bundle, inputVariableSource input.VariableSource) *OperatorVariableSource { return &OperatorVariableSource{ - client: cl, - catalogClient: catalogClient, + operators: operators, + allBundles: allBundles, inputVariableSource: inputVariableSource, } } @@ -32,15 +32,10 @@ func (o *OperatorVariableSource) GetVariables(ctx context.Context) ([]deppy.Vari variableSources = append(variableSources, o.inputVariableSource) } - operatorList := operatorsv1alpha1.OperatorList{} - if err := o.client.List(ctx, &operatorList); err != nil { - return nil, err - } - // build required package variable sources - for _, operator := range operatorList.Items { + for _, operator := range o.operators { rps, err := NewRequiredPackageVariableSource( - o.catalogClient, + o.allBundles, operator.Spec.PackageName, InVersionRange(operator.Spec.Version), InChannel(operator.Spec.Channel), diff --git a/internal/resolution/variablesources/operator_test.go b/internal/resolution/variablesources/operator_test.go index 426cf82c2..1e8d2fbe4 100644 --- a/internal/resolution/variablesources/operator_test.go +++ b/internal/resolution/variablesources/operator_test.go @@ -3,7 +3,6 @@ package variablesources_test import ( "context" "encoding/json" - "errors" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/operator-registry/alpha/declcfg" @@ -21,7 +20,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - testutil "github.com/operator-framework/operator-controller/test/util" "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" @@ -34,8 +32,8 @@ func FakeClient(objects ...client.Object) client.Client { return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() } -func operator(name string) *operatorsv1alpha1.Operator { - return &operatorsv1alpha1.Operator{ +func operator(name string) operatorsv1alpha1.Operator { + return operatorsv1alpha1.Operator{ ObjectMeta: metav1.ObjectMeta{ Name: name, }, @@ -114,9 +112,11 @@ var _ = Describe("OperatorVariableSource", func() { }) It("should produce RequiredPackage variables", func() { - cl := FakeClient(operator("prometheus"), operator("packageA")) - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - opVariableSource := variablesources.NewOperatorVariableSource(cl, &fakeCatalogClient, &MockRequiredPackageSource{}) + operators := []operatorsv1alpha1.Operator{ + operator("prometheus"), + operator("packageA"), + } + opVariableSource := variablesources.NewOperatorVariableSource(operators, testBundleList, &MockRequiredPackageSource{}) variables, err := opVariableSource.GetVariables(context.Background()) Expect(err).ToNot(HaveOccurred()) @@ -133,15 +133,6 @@ var _ = Describe("OperatorVariableSource", func() { deppy.IdentifierFromString("required package packageA"): 1, }))) }) - - It("should return an errors when they occur", func() { - cl := FakeClient(operator("prometheus"), operator("packageA")) - fakeCatalogClient := testutil.NewFakeCatalogClientWithError(errors.New("something bad happened")) - - opVariableSource := variablesources.NewOperatorVariableSource(cl, &fakeCatalogClient, nil) - _, err := opVariableSource.GetVariables(context.Background()) - Expect(err).To(HaveOccurred()) - }) }) func filterVariables[D deppy.Variable](variables []deppy.Variable) []D { diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index e45df4e17..b223e3bac 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -46,7 +46,7 @@ func InChannel(channelName string) RequiredPackageVariableSourceOption { } type RequiredPackageVariableSource struct { - catalogClient BundleProvider + allBundles []*catalogmetadata.Bundle packageName string versionRange string @@ -54,12 +54,12 @@ type RequiredPackageVariableSource struct { predicates []catalogfilter.Predicate[catalogmetadata.Bundle] } -func NewRequiredPackageVariableSource(catalogClient BundleProvider, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { +func NewRequiredPackageVariableSource(allBundles []*catalogmetadata.Bundle, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { if packageName == "" { return nil, fmt.Errorf("package name must not be empty") } r := &RequiredPackageVariableSource{ - catalogClient: catalogClient, + allBundles: allBundles, packageName: packageName, predicates: []catalogfilter.Predicate[catalogmetadata.Bundle]{catalogfilter.WithPackageName(packageName)}, @@ -73,12 +73,7 @@ func NewRequiredPackageVariableSource(catalogClient BundleProvider, packageName } func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { - resultSet, err := r.catalogClient.Bundles(ctx) - if err != nil { - return nil, err - } - - resultSet = catalogfilter.Filter(resultSet, catalogfilter.And(r.predicates...)) + resultSet := catalogfilter.Filter(r.allBundles, catalogfilter.And(r.predicates...)) if len(resultSet) == 0 { return nil, r.notFoundError() } diff --git a/internal/resolution/variablesources/required_package_test.go b/internal/resolution/variablesources/required_package_test.go index a6f0bd32a..27723938d 100644 --- a/internal/resolution/variablesources/required_package_test.go +++ b/internal/resolution/variablesources/required_package_test.go @@ -3,7 +3,6 @@ package variablesources_test import ( "context" "encoding/json" - "errors" "fmt" . "github.com/onsi/ginkgo/v2" @@ -15,14 +14,13 @@ import ( "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" - testutil "github.com/operator-framework/operator-controller/test/util" ) var _ = Describe("RequiredPackageVariableSource", func() { var ( - rpvs *variablesources.RequiredPackageVariableSource - fakeCatalogClient testutil.FakeCatalogClient - packageName string + rpvs *variablesources.RequiredPackageVariableSource + bundleList []*catalogmetadata.Bundle + packageName string ) BeforeEach(func() { @@ -31,7 +29,7 @@ var _ = Describe("RequiredPackageVariableSource", func() { channel := catalogmetadata.Channel{Channel: declcfg.Channel{ Name: "stable", }} - bundleList := []*catalogmetadata.Bundle{ + bundleList = []*catalogmetadata.Bundle{ {Bundle: declcfg.Bundle{ Name: "test-package.v1.0.0", Package: "test-package", @@ -74,8 +72,7 @@ var _ = Describe("RequiredPackageVariableSource", func() { InChannels: []*catalogmetadata.Channel{&channel}, }, } - fakeCatalogClient = testutil.NewFakeCatalogClient(bundleList) - rpvs, err = variablesources.NewRequiredPackageVariableSource(&fakeCatalogClient, packageName) + rpvs, err = variablesources.NewRequiredPackageVariableSource(bundleList, packageName) Expect(err).NotTo(HaveOccurred()) }) @@ -96,7 +93,7 @@ var _ = Describe("RequiredPackageVariableSource", func() { It("should filter by version range", func() { // recreate source with version range option var err error - rpvs, err = variablesources.NewRequiredPackageVariableSource(&fakeCatalogClient, packageName, variablesources.InVersionRange(">=1.0.0 !=2.0.0 <3.0.0")) + rpvs, err = variablesources.NewRequiredPackageVariableSource(bundleList, packageName, variablesources.InVersionRange(">=1.0.0 !=2.0.0 <3.0.0")) Expect(err).NotTo(HaveOccurred()) variables, err := rpvs.GetVariables(context.TODO()) @@ -112,26 +109,15 @@ var _ = Describe("RequiredPackageVariableSource", func() { }) It("should fail with bad semver range", func() { - _, err := variablesources.NewRequiredPackageVariableSource(&fakeCatalogClient, packageName, variablesources.InVersionRange("not a valid semver")) + _, err := variablesources.NewRequiredPackageVariableSource(bundleList, packageName, variablesources.InVersionRange("not a valid semver")) Expect(err).To(HaveOccurred()) }) It("should return an error if package not found", func() { - emptyCatalogClient := testutil.NewFakeCatalogClient([]*catalogmetadata.Bundle{}) - rpvs, err := variablesources.NewRequiredPackageVariableSource(&emptyCatalogClient, packageName) + rpvs, err := variablesources.NewRequiredPackageVariableSource([]*catalogmetadata.Bundle{}, packageName) Expect(err).NotTo(HaveOccurred()) _, err = rpvs.GetVariables(context.TODO()) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("no package 'test-package' found")) }) - - It("should return an error if catalog client errors", func() { - testError := errors.New("something bad happened") - emptyCatalogClient := testutil.NewFakeCatalogClientWithError(testError) - rpvs, err := variablesources.NewRequiredPackageVariableSource(&emptyCatalogClient, packageName, variablesources.InVersionRange(">=1.0.0 !=2.0.0 <3.0.0")) - Expect(err).NotTo(HaveOccurred()) - _, err = rpvs.GetVariables(context.TODO()) - Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError(testError)) - }) })