chore: Fix pr comments, documentation, slight refactor for better apis

harmony/src/domain/topology/k8s.rs

@@ -22,7 +22,7 @@ use kube::{
 };
 use log::{debug, error, info, trace};
 use serde::{Serialize, de::DeserializeOwned};
-use serde_json::{json, Value};
+use serde_json::{Value, json};
 use similar::TextDiff;
 use tokio::{io::AsyncReadExt, time::sleep};
 
@@ -58,8 +58,8 @@ impl K8sClient {
         })
     }
 
-     // Returns true if any deployment in the given namespace matching the label selector
+    /// Returns true if any deployment in the given namespace matching the label selector
-    // has status.availableReplicas > 0 (or condition Available=True).
+    /// has status.availableReplicas > 0 (or condition Available=True).
     pub async fn has_healthy_deployment_with_label(
         &self,
         namespace: &str,
@@ -80,10 +80,10 @@ impl K8sClient {
             }
             // Fallback: scan conditions
             if let Some(conds) = d.status.as_ref().and_then(|s| s.conditions.as_ref()) {
-                if conds.iter().any(|c| {
+                if conds
-                    c.type_ == "Available"
+                    .iter()
-                        && c.status == "True"
+                    .any(|c| c.type_ == "Available" && c.status == "True")
-                }) {
+                {
                     return Ok(true);
                 }
             }
@@ -91,8 +91,8 @@ impl K8sClient {
         Ok(false)
     }
 
-    // Cluster-wide: returns namespaces that have at least one healthy deployment
+    /// Cluster-wide: returns namespaces that have at least one healthy deployment
-    // matching the label selector (equivalent to kubectl -A -l ...).
+    /// matching the label selector (equivalent to kubectl -A -l ...).
     pub async fn list_namespaces_with_healthy_deployments(
         &self,
         label_selector: &str,
@@ -119,10 +119,9 @@ impl K8sClient {
                     .as_ref()
                     .and_then(|s| s.conditions.as_ref())
                     .map(|conds| {
-                        conds.iter().any(|c| {
+                        conds
-                            c.type_ == "Available"
+                            .iter()
-                                && c.status == "True"
+                            .any(|c| c.type_ == "Available" && c.status == "True")
-                        })
                     })
                     .unwrap_or(false)
             };
@@ -134,8 +133,11 @@ impl K8sClient {
         Ok(healthy_ns.into_keys().collect())
     }
 
-    // Get the application-controller ServiceAccount name (fallback to default)
+    /// Get the application-controller ServiceAccount name (fallback to default)
-    pub async fn get_argocd_controller_sa_name(&self, ns: &str) -> Result<String, Error> {
+    pub async fn get_controller_service_account_name(
+        &self,
+        ns: &str,
+    ) -> Result<Option<String>, Error> {
         let api: Api<Deployment> = Api::namespaced(self.client.clone(), ns);
         let lp = ListParams::default().labels("app.kubernetes.io/component=controller");
         let list = api.list(&lp).await?;
@@ -146,10 +148,10 @@ impl K8sClient {
                 .and_then(|ds| ds.template.spec.as_ref())
                 .and_then(|ps| ps.service_account_name.clone())
             {
-                return Ok(sa);
+                return Ok(Some(sa));
             }
         }
-        Ok("argocd-application-controller".to_string())
+        Ok(None)
     }
 
     // List ClusterRoleBindings dynamically and return as JSON values
@@ -170,10 +172,9 @@ impl K8sClient {
         Ok(out)
     }
 
-    // Determine if Argo controller in ns has cluster-wide permissions via CRBs
+    /// Determine if Argo controller in ns has cluster-wide permissions via CRBs
     // TODO This does not belong in the generic k8s client, should be refactored at some point
-    pub async fn is_argocd_cluster_wide(&self, ns: &str) -> Result<bool, Error> {
+    pub async fn is_service_account_cluster_wide(&self, sa: &str, ns: &str) -> Result<bool, Error> {
-        let sa = self.get_argocd_controller_sa_name(ns).await?;
         let crbs = self.list_clusterrolebindings_json().await?;
         let sa_user = format!("system:serviceaccount:{}:{}", ns, sa);
         for crb in crbs {

harmony/src/modules/application/features/packaging_deployment.rs

@@ -197,7 +197,6 @@ impl<
                     namespace: format!("{}", self.application.name()),
                     openshift: true,
                     argo_apps: vec![ArgoApplication::from(CDApplicationConfig {
-                        // helm pull oci://hub.nationtech.io/harmony/harmony-example-rust-webapp-chart --version 0.1.0
                         version: Version::from("0.2.1").unwrap(),
                         helm_chart_repo_url: "hub.nationtech.io/harmony".to_string(),
                         helm_chart_name: format!("{}-chart", self.application.name()),

harmony/src/modules/argocd/mod.rs

@@ -116,7 +116,12 @@ pub async fn discover_argo_all(
         }
 
         trace!("Determining Argo CD scope for namespace '{ns}' (cluster-wide vs namespace-scoped)");
-        let scope = match k8s.is_argocd_cluster_wide(&ns).await {
+
+        let sa = k8s
+            .get_controller_service_account_name(&ns)
+            .await?
+            .unwrap_or("argocd-application-controller".to_string());
+        let scope = match k8s.is_service_account_cluster_wide(&sa, &ns).await {
             Ok(true) => {
                 debug!("Namespace '{ns}' identified as cluster-wide Argo CD control plane");
                 ArgoScope::ClusterWide(ns.to_string())