Skip to content

Remove hardcoded references to the registry gRPC API from the resolver packages. #2340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 42 additions & 5 deletions pkg/controller/operators/olm/labeler.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
package olm

import (
"fmt"
"strings"

"github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
"github.com/operator-framework/operator-registry/pkg/registry"
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
extv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
"k8s.io/apimachinery/pkg/labels"
Expand All @@ -13,9 +18,41 @@ const (
APILabelKeyPrefix = "olm.api."
)

type operatorSurface interface {
GetProvidedAPIs() cache.APISet
GetRequiredAPIs() cache.APISet
type operatorSurface struct {
ProvidedAPIs cache.APISet
RequiredAPIs cache.APISet
}

func apiSurfaceOfCSV(csv *v1alpha1.ClusterServiceVersion) (*operatorSurface, error) {
surface := operatorSurface{
ProvidedAPIs: cache.EmptyAPISet(),
RequiredAPIs: cache.EmptyAPISet(),
}

for _, crdDef := range csv.Spec.CustomResourceDefinitions.Owned {
parts := strings.SplitN(crdDef.Name, ".", 2)
if len(parts) < 2 {
return nil, fmt.Errorf("error parsing crd name: %s", crdDef.Name)
}
surface.ProvidedAPIs[opregistry.APIKey{Plural: parts[0], Group: parts[1], Version: crdDef.Version, Kind: crdDef.Kind}] = struct{}{}
}
for _, api := range csv.Spec.APIServiceDefinitions.Owned {
surface.ProvidedAPIs[opregistry.APIKey{Group: api.Group, Version: api.Version, Kind: api.Kind, Plural: api.Name}] = struct{}{}
}

requiredAPIs := cache.EmptyAPISet()
for _, crdDef := range csv.Spec.CustomResourceDefinitions.Required {
parts := strings.SplitN(crdDef.Name, ".", 2)
if len(parts) < 2 {
return nil, fmt.Errorf("error parsing crd name: %s", crdDef.Name)
}
requiredAPIs[opregistry.APIKey{Plural: parts[0], Group: parts[1], Version: crdDef.Version, Kind: crdDef.Kind}] = struct{}{}
}
for _, api := range csv.Spec.APIServiceDefinitions.Required {
requiredAPIs[opregistry.APIKey{Group: api.Group, Version: api.Version, Kind: api.Kind, Plural: api.Name}] = struct{}{}
}

return &surface, nil
}

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

func labelSetsForOperatorSurface(surface operatorSurface) ([]labels.Set, error) {
labelSet := labels.Set{}
for key := range surface.GetProvidedAPIs().StripPlural() {
for key := range surface.ProvidedAPIs.StripPlural() {
hash, err := cache.APIKeyToGVKHash(key)
if err != nil {
return nil, err
}
labelSet[APILabelKeyPrefix+hash] = "provided"
}
for key := range surface.GetRequiredAPIs().StripPlural() {
for key := range surface.RequiredAPIs.StripPlural() {
hash, err := cache.APIKeyToGVKHash(key)
if err != nil {
return nil, err
Expand Down
11 changes: 5 additions & 6 deletions pkg/controller/operators/olm/labeler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package olm
import (
"testing"

"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
"github.com/stretchr/testify/require"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
Expand Down Expand Up @@ -63,9 +62,9 @@ func TestLabelSetsFor(t *testing.T) {
},
{
name: "OperatorSurface/Provided",
obj: &cache.Operator{
obj: operatorSurface{
ProvidedAPIs: map[opregistry.APIKey]struct{}{
opregistry.APIKey{Group: "ghouls", Version: "v1alpha1", Kind: "Ghost", Plural: "Ghosts"}: {},
{Group: "ghouls", Version: "v1alpha1", Kind: "Ghost", Plural: "Ghosts"}: {},
},
},
expected: []labels.Set{
Expand All @@ -76,12 +75,12 @@ func TestLabelSetsFor(t *testing.T) {
},
{
name: "OperatorSurface/ProvidedAndRequired",
obj: &cache.Operator{
obj: operatorSurface{
ProvidedAPIs: map[opregistry.APIKey]struct{}{
opregistry.APIKey{Group: "ghouls", Version: "v1alpha1", Kind: "Ghost", Plural: "Ghosts"}: {},
{Group: "ghouls", Version: "v1alpha1", Kind: "Ghost", Plural: "Ghosts"}: {},
},
RequiredAPIs: map[opregistry.APIKey]struct{}{
opregistry.APIKey{Group: "ghouls", Version: "v1alpha1", Kind: "Goblin", Plural: "Goblins"}: {},
{Group: "ghouls", Version: "v1alpha1", Kind: "Goblin", Plural: "Goblins"}: {},
},
},
expected: []labels.Set{
Expand Down
5 changes: 2 additions & 3 deletions pkg/controller/operators/olm/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/internal/pruning"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/overrides"
resolvercache "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clients"
csvutility "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/csv"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/event"
Expand Down Expand Up @@ -1364,7 +1363,7 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
out = in.DeepCopy()
now := a.now()

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

groupSurface := NewOperatorGroup(operatorGroup)
otherGroupSurfaces := NewOperatorGroupSurfaces(otherGroups...)
providedAPIs := operatorSurface.GetProvidedAPIs().StripPlural()
providedAPIs := operatorSurface.ProvidedAPIs.StripPlural()

switch result := a.apiReconciler.Reconcile(providedAPIs, groupSurface, otherGroupSurfaces...); {
case operatorGroup.Spec.StaticProvidedAPIs && (result == AddAPIs || result == RemoveAPIs):
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/operators/olm/operatorgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,12 +309,12 @@ func (a *Operator) providedAPIsFromCSVs(group *v1.OperatorGroup, logger *logrus.
// TODO: Throw out CSVs that aren't members of the group due to group related failures?

// Union the providedAPIsFromCSVs from existing members of the group
operatorSurface, err := cache.NewOperatorFromV1Alpha1CSV(csv)
operatorSurface, err := apiSurfaceOfCSV(csv)
if err != nil {
logger.WithError(err).Warn("could not create OperatorSurface from csv")
continue
}
for providedAPI := range operatorSurface.GetProvidedAPIs().StripPlural() {
for providedAPI := range operatorSurface.ProvidedAPIs.StripPlural() {
providedAPIsFromCSVs[providedAPI] = csv
}
}
Expand Down Expand Up @@ -1051,12 +1051,12 @@ func (a *Operator) findCSVsThatProvideAnyOf(provide cache.APISet) ([]*v1alpha1.C
continue
}

operatorSurface, err := cache.NewOperatorFromV1Alpha1CSV(csv)
operatorSurface, err := apiSurfaceOfCSV(csv)
if err != nil {
continue
}

if len(operatorSurface.GetProvidedAPIs().StripPlural().Intersection(provide)) > 0 {
if len(operatorSurface.ProvidedAPIs.StripPlural().Intersection(provide)) > 0 {
providers = append(providers, csv)
}
}
Expand Down
25 changes: 0 additions & 25 deletions pkg/controller/registry/resolver/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/operator-framework/operator-registry/pkg/api"
)

func TestOperatorCacheConcurrency(t *testing.T) {
Expand Down Expand Up @@ -240,29 +238,6 @@ func TestCatalogSnapshotFind(t *testing.T) {

}

func TestStripPluralRequiredAndProvidedAPIKeys(t *testing.T) {
key := SourceKey{Namespace: "testnamespace", Name: "testname"}
o, err := NewOperatorFromBundle(&api.Bundle{
CsvName: fmt.Sprintf("%s/%s", key.Namespace, key.Name),
ProvidedApis: []*api.GroupVersionKind{{
Group: "g",
Version: "v1",
Kind: "K",
Plural: "ks",
}},
RequiredApis: []*api.GroupVersionKind{{
Group: "g2",
Version: "v2",
Kind: "K2",
Plural: "ks2",
}},
}, "", key, "")

assert.NoError(t, err)
assert.Equal(t, "K.v1.g", o.ProvidedAPIs.String())
assert.Equal(t, "K2.v2.g2", o.RequiredAPIs.String())
}

type ErrorSource struct {
Error error
}
Expand Down
Loading