Skip to content

Commit 8d2811d

Browse files
author
Mikalai Radchuk
committed
Move bundle variable code into a func
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
1 parent 0d2e471 commit 8d2811d

File tree

3 files changed

+261
-47
lines changed

3 files changed

+261
-47
lines changed

‎internal/resolution/variablesources/bundle.go

Lines changed: 72 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,62 +2,36 @@ package variablesources
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"sort"
78

9+
"k8s.io/apimachinery/pkg/util/sets"
10+
811
"github.com/operator-framework/deppy/pkg/deppy"
912
"github.com/operator-framework/deppy/pkg/deppy/input"
10-
"k8s.io/apimachinery/pkg/util/sets"
1113

1214
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1315
catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
1416
catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
1517
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
1618
)
1719

18-
var _ input.VariableSource = &BundlesAndDepsVariableSource{}
19-
20-
type BundlesAndDepsVariableSource struct {
21-
catalogClient BundleProvider
22-
variableSources []input.VariableSource
23-
}
24-
25-
func NewBundlesAndDepsVariableSource(catalogClient BundleProvider, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource {
26-
return &BundlesAndDepsVariableSource{
27-
catalogClient: catalogClient,
28-
variableSources: inputVariableSources,
29-
}
30-
}
31-
32-
func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) {
33-
var variables []deppy.Variable
34-
35-
// extract required package variables
36-
for _, variableSource := range b.variableSources {
37-
inputVariables, err := variableSource.GetVariables(ctx)
38-
if err != nil {
39-
return nil, err
40-
}
41-
variables = append(variables, inputVariables...)
42-
}
43-
44-
// create bundle queue for dependency resolution
20+
func MakeBundleVariables(
21+
allBundles []*catalogmetadata.Bundle,
22+
requiredPackages []*olmvariables.RequiredPackageVariable,
23+
installedPackages []*olmvariables.InstalledPackageVariable,
24+
) ([]*olmvariables.BundleVariable, error) {
4525
var bundleQueue []*catalogmetadata.Bundle
46-
for _, variable := range variables {
47-
switch v := variable.(type) {
48-
case *olmvariables.RequiredPackageVariable:
49-
bundleQueue = append(bundleQueue, v.Bundles()...)
50-
case *olmvariables.InstalledPackageVariable:
51-
bundleQueue = append(bundleQueue, v.Bundles()...)
52-
}
26+
for _, variable := range requiredPackages {
27+
bundleQueue = append(bundleQueue, variable.Bundles()...)
5328
}
54-
55-
allBundles, err := b.catalogClient.Bundles(ctx)
56-
if err != nil {
57-
return nil, err
29+
for _, variable := range installedPackages {
30+
bundleQueue = append(bundleQueue, variable.Bundles()...)
5831
}
5932

6033
// build bundle and dependency variables
34+
var result []*olmvariables.BundleVariable
6135
visited := sets.Set[deppy.Identifier]{}
6236
for len(bundleQueue) > 0 {
6337
// pop head of queue
@@ -73,7 +47,7 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
7347
visited.Insert(id)
7448

7549
// get bundle dependencies
76-
dependencies, err := b.filterBundleDependencies(allBundles, head)
50+
dependencies, err := filterBundleDependencies(allBundles, head)
7751
if err != nil {
7852
return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err)
7953
}
@@ -82,23 +56,25 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
8256
bundleQueue = append(bundleQueue, dependencies...)
8357

8458
// create variable
85-
variables = append(variables, olmvariables.NewBundleVariable(head, dependencies))
59+
result = append(result, olmvariables.NewBundleVariable(head, dependencies))
8660
}
8761

88-
return variables, nil
62+
return result, nil
8963
}
9064

91-
func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
65+
func filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
9266
var dependencies []*catalogmetadata.Bundle
9367
added := sets.Set[deppy.Identifier]{}
9468

9569
// gather required package dependencies
96-
// todo(perdasilva): disambiguate between not found and actual errors
9770
requiredPackages, _ := bundle.RequiredPackages()
9871
for _, requiredPackage := range requiredPackages {
99-
packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(requiredPackage.SemverRange)))
72+
packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(
73+
catalogfilter.WithPackageName(requiredPackage.PackageName),
74+
catalogfilter.InBlangSemverRange(requiredPackage.SemverRange),
75+
))
10076
if len(packageDependencyBundles) == 0 {
101-
return nil, fmt.Errorf("could not find package dependencies for bundle '%s'", bundle.Name)
77+
return nil, errors.New("could not find package dependencies for bundle")
10278
}
10379
for i := 0; i < len(packageDependencyBundles); i++ {
10480
bundle := packageDependencyBundles[i]
@@ -117,3 +93,53 @@ func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*ca
11793

11894
return dependencies, nil
11995
}
96+
97+
type BundlesAndDepsVariableSource struct {
98+
catalogClient BundleProvider
99+
variableSources []input.VariableSource
100+
}
101+
102+
func NewBundlesAndDepsVariableSource(catalogClient BundleProvider, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource {
103+
return &BundlesAndDepsVariableSource{
104+
catalogClient: catalogClient,
105+
variableSources: inputVariableSources,
106+
}
107+
}
108+
109+
func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) {
110+
variables := []deppy.Variable{}
111+
112+
for _, variableSource := range b.variableSources {
113+
inputVariables, err := variableSource.GetVariables(ctx)
114+
if err != nil {
115+
return nil, err
116+
}
117+
variables = append(variables, inputVariables...)
118+
}
119+
120+
allBundles, err := b.catalogClient.Bundles(ctx)
121+
if err != nil {
122+
return nil, err
123+
}
124+
125+
requiredPackages := []*olmvariables.RequiredPackageVariable{}
126+
installedPackages := []*olmvariables.InstalledPackageVariable{}
127+
for _, variable := range variables {
128+
switch v := variable.(type) {
129+
case *olmvariables.RequiredPackageVariable:
130+
requiredPackages = append(requiredPackages, v)
131+
case *olmvariables.InstalledPackageVariable:
132+
installedPackages = append(installedPackages, v)
133+
}
134+
}
135+
136+
bundles, err := MakeBundleVariables(allBundles, requiredPackages, installedPackages)
137+
if err != nil {
138+
return nil, err
139+
}
140+
141+
for _, v := range bundles {
142+
variables = append(variables, v)
143+
}
144+
return variables, nil
145+
}

‎internal/resolution/variablesources/bundle_test.go

Lines changed: 178 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,196 @@ import (
44
"context"
55
"encoding/json"
66
"errors"
7+
"testing"
78

89
. "github.com/onsi/ginkgo/v2"
910
. "github.com/onsi/gomega"
1011
"github.com/operator-framework/deppy/pkg/deppy"
1112
"github.com/operator-framework/operator-registry/alpha/declcfg"
1213
"github.com/operator-framework/operator-registry/alpha/property"
14+
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/require"
1316

1417
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1518
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
1619
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
1720
testutil "github.com/operator-framework/operator-controller/test/util"
1821
)
1922

23+
func TestMakeBundleVariables(t *testing.T) {
24+
t.Run("valid dependencies", func(t *testing.T) {
25+
const fakeCatalogName = "fake-catalog"
26+
fakeChannel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}}
27+
bundleSet := map[string]*catalogmetadata.Bundle{
28+
// Test package which we will be using as input into
29+
// the testable function
30+
"test-package.v1.0.0": {
31+
Bundle: declcfg.Bundle{
32+
Name: "test-package.v1.0.0",
33+
Package: "test-package",
34+
Properties: []property.Property{
35+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)},
36+
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "first-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
37+
},
38+
},
39+
CatalogName: fakeCatalogName,
40+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
41+
},
42+
43+
// First level dependency of test-package. Will be explicitly
44+
// provided into the testable function as part of variable.
45+
// This package must have at least one dependency with a version
46+
// range so we can test that result has correct ordering:
47+
// the testable function must give priority to newer versions.
48+
"first-level-dependency.v1.0.0": {
49+
Bundle: declcfg.Bundle{
50+
Name: "first-level-dependency.v1.0.0",
51+
Package: "first-level-dependency",
52+
Properties: []property.Property{
53+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "first-level-dependency", "version": "1.0.0"}`)},
54+
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "second-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
55+
},
56+
},
57+
CatalogName: fakeCatalogName,
58+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
59+
},
60+
61+
// Second level dependency that matches requirements of the first level dependency.
62+
"second-level-dependency.v1.0.0": {
63+
Bundle: declcfg.Bundle{
64+
Name: "second-level-dependency.v1.0.0",
65+
Package: "second-level-dependency",
66+
Properties: []property.Property{
67+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "1.0.0"}`)},
68+
},
69+
},
70+
CatalogName: fakeCatalogName,
71+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
72+
},
73+
74+
// Second level dependency that matches requirements of the first level dependency.
75+
"second-level-dependency.v1.0.1": {
76+
Bundle: declcfg.Bundle{
77+
Name: "second-level-dependency.v1.0.1",
78+
Package: "second-level-dependency",
79+
Properties: []property.Property{
80+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "1.0.1"}`)},
81+
},
82+
},
83+
CatalogName: fakeCatalogName,
84+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
85+
},
86+
87+
// Second level dependency that does not match requirements of the first level dependency.
88+
"second-level-dependency.v2.0.0": {
89+
Bundle: declcfg.Bundle{
90+
Name: "second-level-dependency.v2.0.0",
91+
Package: "second-level-dependency",
92+
Properties: []property.Property{
93+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "2.0.0"}`)},
94+
},
95+
},
96+
CatalogName: fakeCatalogName,
97+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
98+
},
99+
100+
// Package that is in a our fake catalog, but is not involved
101+
// in this dependency chain. We need this to make sure that
102+
// the testable function filters out unrelated bundles.
103+
"uninvolved-package.v1.0.0": {
104+
Bundle: declcfg.Bundle{
105+
Name: "uninvolved-package.v1.0.0",
106+
Package: "uninvolved-package",
107+
Properties: []property.Property{
108+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "uninvolved-package", "version": "1.0.0"}`)},
109+
},
110+
},
111+
CatalogName: fakeCatalogName,
112+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
113+
},
114+
}
115+
116+
allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet))
117+
for _, bundle := range bundleSet {
118+
allBundles = append(allBundles, bundle)
119+
}
120+
requiredPackages := []*olmvariables.RequiredPackageVariable{
121+
olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{
122+
bundleSet["first-level-dependency.v1.0.0"],
123+
}),
124+
}
125+
installedPackages := []*olmvariables.InstalledPackageVariable{
126+
olmvariables.NewInstalledPackageVariable("test-package", []*catalogmetadata.Bundle{
127+
bundleSet["first-level-dependency.v1.0.0"],
128+
}),
129+
}
130+
131+
bundles, err := variablesources.MakeBundleVariables(allBundles, requiredPackages, installedPackages)
132+
require.NoError(t, err)
133+
require.Len(t, bundles, 3)
134+
135+
// Each dependency must have a variable.
136+
// Dependencies from the same package must be sorted by version
137+
// with higher versions first.
138+
expectedIDs := []deppy.Identifier{
139+
"fake-catalog-first-level-dependency-first-level-dependency.v1.0.0",
140+
"fake-catalog-second-level-dependency-second-level-dependency.v1.0.1",
141+
"fake-catalog-second-level-dependency-second-level-dependency.v1.0.0",
142+
}
143+
actualIDs := collectVariableIDs(bundles)
144+
assert.Equal(t, expectedIDs, actualIDs)
145+
146+
// Bundle variables must have correct dependencies
147+
bundle := findVariableWithName(bundles, "first-level-dependency.v1.0.0")
148+
dependencies := bundle.Dependencies()
149+
require.Len(t, dependencies, 2)
150+
assert.Equal(t, bundleSet["second-level-dependency.v1.0.1"], dependencies[0])
151+
assert.Equal(t, bundleSet["second-level-dependency.v1.0.0"], dependencies[1])
152+
153+
bundle = findVariableWithName(bundles, "second-level-dependency.v1.0.1")
154+
dependencies = bundle.Dependencies()
155+
require.Len(t, dependencies, 0)
156+
157+
bundle = findVariableWithName(bundles, "second-level-dependency.v1.0.0")
158+
dependencies = bundle.Dependencies()
159+
require.Len(t, dependencies, 0)
160+
})
161+
162+
t.Run("non existent dependencies", func(t *testing.T) {
163+
const fakeCatalogName = "fake-catalog"
164+
fakeChannel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}}
165+
bundleSet := map[string]*catalogmetadata.Bundle{
166+
"test-package.v1.0.0": {
167+
Bundle: declcfg.Bundle{
168+
Name: "test-package.v1.0.0",
169+
Package: "test-package",
170+
Properties: []property.Property{
171+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)},
172+
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "first-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
173+
},
174+
},
175+
CatalogName: fakeCatalogName,
176+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
177+
},
178+
}
179+
180+
allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet))
181+
for _, bundle := range bundleSet {
182+
allBundles = append(allBundles, bundle)
183+
}
184+
requiredPackages := []*olmvariables.RequiredPackageVariable{
185+
olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{
186+
bundleSet["test-package.v1.0.0"],
187+
}),
188+
}
189+
installedPackages := []*olmvariables.InstalledPackageVariable{}
190+
191+
bundles, err := variablesources.MakeBundleVariables(allBundles, requiredPackages, installedPackages)
192+
assert.ErrorContains(t, err, "could not determine dependencies for bundle with id 'fake-catalog-test-package-test-package.v1.0.0'")
193+
assert.Nil(t, bundles)
194+
})
195+
}
196+
20197
var _ = Describe("BundlesAndDepsVariableSource", func() {
21198
var (
22199
bdvs *variablesources.BundlesAndDepsVariableSource
@@ -374,7 +551,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
374551
)
375552
_, err := bdvs.GetVariables(context.TODO())
376553
Expect(err).To(HaveOccurred())
377-
Expect(err.Error()).To(ContainSubstring("could not determine dependencies for bundle with id 'fake-catalog-test-package-bundle-2': could not find package dependencies for bundle 'bundle-2'"))
554+
Expect(err.Error()).To(ContainSubstring("could not determine dependencies for bundle with id 'fake-catalog-test-package-bundle-2': could not find package dependencies for bundle"))
378555
})
379556

380557
It("should return error if an inner variable source returns an error", func() {

‎internal/resolution/variablesources/variable_utils_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,19 @@ package variablesources_test
22

33
import (
44
"github.com/operator-framework/deppy/pkg/deppy"
5+
6+
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
57
)
68

9+
func findVariableWithName(vars []*olmvariables.BundleVariable, name string) *olmvariables.BundleVariable {
10+
for i := 0; i < len(vars); i++ {
11+
if vars[i].Bundle().Name == name {
12+
return vars[i]
13+
}
14+
}
15+
return nil
16+
}
17+
718
func collectVariableIDs[T deppy.Variable](vars []T) []deppy.Identifier {
819
ids := make([]deppy.Identifier, 0, len(vars))
920
for _, v := range vars {

0 commit comments

Comments
 (0)