fix(operator): Skip workloads in the operator namespace (#362)

Signed-off-by: Daniel Pacak <pacak.daniel@gmail.com>

pkg/operator/controller/configauditreport.go

@@ -35,11 +35,16 @@ type ConfigAuditReportReconciler struct {
 }
 
 func (r *ConfigAuditReportReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	err := ctrl.NewControllerManagedBy(mgr).
+	installModePredicate, err := InstallModePredicate(r.Config)
+	if err != nil {
+		return err
+	}
+	err = ctrl.NewControllerManagedBy(mgr).
 		For(&corev1.Pod{}, builder.WithPredicates(
 			Not(ManagedByStarboardOperator),
 			Not(PodBeingTerminated),
 			PodHasContainersReadyCondition,
+			installModePredicate,
 		)).
 		Complete(r.reconcilePods())
 	if err != nil {

pkg/operator/controller/vulnerabilityreport.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
 	"github.com/aquasecurity/starboard/pkg/docker"
-	"github.com/aquasecurity/starboard/pkg/ext"
 	"github.com/aquasecurity/starboard/pkg/kube"
 	"github.com/aquasecurity/starboard/pkg/operator/etc"
 	. "github.com/aquasecurity/starboard/pkg/operator/predicate"
@@ -18,7 +17,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -40,11 +38,16 @@ type VulnerabilityReportReconciler struct {
 }
 
 func (r *VulnerabilityReportReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	err := ctrl.NewControllerManagedBy(mgr).
+	installModePredicate, err := InstallModePredicate(r.Config)
+	if err != nil {
+		return err
+	}
+	err = ctrl.NewControllerManagedBy(mgr).
 		For(&corev1.Pod{}, builder.WithPredicates(
 			Not(ManagedByStarboardOperator),
 			Not(PodBeingTerminated),
 			PodHasContainersReadyCondition,
+			installModePredicate,
 		)).
 		Complete(r.reconcilePods())
 	if err != nil {
@@ -66,19 +69,8 @@ func (r *VulnerabilityReportReconciler) reconcilePods() reconcile.Func {
 
 		pod := &corev1.Pod{}
 
-		installMode, err := r.GetInstallMode()
-		if err != nil {
-			return ctrl.Result{}, fmt.Errorf("getting install mode: %w", err)
-		}
-
-		// TODO Consider using a predicate
-		if r.IgnorePodInOperatorNamespace(installMode, req.NamespacedName) {
-			log.V(1).Info("Ignoring Pod run in the operator namespace")
-			return ctrl.Result{}, nil
-		}
-
 		// Retrieve the Pod from cache.
-		err = r.Get(ctx, req.NamespacedName, pod)
+		err := r.Get(ctx, req.NamespacedName, pod)
 		if err != nil {
 			if errors.IsNotFound(err) {
 				log.V(1).Info("Ignoring pod that must have been deleted")
@@ -148,7 +140,7 @@ func (r *VulnerabilityReportReconciler) hasVulnerabilityReports(ctx context.Cont
 	}
 
 	expected := map[string]bool{}
-	for containerName:= range images {
+	for containerName := range images {
 		expected[containerName] = true
 	}
 
@@ -376,32 +368,3 @@ func (r *VulnerabilityReportReconciler) processFailedScanJob(ctx context.Context
 	log.V(1).Info("Deleting failed scan job")
 	return r.Client.Delete(ctx, scanJob, client.PropagationPolicy(metav1.DeletePropagationBackground))
 }
-
-// IgnorePodInOperatorNamespace determines whether to reconcile the specified Pod
-// based on the give InstallMode or not. Returns true if the Pod should be ignored,
-// false otherwise.
-//
-// In the SingleNamespace install mode we're configuring Client cache
-// to watch the operator namespace, in which the operator runs scan Jobs.
-// However, we do not want to scan the workloads that might run in the
-// operator namespace.
-//
-// In the MultiNamespace install mode we're configuring Client cache
-// to watch the operator namespace, in which the operator runs scan Jobs.
-// However, we do not want to scan the workloads that might run in the
-// operator namespace unless the operator namespace is added to the list
-// of target namespaces.
-func (r *VulnerabilityReportReconciler) IgnorePodInOperatorNamespace(installMode etc.InstallMode, pod types.NamespacedName) bool {
-	if installMode == etc.InstallModeSingleNamespace &&
-		pod.Namespace == r.Namespace {
-		return true
-	}
-
-	if installMode == etc.InstallModeMultiNamespace &&
-		pod.Namespace == r.Namespace &&
-		!ext.SliceContainsString(r.GetTargetNamespaces(), r.Namespace) {
-		return true
-	}
-
-	return false
-}

pkg/operator/etc/config.go

@@ -48,28 +48,28 @@ func (c Config) GetTargetNamespaces() []string {
 type InstallMode string
 
 const (
-	InstallModeOwnNamespace    InstallMode = "OwnNamespace"
-	InstallModeSingleNamespace InstallMode = "SingleNamespace"
-	InstallModeMultiNamespace  InstallMode = "MultiNamespace"
-	InstallModeAllNamespaces   InstallMode = "AllNamespaces"
+	OwnNamespace    InstallMode = "OwnNamespace"
+	SingleNamespace InstallMode = "SingleNamespace"
+	MultiNamespace  InstallMode = "MultiNamespace"
+	AllNamespaces   InstallMode = "AllNamespaces"
 )
 
-// GetInstallMode resolves InstallMode based on configured operator and target namespaces.
-func (c Config) GetInstallMode() (InstallMode, error) {
+// ResolveInstallMode resolves InstallMode based on configured Config.Namespace and Config.TargetNamespaces.
+func (c Config) ResolveInstallMode() (InstallMode, string, []string, error) {
 	operatorNamespace, err := c.GetOperatorNamespace()
 	if err != nil {
-		return "", nil
+		return "", "", nil, err
 	}
 	targetNamespaces := c.GetTargetNamespaces()
 
 	if len(targetNamespaces) == 1 && operatorNamespace == targetNamespaces[0] {
-		return InstallModeOwnNamespace, nil
+		return OwnNamespace, operatorNamespace, targetNamespaces, nil
 	}
 	if len(targetNamespaces) == 1 && operatorNamespace != targetNamespaces[0] {
-		return InstallModeSingleNamespace, nil
+		return SingleNamespace, operatorNamespace, targetNamespaces, nil
 	}
 	if len(targetNamespaces) > 1 {
-		return InstallModeMultiNamespace, nil
+		return MultiNamespace, operatorNamespace, targetNamespaces, nil
 	}
-	return InstallModeAllNamespaces, nil
+	return AllNamespaces, operatorNamespace, targetNamespaces, nil
 }

pkg/operator/etc/config_test.go

@@ -44,7 +44,7 @@ func TestOperator_GetTargetNamespaces(t *testing.T) {
 	}
 }
 
-func TestOperator_GetInstallMode(t *testing.T) {
+func TestOperator_ResolveInstallMode(t *testing.T) {
 	testCases := []struct {
 		name string
 
@@ -58,7 +58,7 @@ func TestOperator_GetInstallMode(t *testing.T) {
 				Namespace:        "operators",
 				TargetNamespaces: "operators",
 			},
-			expectedInstallMode: etc.InstallModeOwnNamespace,
+			expectedInstallMode: etc.OwnNamespace,
 			expectedError:       "",
 		},
 		{
@@ -67,7 +67,7 @@ func TestOperator_GetInstallMode(t *testing.T) {
 				Namespace:        "operators",
 				TargetNamespaces: "foo",
 			},
-			expectedInstallMode: etc.InstallModeSingleNamespace,
+			expectedInstallMode: etc.SingleNamespace,
 			expectedError:       "",
 		},
 		{
@@ -76,7 +76,7 @@ func TestOperator_GetInstallMode(t *testing.T) {
 				Namespace:        "operators",
 				TargetNamespaces: "foo,bar,baz",
 			},
-			expectedInstallMode: etc.InstallModeMultiNamespace,
+			expectedInstallMode: etc.MultiNamespace,
 			expectedError:       "",
 		},
 		{
@@ -85,14 +85,14 @@ func TestOperator_GetInstallMode(t *testing.T) {
 				Namespace:        "operators",
 				TargetNamespaces: "",
 			},
-			expectedInstallMode: etc.InstallModeAllNamespaces,
+			expectedInstallMode: etc.AllNamespaces,
 			expectedError:       "",
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			installMode, err := tc.operator.GetInstallMode()
+			installMode, _, _, err := tc.operator.ResolveInstallMode()
 			switch tc.expectedError {
 			case "":
 				require.NoError(t, err)

pkg/operator/operator.go

@@ -26,19 +26,11 @@ var (
 func Run(buildInfo starboard.BuildInfo, operatorConfig etc.Config) error {
 	setupLog.Info("Starting operator", "buildInfo", buildInfo)
 
-	// Validate configured namespaces to resolve install mode.
-	operatorNamespace, err := operatorConfig.GetOperatorNamespace()
+	installMode, operatorNamespace, targetNamespaces, err := operatorConfig.ResolveInstallMode()
 	if err != nil {
-		return fmt.Errorf("getting operator namespace: %w", err)
+		return fmt.Errorf("resolving install mode: %w", err)
 	}
-
-	targetNamespaces := operatorConfig.GetTargetNamespaces()
-
-	installMode, err := operatorConfig.GetInstallMode()
-	if err != nil {
-		return fmt.Errorf("getting install mode: %w", err)
-	}
-	setupLog.Info("Resolving install mode", "install mode", installMode,
+	setupLog.Info("Resolved install mode", "install mode", installMode,
 		"operator namespace", operatorNamespace,
 		"target namespaces", targetNamespaces)
 
@@ -50,26 +42,30 @@ func Run(buildInfo starboard.BuildInfo, operatorConfig etc.Config) error {
 	}
 
 	switch installMode {
-	case etc.InstallModeOwnNamespace:
-		// Add support for OwnNamespace set in STARBOARD_NAMESPACE (e.g. marketplace) and STARBOARD_TARGET_NAMESPACES (e.g. marketplace)
-		setupLog.Info("Constructing single-namespaced cache", "namespace", targetNamespaces[0])
+	case etc.OwnNamespace:
+		// Add support for OwnNamespace set in OPERATOR_NAMESPACE (e.g. `starboard-operator`)
+		// and OPERATOR_TARGET_NAMESPACES (e.g. `starboard-operator`).
+		setupLog.Info("Constructing client cache", "namespace", targetNamespaces[0])
 		options.Namespace = targetNamespaces[0]
-	case etc.InstallModeSingleNamespace:
-		// Add support for SingleNamespace set in STARBOARD_NAMESPACE (e.g. marketplace) and STARBOARD_TARGET_NAMESPACES (e.g. foo)
+	case etc.SingleNamespace:
+		// Add support for SingleNamespace set in OPERATOR_NAMESPACE (e.g. `starboard-operator`)
+		// and OPERATOR_TARGET_NAMESPACES (e.g. `default`).
 		cachedNamespaces := append(targetNamespaces, operatorNamespace)
-		setupLog.Info("Constructing multi-namespaced cache", "namespaces", cachedNamespaces)
+		setupLog.Info("Constructing client cache", "namespaces", cachedNamespaces)
 		options.Namespace = targetNamespaces[0]
 		options.NewCache = cache.MultiNamespacedCacheBuilder(cachedNamespaces)
-	case etc.InstallModeMultiNamespace:
-		// Add support for MultiNamespace set in STARBOARD_NAMESPACE (e.g. marketplace) and STARBOARD_TARGET_NAMESPACES (e.g. foo,bar).
-		// Note that we may face performance issues when using this with a high number of namespaces.
+	case etc.MultiNamespace:
+		// Add support for MultiNamespace set in OPERATOR_NAMESPACE (e.g. `starboard-operator`)
+		// and OPERATOR_TARGET_NAMESPACES (e.g. `default,kube-system`).
+		// Note that you may face performance issues when using this mode with a high number of namespaces.
 		// More: https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/cache#MultiNamespacedCacheBuilder
 		cachedNamespaces := append(targetNamespaces, operatorNamespace)
-		setupLog.Info("Constructing multi-namespaced cache", "namespaces", cachedNamespaces)
+		setupLog.Info("Constructing client cache", "namespaces", cachedNamespaces)
 		options.Namespace = ""
 		options.NewCache = cache.MultiNamespacedCacheBuilder(cachedNamespaces)
-	case etc.InstallModeAllNamespaces:
-		// Add support for AllNamespaces set in STARBOARD_NAMESPACE (e.g. marketplace) and STARBOARD_TARGET_NAMESPACES left blank.
+	case etc.AllNamespaces:
+		// Add support for AllNamespaces set in OPERATOR_NAMESPACE (e.g. `operators`)
+		// and OPERATOR_TARGET_NAMESPACES left blank.
 		setupLog.Info("Watching all namespaces")
 		options.Namespace = ""
 	default:

pkg/operator/predicate/predicate.go

@@ -1,14 +1,48 @@
 package predicate
 
 import (
+	"github.com/aquasecurity/starboard/pkg/ext"
 	"github.com/aquasecurity/starboard/pkg/kube"
+	"github.com/aquasecurity/starboard/pkg/operator/etc"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
+// InstallModePredicate is a predicate.Predicate that determines whether to
+// reconcile the specified client.Object based on the give etc.InstallMode.
+//
+// In the etc.SingleNamespace install mode we're configuring client.Client cache
+// to watch the operator namespace, in which the operator runs scan jobs.
+// However, we do not want to scan the workloads that might run in the
+// operator namespace.
+//
+// Similarly, in the etc.MultiNamespace install mode we're configuring
+// client.Client cache to watch the operator namespace, in which the operator
+// runs scan jobs. However, we do not want to scan the workloads that might run
+// in the operator namespace unless the operator namespace is added to the list
+// of target namespaces.
+var InstallModePredicate = func(config etc.Config) (predicate.Predicate, error) {
+	mode, operatorNamespace, targetNamespaces, err := config.ResolveInstallMode()
+	if err != nil {
+		return nil, err
+	}
+	return predicate.NewPredicateFuncs(func(obj client.Object) bool {
+		if mode == etc.SingleNamespace {
+			return targetNamespaces[0] == obj.GetNamespace() &&
+				operatorNamespace != obj.GetNamespace()
+		}
+
+		if mode == etc.MultiNamespace {
+			return ext.SliceContainsString(targetNamespaces, obj.GetNamespace())
+		}
+
+		return true
+	}), nil
+}
+
 // InNamespace is a predicate.Predicate that returns true if the
 // specified client.Object is in the desired namespace.
 var InNamespace = func(namespace string) predicate.Predicate {