Skip to content

Commit b849eb7

Browse files
benluddybenluddy
authored
Remove hardcoded references to the registry gRPC API from the resolver packages. (#2340)
* Stop using the registry's Bundle proto type for resolution. The resolver component does not care whether a given cache entry was populated by use of the registry gRPC API, so it should ultimately be possible to drop the "Bundle" field from cache entries altogether. For now, catalog-operator's "step resolver" still depends on a Bundle proto field in order to unpack inline (as opposed to image-based) packages. Signed-off-by: Ben Luddy <[email protected]> * Remove exported resolver cache entry creation funcs. NewOperatorForBundle and NewOperatorFromV1Alpha1CSV couple the resolver cache package to the registry gRPC API and the operators/v1alpha1 CSV API, respectively. They're effectively used only by the resolver package, so they can be moved there and unexported. The remaining calls to NewOperatorFromV1Alpha1CSV can be extracted from core resolver logic into a Source implementation that is responsible for the conversion from ClusterServiceVersion to cache.Snapshot entry, and the remaining call to NewOperatorFromBundle is used only by the Source implementation based on the registry gRPC API. Signed-off-by: Ben Luddy <[email protected]>
1 parent de4bebe commit b849eb7

15 files changed

+1219
-1191
lines changed

‎pkg/controller/operators/olm/labeler.go

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
package olm
22

33
import (
4+
"fmt"
5+
"strings"
6+
7+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
48
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
59
"github.com/operator-framework/operator-registry/pkg/registry"
10+
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
611
extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
712
extv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
813
"k8s.io/apimachinery/pkg/labels"
@@ -13,9 +18,41 @@ const (
1318
APILabelKeyPrefix = "olm.api."
1419
)
1520

16-
type operatorSurface interface {
17-
GetProvidedAPIs() cache.APISet
18-
GetRequiredAPIs() cache.APISet
21+
type operatorSurface struct {
22+
ProvidedAPIs cache.APISet
23+
RequiredAPIs cache.APISet
24+
}
25+
26+
func apiSurfaceOfCSV(csv *v1alpha1.ClusterServiceVersion) (*operatorSurface, error) {
27+
surface := operatorSurface{
28+
ProvidedAPIs: cache.EmptyAPISet(),
29+
RequiredAPIs: cache.EmptyAPISet(),
30+
}
31+
32+
for _, crdDef := range csv.Spec.CustomResourceDefinitions.Owned {
33+
parts := strings.SplitN(crdDef.Name, ".", 2)
34+
if len(parts) < 2 {
35+
return nil, fmt.Errorf("error parsing crd name: %s", crdDef.Name)
36+
}
37+
surface.ProvidedAPIs[opregistry.APIKey{Plural: parts[0], Group: parts[1], Version: crdDef.Version, Kind: crdDef.Kind}] = struct{}{}
38+
}
39+
for _, api := range csv.Spec.APIServiceDefinitions.Owned {
40+
surface.ProvidedAPIs[opregistry.APIKey{Group: api.Group, Version: api.Version, Kind: api.Kind, Plural: api.Name}] = struct{}{}
41+
}
42+
43+
requiredAPIs := cache.EmptyAPISet()
44+
for _, crdDef := range csv.Spec.CustomResourceDefinitions.Required {
45+
parts := strings.SplitN(crdDef.Name, ".", 2)
46+
if len(parts) < 2 {
47+
return nil, fmt.Errorf("error parsing crd name: %s", crdDef.Name)
48+
}
49+
requiredAPIs[opregistry.APIKey{Plural: parts[0], Group: parts[1], Version: crdDef.Version, Kind: crdDef.Kind}] = struct{}{}
50+
}
51+
for _, api := range csv.Spec.APIServiceDefinitions.Required {
52+
requiredAPIs[opregistry.APIKey{Group: api.Group, Version: api.Version, Kind: api.Kind, Plural: api.Name}] = struct{}{}
53+
}
54+
55+
return &surface, nil
1956
}
2057

2158
// LabelSetsFor returns API label sets for the given object.
@@ -35,14 +72,14 @@ func LabelSetsFor(obj interface{}) ([]labels.Set, error) {
3572

3673
func labelSetsForOperatorSurface(surface operatorSurface) ([]labels.Set, error) {
3774
labelSet := labels.Set{}
38-
for key := range surface.GetProvidedAPIs().StripPlural() {
75+
for key := range surface.ProvidedAPIs.StripPlural() {
3976
hash, err := cache.APIKeyToGVKHash(key)
4077
if err != nil {
4178
return nil, err
4279
}
4380
labelSet[APILabelKeyPrefix+hash] = "provided"
4481
}
45-
for key := range surface.GetRequiredAPIs().StripPlural() {
82+
for key := range surface.RequiredAPIs.StripPlural() {
4683
hash, err := cache.APIKeyToGVKHash(key)
4784
if err != nil {
4885
return nil, err

‎pkg/controller/operators/olm/labeler_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package olm
33
import (
44
"testing"
55

6-
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
76
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
87
"github.com/stretchr/testify/require"
98
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
@@ -63,9 +62,9 @@ func TestLabelSetsFor(t *testing.T) {
6362
},
6463
{
6564
name: "OperatorSurface/Provided",
66-
obj: &cache.Operator{
65+
obj: operatorSurface{
6766
ProvidedAPIs: map[opregistry.APIKey]struct{}{
68-
opregistry.APIKey{Group: "ghouls", Version: "v1alpha1", Kind: "Ghost", Plural: "Ghosts"}: {},
67+
{Group: "ghouls", Version: "v1alpha1", Kind: "Ghost", Plural: "Ghosts"}: {},
6968
},
7069
},
7170
expected: []labels.Set{
@@ -76,12 +75,12 @@ func TestLabelSetsFor(t *testing.T) {
7675
},
7776
{
7877
name: "OperatorSurface/ProvidedAndRequired",
79-
obj: &cache.Operator{
78+
obj: operatorSurface{
8079
ProvidedAPIs: map[opregistry.APIKey]struct{}{
81-
opregistry.APIKey{Group: "ghouls", Version: "v1alpha1", Kind: "Ghost", Plural: "Ghosts"}: {},
80+
{Group: "ghouls", Version: "v1alpha1", Kind: "Ghost", Plural: "Ghosts"}: {},
8281
},
8382
RequiredAPIs: map[opregistry.APIKey]struct{}{
84-
opregistry.APIKey{Group: "ghouls", Version: "v1alpha1", Kind: "Goblin", Plural: "Goblins"}: {},
83+
{Group: "ghouls", Version: "v1alpha1", Kind: "Goblin", Plural: "Goblins"}: {},
8584
},
8685
},
8786
expected: []labels.Set{

‎pkg/controller/operators/olm/operator.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import (
3737
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
3838
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/internal/pruning"
3939
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/overrides"
40-
resolvercache "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
4140
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clients"
4241
csvutility "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/csv"
4342
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/event"
@@ -1381,7 +1380,7 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
13811380
out = in.DeepCopy()
13821381
now := a.now()
13831382

1384-
operatorSurface, err := resolvercache.NewOperatorFromV1Alpha1CSV(out)
1383+
operatorSurface, err := apiSurfaceOfCSV(out)
13851384
if err != nil {
13861385
// If the resolver is unable to retrieve the operator info from the CSV the CSV requires changes, a syncError should not be returned.
13871386
logger.WithError(err).Warn("Unable to retrieve operator information from CSV")
@@ -1445,7 +1444,7 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
14451444

14461445
groupSurface := NewOperatorGroup(operatorGroup)
14471446
otherGroupSurfaces := NewOperatorGroupSurfaces(otherGroups...)
1448-
providedAPIs := operatorSurface.GetProvidedAPIs().StripPlural()
1447+
providedAPIs := operatorSurface.ProvidedAPIs.StripPlural()
14491448

14501449
switch result := a.apiReconciler.Reconcile(providedAPIs, groupSurface, otherGroupSurfaces...); {
14511450
case operatorGroup.Spec.StaticProvidedAPIs && (result == AddAPIs || result == RemoveAPIs):

‎pkg/controller/operators/olm/operatorgroup.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,12 +309,12 @@ func (a *Operator) providedAPIsFromCSVs(group *v1.OperatorGroup, logger *logrus.
309309
// TODO: Throw out CSVs that aren't members of the group due to group related failures?
310310

311311
// Union the providedAPIsFromCSVs from existing members of the group
312-
operatorSurface, err := cache.NewOperatorFromV1Alpha1CSV(csv)
312+
operatorSurface, err := apiSurfaceOfCSV(csv)
313313
if err != nil {
314314
logger.WithError(err).Warn("could not create OperatorSurface from csv")
315315
continue
316316
}
317-
for providedAPI := range operatorSurface.GetProvidedAPIs().StripPlural() {
317+
for providedAPI := range operatorSurface.ProvidedAPIs.StripPlural() {
318318
providedAPIsFromCSVs[providedAPI] = csv
319319
}
320320
}
@@ -1051,12 +1051,12 @@ func (a *Operator) findCSVsThatProvideAnyOf(provide cache.APISet) ([]*v1alpha1.C
10511051
continue
10521052
}
10531053

1054-
operatorSurface, err := cache.NewOperatorFromV1Alpha1CSV(csv)
1054+
operatorSurface, err := apiSurfaceOfCSV(csv)
10551055
if err != nil {
10561056
continue
10571057
}
10581058

1059-
if len(operatorSurface.GetProvidedAPIs().StripPlural().Intersection(provide)) > 0 {
1059+
if len(operatorSurface.ProvidedAPIs.StripPlural().Intersection(provide)) > 0 {
10601060
providers = append(providers, csv)
10611061
}
10621062
}

‎pkg/controller/registry/resolver/cache/cache_test.go

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ import (
1111

1212
"github.com/stretchr/testify/assert"
1313
"github.com/stretchr/testify/require"
14-
15-
"github.com/operator-framework/operator-registry/pkg/api"
1614
)
1715

1816
func TestOperatorCacheConcurrency(t *testing.T) {
@@ -240,29 +238,6 @@ func TestCatalogSnapshotFind(t *testing.T) {
240238

241239
}
242240

243-
func TestStripPluralRequiredAndProvidedAPIKeys(t *testing.T) {
244-
key := SourceKey{Namespace: "testnamespace", Name: "testname"}
245-
o, err := NewOperatorFromBundle(&api.Bundle{
246-
CsvName: fmt.Sprintf("%s/%s", key.Namespace, key.Name),
247-
ProvidedApis: []*api.GroupVersionKind{{
248-
Group: "g",
249-
Version: "v1",
250-
Kind: "K",
251-
Plural: "ks",
252-
}},
253-
RequiredApis: []*api.GroupVersionKind{{
254-
Group: "g2",
255-
Version: "v2",
256-
Kind: "K2",
257-
Plural: "ks2",
258-
}},
259-
}, "", key, "")
260-
261-
assert.NoError(t, err)
262-
assert.Equal(t, "K.v1.g", o.ProvidedAPIs.String())
263-
assert.Equal(t, "K2.v2.g2", o.RequiredAPIs.String())
264-
}
265-
266241
type ErrorSource struct {
267242
Error error
268243
}

0 commit comments

Comments
 (0)