pkg/informers: fix stylistic nits

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

pkg/informers/informers.go

@@ -15,12 +15,9 @@
 package informers
 
 import (
-	"strings"
-
 	"github.com/pkg/errors"
 	"github.com/prometheus-operator/prometheus-operator/pkg/listwatch"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -43,7 +40,7 @@ type FactoriesForNamespaces interface {
 	Namespaces() sets.String
 }
 
-// ForResource contains a list informers for a concrete resource type,
+// ForResource contains a slice of InformLister for a concrete resource type,
 // one per namespace.
 type ForResource struct {
 	informers []InformLister
@@ -62,7 +59,7 @@ func NewInformersForResource(ifs FactoriesForNamespaces, resource schema.GroupVe
 	for _, ns := range namespaces {
 		informer, err := ifs.ForResource(ns, resource)
 		if err != nil {
-			return nil, errors.Wrapf(err, "error getting informer for resource %v", resource)
+			return nil, errors.Wrapf(err, "error getting informer in namespace %q for resource %v", ns, resource)
 		}
 		informers = append(informers, informer)
 	}
@@ -153,10 +150,10 @@ func (w *ForResource) Get(name string) (runtime.Object, error) {
 	return nil, err
 }
 
-// newInformerOptions constructs a list option tweak function and a list of namespaces
+// newInformerOptions returns a list option tweak function and a list of namespaces
 // based on the given allowed and denied namespaces.
 //
-// If allowedNamespaces contains one entry containing k8s.io/apimachinery/pkg/apis/meta/v1.NamespaceAll
+// If allowedNamespaces contains one only entry equal to k8s.io/apimachinery/pkg/apis/meta/v1.NamespaceAll
 // then it returns it and a tweak function filtering denied namespaces using a field selector.
 //
 // Else, denied namespaces are ignored and just the set of allowed namespaces is returned.
@@ -168,12 +165,10 @@ func newInformerOptions(allowedNamespaces, deniedNamespaces map[string]struct{},
 	var namespaces []string
 
 	if listwatch.IsAllNamespaces(allowedNamespaces) {
-		namespaces = append(namespaces, v1.NamespaceAll)
-
 		return func(options *v1.ListOptions) {
 			tweaks(options)
-			denyNamespacesTweak(options, deniedNamespaces)
-		}, namespaces
+			listwatch.DenyTweak(options, "metadata.namespace", deniedNamespaces)
+		}, []string{v1.NamespaceAll}
 	}
 
 	for ns := range allowedNamespaces {
@@ -182,23 +177,3 @@ func newInformerOptions(allowedNamespaces, deniedNamespaces map[string]struct{},
 
 	return tweaks, namespaces
 }
-
-// denyNamespacesTweak is a simple method modifying the given list options
-// by adding a field selector not matching the given namespaces.
-func denyNamespacesTweak(options *metav1.ListOptions, namespaces map[string]struct{}) {
-	if len(namespaces) == 0 {
-		return
-	}
-
-	var selectors []string
-
-	for ns := range namespaces {
-		selectors = append(selectors, "metadata.namespace!="+ns)
-	}
-
-	if options.FieldSelector != "" {
-		selectors = append(selectors, options.FieldSelector)
-	}
-
-	options.FieldSelector = strings.Join(selectors, ",")
-}

pkg/informers/informers_test.go

@@ -205,23 +205,6 @@ func TestNewInformerOptions(t *testing.T) {
 				"denied2": {},
 				"denied1": {},
 			},
-
-			expectedNamespaces: []string{
-				v1.NamespaceAll,
-			},
-			expectedOptions: v1.ListOptions{
-				FieldSelector: "metadata.namespace!=denied1,metadata.namespace!=denied2",
-			},
-		},
-		{
-			name: "all allowed namespaces no denied namespaces",
-			allowedNamespaces: map[string]struct{}{
-				v1.NamespaceAll: {},
-			},
-			deniedNamespaces: map[string]struct{}{
-				"denied2": {},
-				"denied1": {},
-			},
 			tweaks: func(options *v1.ListOptions) {
 				options.FieldSelector = "metadata.name=foo"
 			},

pkg/listwatch/listwatch.go

@@ -70,9 +70,7 @@ func NewFilteredUnprivilegedNamespaceListWatchFromClient(l log.Logger, c cache.G
 				optionsModifier(options)
 			}
 
-			fieldSelector := options.FieldSelector
-			denyNamesTweak(options, deniedNamespaces)
-			options.FieldSelector = strings.Join([]string{options.FieldSelector, fieldSelector}, ",")
+			DenyTweak(options, "metadata.name", deniedNamespaces)
 		}
 
 		return cache.NewFilteredListWatchFromClient(c, "namespaces", metav1.NamespaceAll, tweak)
@@ -127,16 +125,22 @@ func IdenticalNamespaces(a, b map[string]struct{}) bool {
 	return true
 }
 
-func denyNamesTweak(options *metav1.ListOptions, names map[string]struct{}) {
-	if len(names) == 0 {
+// DenyTweak modifies the given list options
+// by adding a field selector not matching the given values.
+func DenyTweak(options *metav1.ListOptions, field string, valueSet map[string]struct{}) {
+	if len(valueSet) == 0 {
 		return
 	}
 
-	var denied []string
+	var selectors []string
 
-	for ns := range names {
-		denied = append(denied, "metadata.name!="+ns)
+	for value := range valueSet {
+		selectors = append(selectors, field+"!="+value)
 	}
 
-	options.FieldSelector = strings.Join(denied, ",")
+	if options.FieldSelector != "" {
+		selectors = append(selectors, options.FieldSelector)
+	}
+
+	options.FieldSelector = strings.Join(selectors, ",")
 }

pkg/prometheus/operator.go

@@ -349,7 +349,7 @@ func New(ctx context.Context, conf Config, logger log.Logger, r prometheus.Regis
 			c.kclient,
 			resyncPeriod,
 			func(options *metav1.ListOptions) {
-				options.FieldSelector = strings.Join([]string{options.FieldSelector, secretListWatchSelector.String()}, ",")
+				options.FieldSelector = secretListWatchSelector.String()
 			},
 		),
 		v1.SchemeGroupVersion.WithResource(string(v1.ResourceSecrets)),