address copilot comments

Signed-off-by: Nader Ziada <nziada@redhat.com>

Author: nader-ziada

internal/operator-controller/applier/boxcutter.go

@@ -102,7 +102,10 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
 	}
 
 	if nsConfig != nil && nsConfig.Managed {
-		nsObj := BuildNamespaceObject(nsConfig.Name, nsConfig.Template)
+		nsObj, err := BuildNamespaceObject(nsConfig.Name, nsConfig.Template)
+		if err != nil {
+			return nil, err
+		}
 		nsObj.SetLabels(mergeStringMaps(nsObj.GetLabels(), objectLabels))
 		objs = append(objs, *ocv1ac.ClusterObjectSetObject().WithObject(nsObj))
 	}
@@ -202,7 +205,10 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
 	}
 
 	if nsConfig != nil && nsConfig.Managed {
-		nsObj := BuildNamespaceObject(nsConfig.Name, nsConfig.Template)
+		nsObj, err := BuildNamespaceObject(nsConfig.Name, nsConfig.Template)
+		if err != nil {
+			return nil, err
+		}
 		nsObj.SetLabels(mergeStringMaps(nsObj.GetLabels(), objectLabels))
 		objs = append(objs, *ocv1ac.ClusterObjectSetObject().WithObject(nsObj))
 	}

internal/operator-controller/applier/namespace.go

@@ -62,7 +62,7 @@ func ResolveNamespaceName(csvAnnotations map[string]string, packageName string)
 	return fmt.Sprintf("%s-system", packageName), template, nil
 }
 
-func BuildNamespaceObject(name string, template *corev1.Namespace) unstructured.Unstructured {
+func BuildNamespaceObject(name string, template *corev1.Namespace) (unstructured.Unstructured, error) {
 	ns := corev1.Namespace{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "v1",
@@ -84,8 +84,8 @@ func BuildNamespaceObject(name string, template *corev1.Namespace) unstructured.
 
 	unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&ns)
 	if err != nil {
-		panic(fmt.Sprintf("failed to convert namespace to unstructured: %v", err))
+		return unstructured.Unstructured{}, fmt.Errorf("failed to convert namespace to unstructured: %w", err)
 	}
 
-	return unstructured.Unstructured{Object: unstructuredObj}
+	return unstructured.Unstructured{Object: unstructuredObj}, nil
 }

internal/operator-controller/applier/namespace_test.go

@@ -234,7 +234,8 @@ func TestBuildNamespaceObject(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := BuildNamespaceObject(tt.nsName, tt.template)
+			result, err := BuildNamespaceObject(tt.nsName, tt.template)
+			require.NoError(t, err)
 			tt.validate(t, result.Object)
 		})
 	}

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

@@ -414,9 +414,9 @@ func ResolveNamespace(nsClient corev1client.NamespacesGetter) ReconcileStepFunc
 			l.V(1).Info("validating user-provided namespace exists", "namespace", ext.Spec.Namespace)
 			_, err := nsClient.Namespaces().Get(ctx, ext.Spec.Namespace, metav1.GetOptions{})
 			if apierrors.IsNotFound(err) {
-				err := fmt.Errorf("namespace %q not found; spec.namespace must reference an existing namespace", ext.Spec.Namespace)
-				setStatusProgressing(ext, reconcile.TerminalError(err))
-				return nil, err
+				termErr := reconcile.TerminalError(fmt.Errorf("namespace %q not found; spec.namespace must reference an existing namespace", ext.Spec.Namespace))
+				setStatusProgressing(ext, termErr)
+				return nil, termErr
 			}
 			if err != nil {
 				return nil, fmt.Errorf("error checking namespace %q: %w", ext.Spec.Namespace, err)
@@ -426,8 +426,14 @@ func ResolveNamespace(nsClient corev1client.NamespacesGetter) ReconcileStepFunc
 			return nil, nil
 		}
 
-		// Managed mode: resolve name from bundle annotations
+		// Managed mode: resolve name from bundle annotations.
+		// If imageFS is nil (fallback path), preserve the previously resolved namespace
+		// from status to avoid wiping it in later steps.
 		if state.imageFS == nil {
+			if ext.Status.Namespace != "" {
+				state.resolvedNamespace = ext.Status.Namespace
+				state.namespaceManaged = true
+			}
 			return nil, nil
 		}
 
@@ -439,15 +445,15 @@ func ResolveNamespace(nsClient corev1client.NamespacesGetter) ReconcileStepFunc
 		packageName := getPackageName(ext)
 		resolvedName, template, err := applier.ResolveNamespaceName(bundleAnnotations, packageName)
 		if err != nil {
-			err := fmt.Errorf("error resolving namespace from bundle annotations: %w", err)
-			setStatusProgressing(ext, reconcile.TerminalError(err))
-			return nil, err
+			termErr := reconcile.TerminalError(fmt.Errorf("error resolving namespace from bundle annotations: %w", err))
+			setStatusProgressing(ext, termErr)
+			return nil, termErr
 		}
 
 		if ext.Status.Namespace != "" && ext.Status.Namespace != resolvedName {
-			err := fmt.Errorf("bundle upgrade changes managed namespace from %q to %q; this is not supported", ext.Status.Namespace, resolvedName)
-			setStatusProgressing(ext, reconcile.TerminalError(err))
-			return nil, err
+			termErr := reconcile.TerminalError(fmt.Errorf("bundle upgrade changes managed namespace from %q to %q; this is not supported", ext.Status.Namespace, resolvedName))
+			setStatusProgressing(ext, termErr)
+			return nil, termErr
 		}
 
 		// On first install, verify managed namespace does not already exist.
@@ -456,9 +462,9 @@ func ResolveNamespace(nsClient corev1client.NamespacesGetter) ReconcileStepFunc
 			l.V(1).Info("checking managed namespace does not already exist", "namespace", resolvedName)
 			_, getErr := nsClient.Namespaces().Get(ctx, resolvedName, metav1.GetOptions{})
 			if getErr == nil {
-				err := fmt.Errorf("managed namespace %q already exists; use spec.namespace to install into an existing namespace", resolvedName)
-				setStatusProgressing(ext, reconcile.TerminalError(err))
-				return nil, err
+				termErr := reconcile.TerminalError(fmt.Errorf("managed namespace %q already exists; use spec.namespace to install into an existing namespace", resolvedName))
+				setStatusProgressing(ext, termErr)
+				return nil, termErr
 			}
 			if !apierrors.IsNotFound(getErr) {
 				return nil, fmt.Errorf("error checking namespace %q: %w", resolvedName, getErr)

internal/operator-controller/controllers/clusterobjectset_controller.go

@@ -600,7 +600,10 @@ func collisionMessage(ores machinery.ObjectResult) string {
 	if gvk.Kind == "Namespace" {
 		return fmt.Sprintf("namespace %q is already managed by another controller", name)
 	}
-	return fmt.Sprintf("%s.%s %s/%s collision: %s", gvk.Kind, gvk.GroupVersion(), obj.GetNamespace(), name, ores.String())
+	if ns := obj.GetNamespace(); ns != "" {
+		return fmt.Sprintf("%s.%s %s/%s collision: %s", gvk.Kind, gvk.GroupVersion(), ns, name, ores.String())
+	}
+	return fmt.Sprintf("%s.%s %s collision: %s", gvk.Kind, gvk.GroupVersion(), name, ores.String())
 }
 
 // EffectiveCollisionProtection resolves the collision protection value using