From 1eb4e4a55bfdc82688583ea7e3bcac5862ebf217 Mon Sep 17 00:00:00 2001 From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com> Date: Wed, 19 Nov 2025 07:33:09 +0000 Subject: [PATCH] fix: ClusterExtension configurations using watchNamespace: "" (empty string) no longer fail with a confusing validation error. --- internal/operator-controller/config/config.go | 17 ++- .../operator-controller/config/config_test.go | 21 +++ test/e2e/single_namespace_support_test.go | 139 ++++++++++++++++++ 3 files changed, 176 insertions(+), 1 deletion(-) diff --git a/internal/operator-controller/config/config.go b/internal/operator-controller/config/config.go index 8fcadf40ad..ed2a4b63b8 100644 --- a/internal/operator-controller/config/config.go +++ b/internal/operator-controller/config/config.go @@ -79,7 +79,8 @@ func newConfig(data map[string]any) *Config { } // GetWatchNamespace returns the watchNamespace value if present in the configuration. -// Returns nil if watchNamespace is not set or is explicitly set to null. +// Returns nil if watchNamespace is not set, is explicitly set to null, or is set to an empty string. +// An empty string is treated as equivalent to "not configured" (AllNamespaces mode). func (c *Config) GetWatchNamespace() *string { if c == nil || *c == nil { return nil @@ -95,6 +96,10 @@ func (c *Config) GetWatchNamespace() *string { // Convert value to string. Schema validation ensures this is a string, // but fmt.Sprintf handles edge cases defensively. str := fmt.Sprintf("%v", val) + // Treat empty string as "not configured" (AllNamespaces mode) + if str == "" { + return nil + } return &str } @@ -166,6 +171,11 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install if !ok { return fmt.Errorf("value must be a string") } + // Empty string is treated as "not configured" (AllNamespaces mode) + // Skip validation for empty strings - they'll be normalized by GetWatchNamespace() + if str == "" { + return nil + } if str != installNamespace { return fmt.Errorf("invalid value %q: watchNamespace must be %q (the namespace where the operator is installed) because this operator only supports OwnNamespace install mode", str, installNamespace) } @@ -186,6 +196,11 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install if !ok { return fmt.Errorf("value must be a string") } + // Empty string is treated as "not configured" (AllNamespaces mode) + // Skip validation for empty strings - they'll be normalized by GetWatchNamespace() + if str == "" { + return nil + } if str == installNamespace { return fmt.Errorf("invalid value %q: watchNamespace must be different from %q (the install namespace) because this operator uses SingleNamespace install mode to watch a different namespace", str, installNamespace) } diff --git a/internal/operator-controller/config/config_test.go b/internal/operator-controller/config/config_test.go index 95bb98f0b4..4860b4e7f8 100644 --- a/internal/operator-controller/config/config_test.go +++ b/internal/operator-controller/config/config_test.go @@ -275,6 +275,27 @@ func Test_UnmarshalConfig(t *testing.T) { installNamespace: "", expectedWatchNamespace: ptr.To("valid-ns"), }, + { + name: "accepts empty string watchNamespace (treated as AllNamespaces) when AllNamespaces+SingleNamespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": ""}`), + installNamespace: "install-ns", + expectedWatchNamespace: nil, + }, + { + name: "accepts empty string watchNamespace (treated as AllNamespaces) when only SingleNamespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": ""}`), + installNamespace: "install-ns", + expectedWatchNamespace: nil, + }, + { + name: "accepts empty string watchNamespace (treated as AllNamespaces) when only OwnNamespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{"watchNamespace": ""}`), + installNamespace: "install-ns", + expectedWatchNamespace: nil, + }, } { t.Run(tc.name, func(t *testing.T) { var rv1 bundle.RegistryV1 diff --git a/test/e2e/single_namespace_support_test.go b/test/e2e/single_namespace_support_test.go index 2c3b825a1a..1364548db6 100644 --- a/test/e2e/single_namespace_support_test.go +++ b/test/e2e/single_namespace_support_test.go @@ -410,3 +410,142 @@ func TestClusterExtensionVersionUpdate(t *testing.T) { require.Len(ct, cerList.Items, 2) }, pollDuration, pollInterval) } + +func TestClusterExtensionEmptyWatchNamespace(t *testing.T) { + SkipIfFeatureGateDisabled(t, soNsFlag) + t.Log("Test support for empty watchNamespace treated as AllNamespaces mode") + defer utils.CollectTestArtifacts(t, artifactName, c, cfg) + + t.Log("By creating install namespace and necessary rbac resources") + namespace := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-watchnamespace-operator", + }, + } + require.NoError(t, c.Create(t.Context(), &namespace)) + t.Cleanup(func() { + require.NoError(t, c.Delete(context.Background(), &namespace)) + }) + + serviceAccount := corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-watchnamespace-operator-installer", + Namespace: namespace.GetName(), + }, + } + require.NoError(t, c.Create(t.Context(), &serviceAccount)) + t.Cleanup(func() { + require.NoError(t, c.Delete(context.Background(), &serviceAccount)) + }) + + clusterRoleBinding := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-watchnamespace-operator-installer", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: corev1.GroupName, + Name: serviceAccount.GetName(), + Namespace: serviceAccount.GetNamespace(), + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: "cluster-admin", + }, + } + require.NoError(t, c.Create(t.Context(), clusterRoleBinding)) + t.Cleanup(func() { + require.NoError(t, c.Delete(context.Background(), clusterRoleBinding)) + }) + + t.Log("By creating the test-catalog ClusterCatalog") + extensionCatalog := &ocv1.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-catalog", + }, + Spec: ocv1.ClusterCatalogSpec{ + Source: ocv1.CatalogSource{ + Type: ocv1.SourceTypeImage, + Image: &ocv1.ImageSource{ + Ref: fmt.Sprintf("%s/e2e/test-catalog:v1", os.Getenv("CLUSTER_REGISTRY_HOST")), + PollIntervalMinutes: ptr.To(1), + }, + }, + }, + } + require.NoError(t, c.Create(t.Context(), extensionCatalog)) + t.Cleanup(func() { + require.NoError(t, c.Delete(context.Background(), extensionCatalog)) + }) + + t.Log("By waiting for the catalog to serve its metadata") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: extensionCatalog.GetName()}, extensionCatalog)) + cond := apimeta.FindStatusCondition(extensionCatalog.Status.Conditions, ocv1.TypeServing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonAvailable, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By installing the single-namespace-operator ClusterExtension with empty watchNamespace") + clusterExtension := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-watchnamespace-operator-extension", + }, + Spec: ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogFilter{ + PackageName: "single-namespace-operator", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name}, + }, + }, + }, + Namespace: namespace.GetName(), + ServiceAccount: ocv1.ServiceAccountReference{ + Name: serviceAccount.GetName(), + }, + Config: &ocv1.ClusterExtensionConfig{ + ConfigType: ocv1.ClusterExtensionConfigTypeInline, + Inline: &apiextensionsv1.JSON{ + Raw: []byte(`{"watchNamespace": ""}`), + }, + }, + }, + } + require.NoError(t, c.Create(t.Context(), clusterExtension)) + t.Cleanup(func() { + require.NoError(t, c.Delete(context.Background(), clusterExtension)) + }) + + t.Log("By waiting for the extension to be installed successfully (empty watchNamespace treated as AllNamespaces)") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(t.Context(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + require.Contains(ct, cond.Message, "Installed bundle") + require.NotNil(ct, clusterExtension.Status.Install) + require.NotEmpty(ct, clusterExtension.Status.Install.Bundle) + }, pollDuration, pollInterval) + + t.Log("By verifying the deployment does not have watchNamespace annotation (should watch all namespaces)") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + deployment := &appsv1.Deployment{} + require.NoError(ct, c.Get(t.Context(), types.NamespacedName{Namespace: namespace.GetName(), Name: "single-namespace-operator"}, deployment)) + // When watchNamespace is empty/nil, the olm.targetNamespaces annotation should not be set + // or should be empty, indicating the operator watches all namespaces + annotations := deployment.Spec.Template.GetAnnotations() + if annotations != nil { + targetNs, exists := annotations["olm.targetNamespaces"] + if exists { + require.Empty(ct, targetNs, "olm.targetNamespaces should be empty for AllNamespaces mode") + } + } + }, pollDuration, pollInterval) +}