diff --git a/internal/resolution/variablesources/operator.go b/internal/resolution/variablesources/operator.go index 19cc88443..79dd37099 100644 --- a/internal/resolution/variablesources/operator.go +++ b/internal/resolution/variablesources/operator.go @@ -37,19 +37,23 @@ func (o *OperatorVariableSource) GetVariables(ctx context.Context) ([]deppy.Vari return nil, err } - // build required package variable sources - for _, operator := range operatorList.Items { - rps, err := NewRequiredPackageVariableSource( - o.catalogClient, - operator.Spec.PackageName, - InVersionRange(operator.Spec.Version), - InChannel(operator.Spec.Channel), - ) - if err != nil { - return nil, err - } - variableSources = append(variableSources, rps) + allBundles, err := o.catalogClient.Bundles(ctx) + if err != nil { + return nil, err + } + + requiredPackages, err := MakeRequiredPackageVariables(allBundles, operatorList.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 requiredPackages { + variables = append(variables, v) + } + return variables, nil } diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index e45df4e17..bdf45fb38 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -1,104 +1,64 @@ 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" + operatorsv1alpha1 "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" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) -var _ input.VariableSource = &RequiredPackageVariableSource{} +// MakeRequiredPackageVariables returns a variable which represent +// explicit requirement for a package from an user. +// This is when an user explicitly asks "install this" via Operator API. +func MakeRequiredPackageVariables(allBundles []*catalogmetadata.Bundle, operators []operatorsv1alpha1.Operator) ([]*olmvariables.RequiredPackageVariable, error) { + result := make([]*olmvariables.RequiredPackageVariable, 0, len(operators)) -type RequiredPackageVariableSourceOption func(*RequiredPackageVariableSource) error + for _, operator := range operators { + packageName := operator.Spec.PackageName + channelName := operator.Spec.Channel + versionRange := operator.Spec.Version -func InVersionRange(versionRange string) RequiredPackageVariableSourceOption { - return func(r *RequiredPackageVariableSource) error { - if versionRange != "" { - vr, err := mmsemver.NewConstraint(versionRange) - if err == nil { - r.versionRange = versionRange - r.predicates = append(r.predicates, catalogfilter.InMastermindsSemverRange(vr)) - return nil - } - - return fmt.Errorf("invalid version range '%s': %w", versionRange, err) + predicates := []catalogfilter.Predicate[catalogmetadata.Bundle]{ + catalogfilter.WithPackageName(packageName), } - return nil - } -} -func InChannel(channelName string) RequiredPackageVariableSourceOption { - return func(r *RequiredPackageVariableSource) error { if channelName != "" { - r.channelName = channelName - r.predicates = append(r.predicates, catalogfilter.InChannel(channelName)) + predicates = append(predicates, catalogfilter.InChannel(channelName)) } - return nil - } -} - -type RequiredPackageVariableSource struct { - catalogClient BundleProvider - - packageName string - versionRange string - channelName string - predicates []catalogfilter.Predicate[catalogmetadata.Bundle] -} -func NewRequiredPackageVariableSource(catalogClient BundleProvider, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { - if packageName == "" { - return nil, fmt.Errorf("package name must not be empty") - } - r := &RequiredPackageVariableSource{ - catalogClient: catalogClient, - - packageName: packageName, - predicates: []catalogfilter.Predicate[catalogmetadata.Bundle]{catalogfilter.WithPackageName(packageName)}, - } - for _, option := range options { - if err := option(r); err != nil { - return nil, err + if versionRange != "" { + vr, err := mmsemver.NewConstraint(versionRange) + if err != nil { + return nil, fmt.Errorf("invalid version range '%s': %w", versionRange, err) + } + predicates = append(predicates, catalogfilter.InMastermindsSemverRange(vr)) } - } - return r, nil -} -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(allBundles, catalogfilter.And(predicates...)) + if len(resultSet) == 0 { + if versionRange != "" && channelName != "" { + return nil, fmt.Errorf("no package '%s' matching version '%s' found in channel '%s'", packageName, versionRange, channelName) + } + if versionRange != "" { + return nil, fmt.Errorf("no package '%s' matching version '%s' found", packageName, versionRange) + } + if channelName != "" { + return nil, fmt.Errorf("no package '%s' found in channel '%s'", packageName, channelName) + } + return nil, fmt.Errorf("no package '%s' found", packageName) + } + sort.SliceStable(resultSet, func(i, j int) bool { + return catalogsort.ByVersion(resultSet[i], resultSet[j]) + }) - resultSet = catalogfilter.Filter(resultSet, catalogfilter.And(r.predicates...)) - if len(resultSet) == 0 { - return nil, r.notFoundError() + result = append(result, olmvariables.NewRequiredPackageVariable(packageName, resultSet)) } - sort.SliceStable(resultSet, func(i, j int) bool { - return catalogsort.ByVersion(resultSet[i], resultSet[j]) - }) - return []deppy.Variable{ - olmvariables.NewRequiredPackageVariable(r.packageName, resultSet), - }, nil -} -func (r *RequiredPackageVariableSource) notFoundError() error { - if r.versionRange != "" && r.channelName != "" { - return fmt.Errorf("no package '%s' matching version '%s' found in channel '%s'", r.packageName, r.versionRange, r.channelName) - } - if r.versionRange != "" { - return fmt.Errorf("no package '%s' matching version '%s' found", r.packageName, r.versionRange) - } - if r.channelName != "" { - return fmt.Errorf("no package '%s' found in channel '%s'", r.packageName, r.channelName) - } - return fmt.Errorf("no package '%s' found", r.packageName) + return result, nil } diff --git a/internal/resolution/variablesources/required_package_test.go b/internal/resolution/variablesources/required_package_test.go index a6f0bd32a..260c879c8 100644 --- a/internal/resolution/variablesources/required_package_test.go +++ b/internal/resolution/variablesources/required_package_test.go @@ -1,137 +1,168 @@ package variablesources_test import ( - "context" "encoding/json" - "errors" "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" + operatorsv1alpha1 "github.com/operator-framework/operator-controller/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" - testutil "github.com/operator-framework/operator-controller/test/util" ) -var _ = Describe("RequiredPackageVariableSource", func() { - var ( - rpvs *variablesources.RequiredPackageVariableSource - fakeCatalogClient testutil.FakeCatalogClient - packageName string - ) - - BeforeEach(func() { - var err error - packageName = "test-package" - channel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "stable", - }} - bundleList := []*catalogmetadata.Bundle{ - {Bundle: declcfg.Bundle{ - Name: "test-package.v1.0.0", - Package: "test-package", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, - }, - {Bundle: declcfg.Bundle{ - Name: "test-package.v3.0.0", - Package: "test-package", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, - }, - {Bundle: declcfg.Bundle{ - Name: "test-package.v2.0.0", - Package: "test-package", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, - }, - // add some bundles from a different package - {Bundle: declcfg.Bundle{ - Name: "test-package-2.v1.0.0", - Package: "test-package-2", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "1.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, +func TestMakeRequiredPackageVariables(t *testing.T) { + stableChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "stable", + }} + betaChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "beta", + }} + allBundles := []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{ + Name: "test-package.v1.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&stableChannel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v3.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&stableChannel, &betaChannel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v2.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&stableChannel}, + }, + // add some bundles from a different package + {Bundle: declcfg.Bundle{ + Name: "test-package-2.v1.0.0", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "1.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&stableChannel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package-2.v2.0.0", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "2.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&stableChannel}, + }, + } + + fakeOperator := func(packageName, channelName, versionRange string) operatorsv1alpha1.Operator { + return operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("op-%s-%s-%s", packageName, channelName, versionRange), }, - {Bundle: declcfg.Bundle{ - Name: "test-package-2.v2.0.0", - Package: "test-package-2", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "2.0.0"}`)}, - }}, - InChannels: []*catalogmetadata.Channel{&channel}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: packageName, + Version: versionRange, + Channel: channelName, }, } - fakeCatalogClient = testutil.NewFakeCatalogClient(bundleList) - rpvs, err = variablesources.NewRequiredPackageVariableSource(&fakeCatalogClient, packageName) - Expect(err).NotTo(HaveOccurred()) + } + + t.Run("package name only", func(t *testing.T) { + vars, err := variablesources.MakeRequiredPackageVariables(allBundles, []operatorsv1alpha1.Operator{ + fakeOperator("test-package", "", ""), + }) + require.NoError(t, err) + require.Len(t, vars, 1) + + reqPackageVar := vars[0] + assert.Equal(t, deppy.IdentifierFromString("required package test-package"), reqPackageVar.Identifier()) + + bundles := reqPackageVar.Bundles() + require.Len(t, bundles, 3) + // ensure bundles are in version order (high to low) + assert.Equal(t, "test-package.v3.0.0", bundles[0].Name) + assert.Equal(t, "test-package.v2.0.0", bundles[1].Name) + assert.Equal(t, "test-package.v1.0.0", bundles[2].Name) }) - It("should return the correct package variable", func() { - variables, err := rpvs.GetVariables(context.TODO()) - Expect(err).NotTo(HaveOccurred()) - Expect(variables).To(HaveLen(1)) - reqPackageVar, ok := variables[0].(*olmvariables.RequiredPackageVariable) - Expect(ok).To(BeTrue()) - Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) - Expect(reqPackageVar.Bundles()).To(HaveLen(3)) + t.Run("package name and channel", func(t *testing.T) { + vars, err := variablesources.MakeRequiredPackageVariables(allBundles, []operatorsv1alpha1.Operator{ + fakeOperator("test-package", "beta", ""), + }) + require.NoError(t, err) + require.Len(t, vars, 1) + + reqPackageVar := vars[0] + assert.Equal(t, deppy.IdentifierFromString("required package test-package"), reqPackageVar.Identifier()) + + bundles := reqPackageVar.Bundles() + require.Len(t, bundles, 1) // ensure bundles are in version order (high to low) - Expect(reqPackageVar.Bundles()[0].Name).To(Equal("test-package.v3.0.0")) - Expect(reqPackageVar.Bundles()[1].Name).To(Equal("test-package.v2.0.0")) - Expect(reqPackageVar.Bundles()[2].Name).To(Equal("test-package.v1.0.0")) + assert.Equal(t, "test-package.v3.0.0", bundles[0].Name) }) - 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")) - Expect(err).NotTo(HaveOccurred()) + t.Run("package name and version range", func(t *testing.T) { + vars, err := variablesources.MakeRequiredPackageVariables(allBundles, []operatorsv1alpha1.Operator{ + fakeOperator("test-package", "", ">=1.0.0 !=2.0.0 <3.0.0"), + }) + require.NoError(t, err) + require.Len(t, vars, 1) - variables, err := rpvs.GetVariables(context.TODO()) - Expect(err).NotTo(HaveOccurred()) - Expect(variables).To(HaveLen(1)) - reqPackageVar, ok := variables[0].(*olmvariables.RequiredPackageVariable) - Expect(ok).To(BeTrue()) - Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) + reqPackageVar := vars[0] + assert.Equal(t, deppy.IdentifierFromString("required package test-package"), reqPackageVar.Identifier()) - Expect(reqPackageVar.Bundles()).To(HaveLen(1)) + bundles := reqPackageVar.Bundles() + require.Len(t, bundles, 1) // test-package.v1.0.0 is the only package that matches the provided filter - Expect(reqPackageVar.Bundles()[0].Name).To(Equal("test-package.v1.0.0")) + assert.Equal(t, "test-package.v1.0.0", bundles[0].Name) }) - It("should fail with bad semver range", func() { - _, err := variablesources.NewRequiredPackageVariableSource(&fakeCatalogClient, packageName, variablesources.InVersionRange("not a valid semver")) - Expect(err).To(HaveOccurred()) + t.Run("package name and invalid version range", func(t *testing.T) { + vars, err := variablesources.MakeRequiredPackageVariables(allBundles, []operatorsv1alpha1.Operator{ + fakeOperator("test-package", "", "not a valid semver"), + }) + assert.Nil(t, vars) + assert.Error(t, err) }) - It("should return an error if package not found", func() { - emptyCatalogClient := testutil.NewFakeCatalogClient([]*catalogmetadata.Bundle{}) - rpvs, err := variablesources.NewRequiredPackageVariableSource(&emptyCatalogClient, packageName) - Expect(err).NotTo(HaveOccurred()) - _, err = rpvs.GetVariables(context.TODO()) - Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError("no package 'test-package' found")) - }) + t.Run("package not found", func(t *testing.T) { + vars, err := variablesources.MakeRequiredPackageVariables([]*catalogmetadata.Bundle{}, []operatorsv1alpha1.Operator{ + fakeOperator("test-package", "", ""), + }) + assert.Nil(t, vars) + assert.ErrorContains(t, err, "no package 'test-package' found") + + vars, err = variablesources.MakeRequiredPackageVariables([]*catalogmetadata.Bundle{}, []operatorsv1alpha1.Operator{ + fakeOperator("test-package", "stable", ""), + }) + assert.Nil(t, vars) + assert.ErrorContains(t, err, "no package 'test-package' found in channel 'stable'") + + vars, err = variablesources.MakeRequiredPackageVariables([]*catalogmetadata.Bundle{}, []operatorsv1alpha1.Operator{ + fakeOperator("test-package", "", "1.0.0"), + }) + assert.Nil(t, vars) + assert.ErrorContains(t, err, "no package 'test-package' matching version '1.0.0' 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)) + vars, err = variablesources.MakeRequiredPackageVariables([]*catalogmetadata.Bundle{}, []operatorsv1alpha1.Operator{ + fakeOperator("test-package", "stable", "1.0.0"), + }) + assert.Nil(t, vars) + assert.ErrorContains(t, err, "no package 'test-package' matching version '1.0.0' found in channel 'stable'") }) -}) +}