Allow restricting the Cache's ListWatch to be namespaced

[hasbro17]

Problem

Currently the ListWatch for the cache's informers are non-namespaced.
https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/cache/internal/informers_map.go#L218-L227

This means the Manager always requires cluster scoped permissions to work. While kubebuilder uses ClusterRole and ClusterRolebinding by default, that assumption isn't always true for an operator/controller (at least not in our context with the operator-sdk).

With just a Role and Rolebinding, the informers fail to list resources at the cluster scope.

E0828 23:41:19.472228       1 reflector.go:205] github.com/operator-framework/operator-sdk-samples/app-operator/vendor/sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:106: Failed to list *v1.Pod: pods is forbidden: User "system:serviceaccount:haseeb:default" cannot list pods at the cluster scope
E0828 23:41:20.141658       1 reflector.go:205] github.com/operator-framework/operator-sdk-samples/app-operator/vendor/sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:106: Failed to list *v1alpha1.App: apps.app.example.com is forbidden: User "system:serviceaccount:haseeb:default" cannot list apps.app.example.com at the cluster scope

Proposed Fix

Unless this is already supported or I've missed an easier way to do this, I've found that I can easily pipe down the namespace as an option from the Manager->Cache->InfromersMap->ListWatch.

mgr, err := manager.New(cfg, manager.Options{Namespace: namespace})

Possible fix: hasbro17@55894c2
That fixes the permissions issue as the ListWatch requests are now restricted to the desired namespace.

And in the default case of not specifying a namespace the ListWatch goes back to making cluster-scoped requests.
https://github.com/kubernetes/client-go/blob/master/rest/request.go#L424

[hasbro17]

If configuring all ListWatches via the manager options is not the right place to restrict namespaces then another place to do it might be at the Source.

c.Watch(&source.Kind{ Type: &corev1.Pod{}, Namespace: namespace}, &handler.EnqueueRequestForObject{})

We could namespace the Kind and then propagate that down to get a namespaced informer in the cache.

I don’t have a reference implementation for that yet but I think that would involve more substantial changes to the informers cache interface and informers map. Most likely changing the InformerMap key to be GVK+namespace.

I’ll try to think of a way of introducing namespaces into the Source and informers cache but if we could decide on the high level approach to this issue I’ll be glad to come up with a proposal/PR to move this forward.

/cc @droot @DirectXMan12

[DirectXMan12]

While the latter offers more control, I'm tempted to just go with the former for now (manager-level), since it makes the behavior consistent between c.Watch and c.client.Get on a new resource (which dynamically creates a watch). Thoughts?

[hasbro17]

c.client.Get on a new resource (which dynamically creates a watch)

@DirectXMan12 Did you mean c.cache.Get? That would dynamically create a new informer/watch. As opposed to c.client.Get which would just get it from the server.

But yes, I'm leaning towards doing it at the manager-level as well. That way all the Cache informers are consistently restricted to the desired namespace. For watched Kind Sources as well as cache.Get on previously unwatched resources.

One other potential issue is the question of restricting the cache to a set of namespaces.
My current proposal is just a manger-level switch between all-namespaces vs single-namespace. But I think multi-namespaces can still be addressed later on at the manager level. Possibly by changing the default ListerWatcher to a MultiListerWatcher that can handle a set of namespaces. But I can address that in a separate issue later on. Just wanted to point that out for now.

But for now if we can agree on the manager-level option for the namespace I'll put up a PR for that.

[DirectXMan12]

Did you mean c.cache.Get?

the "top-level" client is a "split" client that always calls c.cache.Get.

But I can address that in a separate issue later on. Just wanted to point that out for now.

Yeah, I think that's fine to tackle in a follow up if it's a highly desired feature, as long as we keep it in mind.

[DirectXMan12]

But for now if we can agree on the manager-level option for the namespace I'll put up a PR for that.

You can just repurpose this PR for that purpose.