Skip to content

Commit 1a8bba6

Browse files
committed
fix MultiScopedCache
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
1 parent 6df5af3 commit 1a8bba6

File tree

4 files changed

+62
-22
lines changed

4 files changed

+62
-22
lines changed

cmd/app/app.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,13 @@ func NewCommand() *cobra.Command {
6060
eventBroadcaster.StartLogging(func(format string, args ...interface{}) { mlog.V(3).Info(fmt.Sprintf(format, args...)) })
6161
eventBroadcaster.StartRecordingToSink(&clientv1.EventSinkImpl{Interface: cl.CoreV1().Events("")})
6262

63+
scheme := trustapi.GlobalScheme
6364
mgr, err := ctrl.NewManager(opts.RestConfig, ctrl.Options{
64-
Scheme: trustapi.GlobalScheme,
65+
Scheme: scheme,
6566
EventBroadcaster: eventBroadcaster,
6667
LeaderElection: true,
6768
LeaderElectionNamespace: opts.Bundle.Namespace,
68-
NewCache: bundle.NewCacheFunc(opts.Bundle),
69-
ClientDisableCacheFor: bundle.ClientDisableCacheFor(),
69+
NewCache: bundle.NewCacheFunc(scheme, opts.Bundle),
7070
LeaderElectionID: "trust-manager-leader-election",
7171
LeaderElectionReleaseOnCancel: true,
7272
ReadinessEndpointName: opts.ReadyzPath,

pkg/bundle/controller.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
corev1 "k8s.io/api/core/v1"
2525
apierrors "k8s.io/apimachinery/pkg/api/errors"
26+
"k8s.io/apimachinery/pkg/runtime"
2627
"k8s.io/apimachinery/pkg/runtime/schema"
2728
"k8s.io/apimachinery/pkg/types"
2829
"k8s.io/utils/clock"
@@ -194,12 +195,6 @@ func (b *bundle) mustBundleList(ctx context.Context) *trustapi.BundleList {
194195

195196
// NewCacheFunc will return a multi-scoped controller-runtime NewCacheFunc
196197
// where Secret resources will only be watched within the trust Namespace.
197-
func NewCacheFunc(opts Options) cache.NewCacheFunc {
198-
return internal.NewMultiScopedCache(opts.Namespace, []schema.GroupKind{{Kind: "Secret"}})
199-
}
200-
201-
// ClientDisableCacheFor returns resources which should only be watched within
202-
// the Trust Namespace, and not at the cluster level.
203-
func ClientDisableCacheFor() []client.Object {
204-
return []client.Object{new(corev1.Secret)}
198+
func NewCacheFunc(scheme *runtime.Scheme, opts Options) cache.NewCacheFunc {
199+
return internal.NewMultiScopedCache(scheme, opts.Namespace, []schema.GroupKind{{Kind: "Secret"}})
205200
}

pkg/bundle/internal/cache.go

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@ package internal
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"sync"
2223

24+
"k8s.io/apimachinery/pkg/runtime"
2325
"k8s.io/apimachinery/pkg/runtime/schema"
2426
utilerrors "k8s.io/apimachinery/pkg/util/errors"
2527
"k8s.io/client-go/rest"
@@ -36,6 +38,9 @@ var _ cache.Cache = &multiScopedCache{}
3638
// namespace, whilst the other Namespaced resources in all namespaces.
3739
// It wraps both the default and Namespaced controller-runtime Cache.
3840
type multiScopedCache struct {
41+
// scheme is the scheme used to determine the GVK for objects.
42+
scheme *runtime.Scheme
43+
3944
// namespacedInformers is the set of resource types that should only be
4045
// watched in the namespace pool.
4146
namespacedInformers []schema.GroupKind
@@ -51,23 +56,23 @@ type multiScopedCache struct {
5156
// resources in the given namespace. namespacedInformers is the set of resource
5257
// types which should only be watched in the given namespace.
5358
// namespacedInformers expects Namespaced resource types.
54-
func NewMultiScopedCache(namespace string, namespacedInformers []schema.GroupKind) cache.NewCacheFunc {
59+
func NewMultiScopedCache(scheme *runtime.Scheme, namespace string, namespacedInformers []schema.GroupKind) cache.NewCacheFunc {
5560
return func(config *rest.Config, opts cache.Options) (cache.Cache, error) {
56-
namespacedCache, err := cache.New(config, cache.Options{
57-
Scheme: opts.Scheme,
58-
Mapper: opts.Mapper,
59-
Namespace: namespace,
60-
Resync: opts.Resync,
61-
SelectorsByObject: opts.SelectorsByObject,
62-
})
61+
namespacedOpts := opts
62+
namespacedOpts.Namespace = namespace
63+
clusterOpts := opts
64+
clusterOpts.Namespace = ""
65+
66+
namespacedCache, err := cache.New(config, namespacedOpts)
6367
if err != nil {
6468
return nil, err
6569
}
66-
clusterCache, err := cache.New(config, opts)
70+
clusterCache, err := cache.New(config, clusterOpts)
6771
if err != nil {
6872
return nil, err
6973
}
7074
return &multiScopedCache{
75+
scheme: scheme,
7176
namespacedCache: namespacedCache,
7277
clusterCache: clusterCache,
7378
namespacedInformers: namespacedInformers,
@@ -77,6 +82,9 @@ func NewMultiScopedCache(namespace string, namespacedInformers []schema.GroupKin
7782

7883
// GetInformer returns the underlying cache's GetInformer based on resource type.
7984
func (b *multiScopedCache) GetInformer(ctx context.Context, obj client.Object) (cache.Informer, error) {
85+
if err := setGroupVersionKind(b.scheme, obj); err != nil {
86+
return nil, err
87+
}
8088
return b.cacheFromGVK(obj.GetObjectKind().GroupVersionKind()).GetInformer(ctx, obj)
8189
}
8290

@@ -126,26 +134,62 @@ func (b *multiScopedCache) WaitForCacheSync(ctx context.Context) bool {
126134

127135
// IndexField returns the underlying cache's IndexField based on resource type.
128136
func (b *multiScopedCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
137+
if err := setGroupVersionKind(b.scheme, obj); err != nil {
138+
return err
139+
}
129140
return b.cacheFromGVK(obj.GetObjectKind().GroupVersionKind()).IndexField(ctx, obj, field, extractValue)
130141
}
131142

132143
// Get returns the underlying cache's Get based on resource type.
133144
func (b *multiScopedCache) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
145+
if err := setGroupVersionKind(b.scheme, obj); err != nil {
146+
return err
147+
}
134148
return b.cacheFromGVK(obj.GetObjectKind().GroupVersionKind()).Get(ctx, key, obj)
135149
}
136150

137151
// List returns the underlying cache's List based on resource type.
138152
func (b *multiScopedCache) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
153+
if err := setGroupVersionKind(b.scheme, list); err != nil {
154+
return err
155+
}
139156
return b.cacheFromGVK(list.GetObjectKind().GroupVersionKind()).List(ctx, list, opts...)
140157
}
141158

142159
// cacheFromGVK returns either the cluster or namespaced cache, based on the
143160
// resource type given.
144161
func (b *multiScopedCache) cacheFromGVK(gvk schema.GroupVersionKind) cache.Cache {
162+
if gvk.Group == "" && gvk.Kind == "" {
163+
panic("The Group and/or Kind must be set")
164+
}
165+
145166
for _, namespacedInformer := range b.namespacedInformers {
146167
if namespacedInformer.Group == gvk.Group && namespacedInformer.Kind == gvk.Kind {
147168
return b.namespacedCache
148169
}
149170
}
150171
return b.clusterCache
151172
}
173+
174+
// setGroupVersionKind populates the Group and Kind fields of obj using the
175+
// scheme type registry.
176+
// Inspired by https://github.com/kubernetes-sigs/controller-runtime/issues/1735#issuecomment-984763173
177+
func setGroupVersionKind(scheme *runtime.Scheme, obj runtime.Object) error {
178+
gvk := obj.GetObjectKind().GroupVersionKind()
179+
if gvk.Group != "" || gvk.Kind != "" {
180+
return nil // eg. in case of PartialMetadata, we don't want to overwrite the Group/ Kind
181+
}
182+
183+
gvks, unversioned, err := scheme.ObjectKinds(obj)
184+
if err != nil {
185+
return err
186+
}
187+
if unversioned {
188+
return fmt.Errorf("ObjectKinds unexpectedly returned unversioned: %#v", unversioned)
189+
}
190+
if len(gvks) != 1 {
191+
return fmt.Errorf("ObjectKinds unexpectedly returned zero or multiple gvks: %#v", gvks)
192+
}
193+
obj.GetObjectKind().SetGroupVersionKind(gvks[0])
194+
return nil
195+
}

test/integration/bundle/suite.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,10 @@ var _ = Describe("Integration", func() {
9090
DefaultPackageLocation: tmpFileName,
9191
}
9292

93+
scheme := trustapi.GlobalScheme
9394
mgr, err = ctrl.NewManager(env.Config, ctrl.Options{
94-
Scheme: trustapi.GlobalScheme,
95-
NewCache: bundle.NewCacheFunc(opts),
95+
Scheme: scheme,
96+
NewCache: bundle.NewCacheFunc(scheme, opts),
9697
// TODO: can we disable leader election here? The mgr goroutine prints extra output we probably don't need
9798
// and it might not be valuable to enable leader election here
9899
LeaderElection: true,

0 commit comments

Comments
 (0)