fix: fix setting secret or configmap label selector to empty (#2815) (#2826)

Author: pmalek

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Table of Contents
 
+- [v2.0.7](#v207)
 - [v2.0.6](#v206)
 - [v2.0.5](#v205)
 - [v2.0.4](#v204)
@@ -36,6 +37,16 @@
 - [v0.1.1](#v011)
 - [v0.1.0](#v010)
 
+## [v2.0.7]
+
+> Release date: TBD
+
+### Fixed
+
+- Fixed an issue where users could set the secret of configmap label selectors
+  to empty when the other one was left non-empty.
+  [#2815](https://github.com/Kong/kong-operator/pull/2815)
+
 ## [v2.0.6]
 
 > Release date: 2025-12-01
@@ -1415,6 +1426,7 @@ leftovers from previous operator deployments in the cluster. The user needs to d
 (clusterrole, clusterrolebinding, validatingWebhookConfiguration) before
 re-installing the operator through the bundle.
 
+[v2.0.7]: https://github.com/Kong/kong-operator/compare/v2.0.6..v2.0.7
 [v2.0.6]: https://github.com/Kong/kong-operator/compare/v2.0.5..v2.0.6
 [v2.0.5]: https://github.com/Kong/kong-operator/compare/v2.0.4..v2.0.5
 [v2.0.4]: https://github.com/Kong/kong-operator/compare/v2.0.3..v2.0.4

modules/manager/cache_options.go

@@ -0,0 +1,58 @@
+package manager
+
+import (
+	"fmt"
+
+	"github.com/go-logr/logr"
+	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func createCacheOptions(l logr.Logger, cfg Config) (cache.Options, error) {
+	var cacheOptions cache.Options
+	if cfg.CacheSyncPeriod > 0 {
+		l.Info("cache sync period set", "period", cfg.CacheSyncPeriod)
+		cacheOptions.SyncPeriod = &cfg.CacheSyncPeriod
+	}
+
+	// If there are no configured watch namespaces, then we're watching ALL namespaces,
+	// and we don't have to bother individually caching any particular namespaces.
+	// This is the default behavior of the controller-runtime manager.
+	// If there are configured watch namespaces, then we're watching only those namespaces.
+	if len(cfg.WatchNamespaces) > 0 {
+		l.Info("Manager set up with multiple namespaces", "namespaces", cfg.WatchNamespaces)
+		watched := make(map[string]cache.Config)
+		for _, ns := range cfg.WatchNamespaces {
+			watched[ns] = cache.Config{}
+		}
+		cacheOptions.DefaultNamespaces = watched
+	}
+
+	cacheByObject, err := createCacheByObject(cfg)
+	if err != nil {
+		return cacheOptions, fmt.Errorf("failed to create cache options: %w", err)
+	}
+	cacheOptions.ByObject = cacheByObject
+
+	return cacheOptions, nil
+}
+
+func createCacheByObject(cfg Config) (map[client.Object]cache.ByObject, error) {
+	if cfg.ConfigMapLabelSelector == "" && cfg.SecretLabelSelector == "" {
+		return nil, nil
+	}
+	byObject := map[client.Object]cache.ByObject{}
+	if cfg.SecretLabelSelector != "" {
+		if err := setByObjectFor[corev1.Secret](cfg.SecretLabelSelector, byObject); err != nil {
+			return nil, fmt.Errorf("failed to set byObject for Secrets: %w", err)
+		}
+	}
+	if cfg.ConfigMapLabelSelector != "" {
+		if err := setByObjectFor[corev1.ConfigMap](cfg.ConfigMapLabelSelector, byObject); err != nil {
+			return nil, fmt.Errorf("failed to set byObject for ConfigMaps: %w", err)
+		}
+	}
+
+	return byObject, nil
+}

modules/manager/cache_options_test.go

@@ -0,0 +1,112 @@
+package manager
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+)
+
+func TestCreateCacheByObject(t *testing.T) {
+	tests := []struct {
+		name                   string
+		cfg                    Config
+		expectError            bool
+		expectNil              bool
+		expectedConfigMapLabel string
+		expectedSecretLabel    string
+	}{
+		{
+			name: "no label selectors returns nil",
+			cfg: Config{
+				ConfigMapLabelSelector: "",
+				SecretLabelSelector:    "",
+			},
+			expectError: false,
+			expectNil:   true,
+		},
+		{
+			name: "only secret label selector",
+			cfg: Config{
+				SecretLabelSelector: "app",
+			},
+			expectError:         false,
+			expectNil:           false,
+			expectedSecretLabel: "app",
+		},
+		{
+			name: "only configmap label selector",
+			cfg: Config{
+				ConfigMapLabelSelector: "configmap.konghq.com",
+			},
+			expectError:            false,
+			expectNil:              false,
+			expectedConfigMapLabel: "configmap.konghq.com",
+		},
+		{
+			name: "both label selectors",
+			cfg: Config{
+				ConfigMapLabelSelector: "configmap.konghq.com",
+				SecretLabelSelector:    "app",
+			},
+			expectError:            false,
+			expectNil:              false,
+			expectedConfigMapLabel: "configmap.konghq.com",
+			expectedSecretLabel:    "app",
+		},
+		{
+			name: "invalid secret label selector",
+			cfg: Config{
+				SecretLabelSelector: "invalid==label",
+			},
+			expectError: true,
+		},
+		{
+			name: "invalid configmap label selector",
+			cfg: Config{
+				ConfigMapLabelSelector: "invalid==label",
+			},
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := createCacheByObject(tt.cfg)
+
+			if tt.expectError {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+
+			if tt.expectNil {
+				require.Nil(t, result)
+				return
+			}
+
+			require.NotNil(t, result)
+
+			if tt.expectedSecretLabel != "" {
+				for obj, v := range result {
+					if _, ok := obj.(*corev1.Secret); ok {
+						r, _ := v.Label.Requirements()
+						require.Len(t, r, 1)
+						require.Equal(t, tt.expectedSecretLabel, r[0].Key())
+					}
+				}
+			}
+
+			if tt.expectedConfigMapLabel != "" {
+				for obj, v := range result {
+					if _, ok := obj.(*corev1.ConfigMap); ok {
+						r, _ := v.Label.Requirements()
+						require.Len(t, r, 1)
+						require.Equal(t, tt.expectedConfigMapLabel, r[0].Key())
+					}
+				}
+			}
+		})
+	}
+}

modules/manager/run.go

@@ -205,33 +205,9 @@ func Run(
 		return err
 	}
 
-	var cacheOptions cache.Options
-	if cfg.CacheSyncPeriod > 0 {
-		setupLog.Info("cache sync period set", "period", cfg.CacheSyncPeriod)
-		cacheOptions.SyncPeriod = &cfg.CacheSyncPeriod
-	}
-
-	// If there are no configured watch namespaces, then we're watching ALL namespaces,
-	// and we don't have to bother individually caching any particular namespaces.
-	// This is the default behavior of the controller-runtime manager.
-	// If there are configured watch namespaces, then we're watching only those namespaces.
-	if len(cfg.WatchNamespaces) > 0 {
-		setupLog.Info("Manager set up with multiple namespaces", "namespaces", cfg.WatchNamespaces)
-		watched := make(map[string]cache.Config)
-		for _, ns := range cfg.WatchNamespaces {
-			watched[ns] = cache.Config{}
-		}
-		cacheOptions.DefaultNamespaces = watched
-	}
-
-	if cfg.ConfigMapLabelSelector != "" || cfg.SecretLabelSelector != "" {
-		cacheOptions.ByObject = map[client.Object]cache.ByObject{}
-		if err := setByObjectFor[corev1.Secret](cfg.SecretLabelSelector, cacheOptions.ByObject); err != nil {
-			return fmt.Errorf("failed to set byObject for Secrets: %w", err)
-		}
-		if err := setByObjectFor[corev1.ConfigMap](cfg.ConfigMapLabelSelector, cacheOptions.ByObject); err != nil {
-			return fmt.Errorf("failed to set byObject for ConfigMaps: %w", err)
-		}
+	cacheOptions, err := createCacheOptions(setupLog, cfg)
+	if err != nil {
+		return fmt.Errorf("failed to create cache options: %w", err)
 	}
 
 	managerOpts := ctrl.Options{