diff --git a/cmd/main.go b/cmd/main.go index 022d54d1..e3745d6d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -32,7 +32,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apimachinery/pkg/util/validation" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" @@ -275,16 +274,16 @@ func configureWebhookServingCerts(cfg *rest.Config, cli client.Client, enableWeb var webhookTLSConfigure = webhooktls.Configure func resolveApplicationsNamespace() string { - provided := strings.TrimSpace(os.Getenv("APPLICATIONS_NAMESPACE")) - if provided != "" && len(validation.IsDNS1123Label(provided)) == 0 { - return provided + provided := os.Getenv("APPLICATIONS_NAMESPACE") + if valid := platform.ValidApplicationsNamespace(provided); valid != "" { + return valid } // Leave empty so the reconciler can fall back from Workbenches.spec.platform // (opendatahub vs redhat-ods-applications). Product installs always inject the env. // setupLog is intentional here: this runs during process startup before a // reconcile context exists. - if provided == "" { + if strings.TrimSpace(provided) == "" { setupLog.Info("APPLICATIONS_NAMESPACE not set; reconciler will use platform default") } else { setupLog.Info("APPLICATIONS_NAMESPACE invalid; reconciler will use platform default", diff --git a/internal/controller/platform_config_predicate_test.go b/internal/controller/platform_config_predicate_test.go index 1c9644ae..080fa0aa 100644 --- a/internal/controller/platform_config_predicate_test.go +++ b/internal/controller/platform_config_predicate_test.go @@ -40,6 +40,21 @@ func TestPlatformConfigWatchNamespaces(t *testing.T) { } }) + t.Run("invalid configured namespace falls back to both platform defaults", func(t *testing.T) { + t.Parallel() + + r := &WorkbenchesReconciler{ApplicationsNamespace: "bad/name"} + got := r.platformConfigWatchNamespaces() + if len(got) != 2 { + t.Fatalf("platformConfigWatchNamespaces() len = %d, want 2", len(got)) + } + if got[0] != platform.DefaultApplicationsNamespaceODH || + got[1] != platform.DefaultApplicationsNamespaceRHOAI { + t.Fatalf("platformConfigWatchNamespaces() = %#v, want [%s %s]", + got, platform.DefaultApplicationsNamespaceODH, platform.DefaultApplicationsNamespaceRHOAI) + } + }) + t.Run("unset falls back to both platform defaults", func(t *testing.T) { t.Parallel() diff --git a/internal/controller/workbenches_controller.go b/internal/controller/workbenches_controller.go index 9875610c..68b3f2bb 100644 --- a/internal/controller/workbenches_controller.go +++ b/internal/controller/workbenches_controller.go @@ -34,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -197,8 +196,8 @@ func (r *WorkbenchesReconciler) SetupWithManager(mgr ctrl.Manager) error { // only that namespace; otherwise watch both platform defaults until CR platform // selects one. func (r *WorkbenchesReconciler) platformConfigWatchNamespaces() []string { - if r.ApplicationsNamespace != "" { - return []string{r.ApplicationsNamespace} + if ns := r.configuredApplicationsNamespace(); ns != "" { + return []string{ns} } return []string{ @@ -207,6 +206,13 @@ func (r *WorkbenchesReconciler) platformConfigWatchNamespaces() []string { } } +// configuredApplicationsNamespace returns ApplicationsNamespace when it is a valid +// DNS-1123 label. Invalid values are ignored so reconcile and ConfigMap watches +// fall back to platform defaults consistently. +func (r *WorkbenchesReconciler) configuredApplicationsNamespace() string { + return platform.ValidApplicationsNamespace(r.ApplicationsNamespace) +} + func shouldWatchImageStreams(mapper meta.RESTMapper) (bool, error) { _, err := mapper.RESTMapping(gvk.ImageStream.GroupKind(), gvk.ImageStream.Version) if err == nil { @@ -670,13 +676,13 @@ func (r *WorkbenchesReconciler) setReadyCondition( func (r *WorkbenchesReconciler) configureDependencies(ctx context.Context, wb *componentsv1alpha1.Workbenches) error { appsNS := r.resolveOperandNamespace(wb.Spec.Platform) - if err := r.ensureGeneratedNamespace(ctx, wb, appsNS, "applications"); err != nil { + if err := r.ensureGeneratedNamespace(ctx, appsNS, "applications"); err != nil { return fmt.Errorf("applications namespace: %w", err) } legacyNS := r.resolveLegacyWorkbenchNamespace(wb) if legacyNS != appsNS { - if err := r.ensureGeneratedNamespace(ctx, wb, legacyNS, "legacy workbench"); err != nil { + if err := r.ensureGeneratedNamespace(ctx, legacyNS, "legacy workbench"); err != nil { return fmt.Errorf("legacy workbench namespace: %w", err) } } @@ -686,7 +692,6 @@ func (r *WorkbenchesReconciler) configureDependencies(ctx context.Context, wb *c func (r *WorkbenchesReconciler) ensureGeneratedNamespace( ctx context.Context, - _ *componentsv1alpha1.Workbenches, nsName, purpose string, ) error { l := log.FromContext(ctx) @@ -757,8 +762,8 @@ func (r *WorkbenchesReconciler) setStatusNamespaces(wb *componentsv1alpha1.Workb // fall back by platform: opendatahub (ODH/default) or redhat-ods-applications // (SelfManagedRhoai). Spec.WorkbenchNamespace is not used for operand deploy. func (r *WorkbenchesReconciler) resolveOperandNamespace(platformType string) string { - if r.ApplicationsNamespace != "" && len(validation.IsDNS1123Label(r.ApplicationsNamespace)) == 0 { - return r.ApplicationsNamespace + if ns := r.configuredApplicationsNamespace(); ns != "" { + return ns } return platform.DefaultApplicationsNamespace(platformType) diff --git a/internal/controller/workbenches_controller_test.go b/internal/controller/workbenches_controller_test.go index 90042a32..aad161ef 100644 --- a/internal/controller/workbenches_controller_test.go +++ b/internal/controller/workbenches_controller_test.go @@ -443,6 +443,8 @@ var _ = Describe("Workbenches Controller", func() { DeferCleanup(func() { cleanupWorkbenches(wb) cleanupNamespace("legacy-invalid-apps-ns") + // fallbackNS is the shared suite applications namespace — do not delete it. + removeOwnedNamespaceLabel(fallbackNS) }) _, err := reconcileWorkbenches(invalidReconciler, wb) @@ -1079,6 +1081,31 @@ func cleanupNamespace(name string) { ExpectWithOffset(1, k8sClient.Delete(ctx, ns)).To(Succeed()) } +func removeOwnedNamespaceLabel(name string) { + ns := &corev1.Namespace{} + + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, ns) + if client.IgnoreNotFound(err) != nil { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + return + } + + if err != nil { + return + } + + if ns.Labels == nil { + return + } + + delete(ns.Labels, metadata.OwnedNamespaceLabel) + if len(ns.Labels) == 0 { + ns.Labels = nil + } + + ExpectWithOffset(1, k8sClient.Update(ctx, ns)).To(Succeed()) +} + func cleanupDeployments(namespace string) { deployments := &appsv1.DeploymentList{} diff --git a/internal/platform/platform.go b/internal/platform/platform.go index b97fb8ba..f6b70152 100644 --- a/internal/platform/platform.go +++ b/internal/platform/platform.go @@ -17,6 +17,12 @@ limitations under the License. // Package platform provides platform type constants and helpers. package platform +import ( + "strings" + + "k8s.io/apimachinery/pkg/util/validation" +) + // Platform type constants matching the orchestrator's platform identity values. const ( OpenDataHub = "OpenDataHub" @@ -61,6 +67,17 @@ func SectionTitle(platformType string) string { return titles[OpenDataHub] } +// ValidApplicationsNamespace returns name when it is a non-empty DNS-1123 label. +// Invalid or empty values return "" so callers can fall back to platform defaults. +func ValidApplicationsNamespace(name string) string { + name = strings.TrimSpace(name) + if name != "" && len(validation.IsDNS1123Label(name)) == 0 { + return name + } + + return "" +} + // DefaultApplicationsNamespace returns the fallback applications namespace // when APPLICATIONS_NAMESPACE is unset: opendatahub for ODH (and unknown), // redhat-ods-applications for SelfManagedRhoai. diff --git a/internal/platform/suite_test.go b/internal/platform/suite_test.go index d9b36de6..300981d8 100644 --- a/internal/platform/suite_test.go +++ b/internal/platform/suite_test.go @@ -41,3 +41,18 @@ var _ = Describe("DefaultLegacyWorkbenchNamespace", func() { Entry("empty defaults to ODH", "", platform.LegacyWorkbenchNamespaceODH), ) }) + +var _ = Describe("ValidApplicationsNamespace", func() { + DescribeTable("accepts DNS-1123 labels only", + func(provided, want string) { + Expect(platform.ValidApplicationsNamespace(provided)).To(Equal(want)) + }, + Entry("empty", "", ""), + Entry("whitespace", " ", ""), + Entry("invalid slash", "bad/name", ""), + Entry("invalid underscore", "Invalid_Namespace", ""), + Entry("valid ODH default", "opendatahub", "opendatahub"), + Entry("valid RHOAI default", "redhat-ods-applications", "redhat-ods-applications"), + Entry("trimmed valid", " custom-apps ", "custom-apps"), + ) +})