Skip to content

Commit 0e4e7e3

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1284 from dinhxuanvu/pkgm-fix
Bug 1786471: fix(packagemanifests): Fix index key func to avoid key collision
2 parents 4cb846e + daf5ac2 commit 0e4e7e3

File tree

2 files changed

+176
-4
lines changed

2 files changed

+176
-4
lines changed

‎pkg/package-server/provider/registry.go

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"io"
8+
"strings"
89
"sync"
910
"time"
1011

@@ -57,6 +58,30 @@ func catalogIndexFunc(obj interface{}) ([]string, error) {
5758
return []string{getSourceKey(pkg).String()}, nil
5859
}
5960

61+
func PackageManifestKeyFunc(obj interface{}) (string, error) {
62+
if key, ok := obj.(string); ok {
63+
return string(key), nil
64+
}
65+
66+
pkg, ok := obj.(*operators.PackageManifest)
67+
if !ok {
68+
return "", fmt.Errorf("obj is not a packagemanifest %v", obj)
69+
}
70+
71+
return pkg.Status.CatalogSource + "/" + pkg.GetNamespace() + "/" + pkg.GetName(), nil
72+
}
73+
74+
func SplitPackageManifestKey(key string) (catsrcname, namespace, name string, err error) {
75+
parts := strings.Split(key, "/")
76+
switch len(parts) {
77+
case 3:
78+
// catalogsource name, namespace and packagemanifest name
79+
return parts[0], parts[1], parts[2], nil
80+
}
81+
82+
return "", "", "", fmt.Errorf("unexpected key format: %q", key)
83+
}
84+
6085
type registryClient struct {
6186
api.RegistryClient
6287
catsrc *operatorsv1alpha1.CatalogSource
@@ -104,7 +129,7 @@ func NewRegistryProvider(ctx context.Context, crClient versioned.Interface, oper
104129
Operator: operator,
105130

106131
globalNamespace: globalNamespace,
107-
cache: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{
132+
cache: cache.NewIndexer(PackageManifestKeyFunc, cache.Indexers{
108133
cache.NamespaceIndex: cache.MetaNamespaceIndexFunc,
109134
catalogIndex: catalogIndexFunc,
110135
}),
@@ -311,14 +336,13 @@ func (p *RegistryProvider) gcPackages(key resolver.CatalogKey, keep map[string]s
311336

312337
var errs []error
313338
for _, storedPkgKey := range storedPkgKeys {
314-
_, name, _ := cache.SplitMetaNamespaceKey(storedPkgKey)
339+
_, _, name, _ := SplitPackageManifestKey(storedPkgKey)
315340
if keep != nil {
316341
if _, ok := keep[name]; ok {
317342
continue
318343
}
319344
}
320-
321-
if err := p.cache.Delete(cache.ExplicitKey(storedPkgKey)); err != nil {
345+
if err := p.cache.Delete(string(storedPkgKey)); err != nil {
322346
logger.WithField("pkg", name).WithError(err).Warn("failed to delete cache entry")
323347
errs = append(errs, err)
324348
}

‎pkg/package-server/provider/registry_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,154 @@ func TestRegistryProviderList(t *testing.T) {
812812
},
813813
}},
814814
},
815+
{
816+
name: "TwoCatalogs/SameNamespace/DuplicatePackages/PackagesFound",
817+
globalNS: "ns",
818+
registryClients: []*registryClient{
819+
newTestRegistryClient(t, withRegistryServiceStatus(catalogSource("cool-operators", "ns"), "grpc", "cool-operators", "ns", port, metav1.NewTime(time.Now()))),
820+
newTestRegistryClient(t, withRegistryServiceStatus(catalogSource("cool-operators-2", "ns"), "grpc", "cool-operators-2", "ns", port, metav1.NewTime(time.Now()))),
821+
},
822+
requestNamespace: "ns",
823+
expectedErr: "",
824+
expected: &operators.PackageManifestList{Items: []operators.PackageManifest{
825+
{
826+
ObjectMeta: metav1.ObjectMeta{
827+
Name: "prometheus",
828+
Namespace: "ns",
829+
Labels: labels.Set{
830+
"catalog": "cool-operators",
831+
"catalog-namespace": "ns",
832+
"provider": "Red Hat",
833+
"provider-url": "",
834+
"operatorframework.io/arch.amd64": "supported",
835+
"operatorframework.io/os.linux": "supported",
836+
},
837+
},
838+
Status: operators.PackageManifestStatus{
839+
CatalogSource: "cool-operators",
840+
CatalogSourceNamespace: "ns",
841+
PackageName: "prometheus",
842+
Provider: operators.AppLink{
843+
Name: "Red Hat",
844+
},
845+
DefaultChannel: "preview",
846+
Channels: []operators.PackageChannel{
847+
{
848+
Name: "preview",
849+
CurrentCSV: "prometheusoperator.0.22.2",
850+
CurrentCSVDesc: func() operators.CSVDescription {
851+
csv := operatorsv1alpha1.ClusterServiceVersion{}
852+
require.NoError(t, json.Unmarshal([]byte(prometheusCSVJSON), &csv))
853+
return operators.CreateCSVDescription(&csv)
854+
}(),
855+
},
856+
},
857+
},
858+
},
859+
{
860+
ObjectMeta: metav1.ObjectMeta{
861+
Name: "etcd",
862+
Namespace: "ns",
863+
Labels: labels.Set{
864+
"catalog": "cool-operators",
865+
"catalog-namespace": "ns",
866+
"provider": "CoreOS, Inc",
867+
"provider-url": "",
868+
"operatorframework.io/arch.amd64": "supported",
869+
"operatorframework.io/os.linux": "supported",
870+
},
871+
},
872+
Status: operators.PackageManifestStatus{
873+
CatalogSource: "cool-operators",
874+
CatalogSourceNamespace: "ns",
875+
PackageName: "etcd",
876+
Provider: operators.AppLink{
877+
Name: "CoreOS, Inc",
878+
},
879+
DefaultChannel: "alpha",
880+
Channels: []operators.PackageChannel{
881+
{
882+
Name: "alpha",
883+
CurrentCSV: "etcdoperator.v0.9.2",
884+
CurrentCSVDesc: func() operators.CSVDescription {
885+
csv := operatorsv1alpha1.ClusterServiceVersion{}
886+
require.NoError(t, json.Unmarshal([]byte(etcdCSVJSON), &csv))
887+
return operators.CreateCSVDescription(&csv)
888+
}(),
889+
},
890+
},
891+
},
892+
},
893+
{
894+
ObjectMeta: metav1.ObjectMeta{
895+
Name: "prometheus",
896+
Namespace: "ns",
897+
Labels: labels.Set{
898+
"catalog": "cool-operators-2",
899+
"catalog-namespace": "ns",
900+
"provider": "Red Hat",
901+
"provider-url": "",
902+
"operatorframework.io/arch.amd64": "supported",
903+
"operatorframework.io/os.linux": "supported",
904+
},
905+
},
906+
Status: operators.PackageManifestStatus{
907+
CatalogSource: "cool-operators-2",
908+
CatalogSourceNamespace: "ns",
909+
PackageName: "prometheus",
910+
Provider: operators.AppLink{
911+
Name: "Red Hat",
912+
},
913+
DefaultChannel: "preview",
914+
Channels: []operators.PackageChannel{
915+
{
916+
Name: "preview",
917+
CurrentCSV: "prometheusoperator.0.22.2",
918+
CurrentCSVDesc: func() operators.CSVDescription {
919+
csv := operatorsv1alpha1.ClusterServiceVersion{}
920+
require.NoError(t, json.Unmarshal([]byte(prometheusCSVJSON), &csv))
921+
return operators.CreateCSVDescription(&csv)
922+
}(),
923+
},
924+
},
925+
},
926+
},
927+
{
928+
ObjectMeta: metav1.ObjectMeta{
929+
Name: "etcd",
930+
Namespace: "ns",
931+
Labels: labels.Set{
932+
"catalog": "cool-operators-2",
933+
"catalog-namespace": "ns",
934+
"provider": "CoreOS, Inc",
935+
"provider-url": "",
936+
"operatorframework.io/arch.amd64": "supported",
937+
"operatorframework.io/os.linux": "supported",
938+
},
939+
},
940+
Status: operators.PackageManifestStatus{
941+
CatalogSource: "cool-operators-2",
942+
CatalogSourceNamespace: "ns",
943+
PackageName: "etcd",
944+
Provider: operators.AppLink{
945+
Name: "CoreOS, Inc",
946+
},
947+
DefaultChannel: "alpha",
948+
Channels: []operators.PackageChannel{
949+
{
950+
Name: "alpha",
951+
CurrentCSV: "etcdoperator.v0.9.2",
952+
CurrentCSVDesc: func() operators.CSVDescription {
953+
csv := operatorsv1alpha1.ClusterServiceVersion{}
954+
require.NoError(t, json.Unmarshal([]byte(etcdCSVJSON), &csv))
955+
return operators.CreateCSVDescription(&csv)
956+
}(),
957+
},
958+
},
959+
},
960+
},
961+
}},
962+
},
815963
{
816964
name: "OneCatalog/ManyPackages/OneMissingBundle/Elided",
817965
globalNS: "ns",

0 commit comments

Comments
 (0)