From eeaaa26d0e0c9cdf964df5afc9afa17068390261 Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Tue, 6 Jan 2026 15:03:55 -0500 Subject: [PATCH] chore: Fix pr comments, documentation, slight refactor for better apis --- harmony/src/domain/topology/k8s.rs | 41 ++++++++++--------- .../features/packaging_deployment.rs | 1 - harmony/src/modules/argocd/mod.rs | 7 +++- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/harmony/src/domain/topology/k8s.rs b/harmony/src/domain/topology/k8s.rs index 2cb93c9..9c8951d 100644 --- a/harmony/src/domain/topology/k8s.rs +++ b/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 - // has status.availableReplicas > 0 (or condition Available=True). + /// Returns true if any deployment in the given namespace matching the label selector + /// 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| { - c.type_ == "Available" - && c.status == "True" - }) { + if conds + .iter() + .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 - // matching the label selector (equivalent to kubectl -A -l ...). + /// Cluster-wide: returns namespaces that have at least one healthy deployment + /// 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| { - c.type_ == "Available" - && c.status == "True" - }) + conds + .iter() + .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) - pub async fn get_argocd_controller_sa_name(&self, ns: &str) -> Result { + /// Get the application-controller ServiceAccount name (fallback to default) + pub async fn get_controller_service_account_name( + &self, + ns: &str, + ) -> Result, Error> { let api: Api = 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 { - let sa = self.get_argocd_controller_sa_name(ns).await?; + pub async fn is_service_account_cluster_wide(&self, sa: &str, ns: &str) -> Result { let crbs = self.list_clusterrolebindings_json().await?; let sa_user = format!("system:serviceaccount:{}:{}", ns, sa); for crb in crbs { diff --git a/harmony/src/modules/application/features/packaging_deployment.rs b/harmony/src/modules/application/features/packaging_deployment.rs index ed46090..be239ae 100644 --- a/harmony/src/modules/application/features/packaging_deployment.rs +++ b/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()), diff --git a/harmony/src/modules/argocd/mod.rs b/harmony/src/modules/argocd/mod.rs index 1d59d0a..402c90f 100644 --- a/harmony/src/modules/argocd/mod.rs +++ b/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())