Skip to content

Commit ffe7458

Browse files
committed
address copilot comments
Signed-off-by: Nader Ziada <nziada@redhat.com>
1 parent 14b722f commit ffe7458

5 files changed

Lines changed: 24 additions & 8 deletions

File tree

internal/operator-controller/applier/boxcutter.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
102102
}
103103

104104
if nsConfig != nil && nsConfig.Managed {
105-
nsObj := BuildNamespaceObject(nsConfig.Name, nsConfig.Template)
105+
nsObj, err := BuildNamespaceObject(nsConfig.Name, nsConfig.Template)
106+
if err != nil {
107+
return nil, err
108+
}
106109
nsObj.SetLabels(mergeStringMaps(nsObj.GetLabels(), objectLabels))
107110
objs = append(objs, *ocv1ac.ClusterObjectSetObject().WithObject(nsObj))
108111
}
@@ -202,7 +205,10 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
202205
}
203206

204207
if nsConfig != nil && nsConfig.Managed {
205-
nsObj := BuildNamespaceObject(nsConfig.Name, nsConfig.Template)
208+
nsObj, err := BuildNamespaceObject(nsConfig.Name, nsConfig.Template)
209+
if err != nil {
210+
return nil, err
211+
}
206212
nsObj.SetLabels(mergeStringMaps(nsObj.GetLabels(), objectLabels))
207213
objs = append(objs, *ocv1ac.ClusterObjectSetObject().WithObject(nsObj))
208214
}

internal/operator-controller/applier/namespace.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func ResolveNamespaceName(csvAnnotations map[string]string, packageName string)
6262
return fmt.Sprintf("%s-system", packageName), template, nil
6363
}
6464

65-
func BuildNamespaceObject(name string, template *corev1.Namespace) unstructured.Unstructured {
65+
func BuildNamespaceObject(name string, template *corev1.Namespace) (unstructured.Unstructured, error) {
6666
ns := corev1.Namespace{
6767
TypeMeta: metav1.TypeMeta{
6868
APIVersion: "v1",
@@ -84,8 +84,8 @@ func BuildNamespaceObject(name string, template *corev1.Namespace) unstructured.
8484

8585
unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&ns)
8686
if err != nil {
87-
panic(fmt.Sprintf("failed to convert namespace to unstructured: %v", err))
87+
return unstructured.Unstructured{}, fmt.Errorf("failed to convert namespace to unstructured: %w", err)
8888
}
8989

90-
return unstructured.Unstructured{Object: unstructuredObj}
90+
return unstructured.Unstructured{Object: unstructuredObj}, nil
9191
}

internal/operator-controller/applier/namespace_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,8 @@ func TestBuildNamespaceObject(t *testing.T) {
234234

235235
for _, tt := range tests {
236236
t.Run(tt.name, func(t *testing.T) {
237-
result := BuildNamespaceObject(tt.nsName, tt.template)
237+
result, err := BuildNamespaceObject(tt.nsName, tt.template)
238+
require.NoError(t, err)
238239
tt.validate(t, result.Object)
239240
})
240241
}

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,8 +426,14 @@ func ResolveNamespace(nsClient corev1client.NamespacesGetter) ReconcileStepFunc
426426
return nil, nil
427427
}
428428

429-
// Managed mode: resolve name from bundle annotations
429+
// Managed mode: resolve name from bundle annotations.
430+
// If imageFS is nil (fallback path), preserve the previously resolved namespace
431+
// from status to avoid wiping it in later steps.
430432
if state.imageFS == nil {
433+
if ext.Status.Namespace != "" {
434+
state.resolvedNamespace = ext.Status.Namespace
435+
state.namespaceManaged = true
436+
}
431437
return nil, nil
432438
}
433439

internal/operator-controller/controllers/clusterobjectset_controller.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,10 @@ func collisionMessage(ores machinery.ObjectResult) string {
600600
if gvk.Kind == "Namespace" {
601601
return fmt.Sprintf("namespace %q is already managed by another controller", name)
602602
}
603-
return fmt.Sprintf("%s.%s %s/%s collision: %s", gvk.Kind, gvk.GroupVersion(), obj.GetNamespace(), name, ores.String())
603+
if ns := obj.GetNamespace(); ns != "" {
604+
return fmt.Sprintf("%s.%s %s/%s collision: %s", gvk.Kind, gvk.GroupVersion(), ns, name, ores.String())
605+
}
606+
return fmt.Sprintf("%s.%s %s collision: %s", gvk.Kind, gvk.GroupVersion(), name, ores.String())
604607
}
605608

606609
// EffectiveCollisionProtection resolves the collision protection value using

0 commit comments

Comments
 (0)