feat(application): Webapp feature with production dns #167

						
				@ -58,2 +59,4 @@

				    }

				     // Returns true if any deployment in the given namespace matching the label selector

				    // has status.availableReplicas > 0 (or condition Available=True).

letian commented

2025-10-20 18:39:56 +00:00

Should it be doc comments? Same question for the other functions below

letian reviewed 2025-10-20 18:43:12 +00:00

harmony/src/domain/topology/k8s.rs

						
				@ -60,0 +135,4 @@

				    }

				    // Get the application-controller ServiceAccount name (fallback to default)

				    pub async fn get_argocd_controller_sa_name(&self, ns: &str) -> Result<String, Error> {

letian commented

2025-10-20 18:43:12 +00:00

Does it have to be that specific? Or would it be ok if the signature was Result<Option<String>, Error> and that users of this function can decide what to do without values?

For example:

client.get_controller_service_account_name("namespace").unwrap_or("argocd-application-controller".to_string())

Does it have to be that specific? Or would it be ok if the signature was `Result<Option<String>, Error>` and that users of this function can decide what to do without values? For example: ```rs client.get_controller_service_account_name("namespace").unwrap_or("argocd-application-controller".to_string()) ```

letian reviewed 2025-10-20 18:50:15 +00:00

harmony/src/domain/topology/k8s.rs

						
				@ -60,0 +172,4 @@

				    // 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> {

letian commented

2025-10-20 18:50:15 +00:00

In addition to the comment above, I think it wouldn't be too tricky to refactor (if needed in another PR) with something like:

let service_account = k8s.get_controller_service_account("namespace").await?.unwrap_or("argocd-application-controller".to_string());
let cluster_wide = k8s.is_service_account_cluster_wide(service_account, "namespace").await?;

In addition to the comment above, I think it wouldn't be too tricky to refactor (if needed in another PR) with something like: ```rs let service_account = k8s.get_controller_service_account("namespace").await?.unwrap_or("argocd-application-controller".to_string()); let cluster_wide = k8s.is_service_account_cluster_wide(service_account, "namespace").await?; ```

letian reviewed 2025-10-20 18:52:21 +00:00

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

						
				@ -22,3 +22,3 @@

				    pub version: Option<String>,

				    pub kube_version: Option<String>,

				    pub api_versions: Vec<String>,

				    // pub api_versions: Vec<String>,

letian commented

2025-10-20 18:52:21 +00:00

still needed?

letian reviewed 2025-10-20 18:55:45 +00:00

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

						
				@ -194,7 +198,7 @@ impl<

				                    openshift: true,

				                    argo_apps: vec![ArgoApplication::from(CDApplicationConfig {

				                        // helm pull oci://hub.nationtech.io/harmony/harmony-example-rust-webapp-chart --version 0.1.0

letian commented

2025-10-20 18:55:45 +00:00

outdated comment -> to be removed

letian reviewed 2025-10-20 18:59:04 +00:00

harmony/src/modules/argocd/mod.rs

						
				@ -0,0 +26,4 @@

				    InstalledNamespaceScoped(String),

				}

				pub async fn discover_argo_all(

letian commented

2025-10-20 18:59:04 +00:00

a quick cleanup/refactoring for this function could make things easier to read/understand, for example by breaking down the different steps into different (module private) functions

letian commented

2025-10-20 19:10:28 +00:00

Left a few comments here and there that could be small improvements for a future PR

Left a few comments here and there that could be small improvements for a future PR <img src="https://i.giphy.com/NEvPzZ8bd1V4Y.webp"/>

wjro changed target branch from feat/detect_k8s_flavour to master

2025-10-21 15:56:49 +00:00

Run Check Script / check (pull_request) Failing after 16s

Details

This pull request has changes conflicting with the target branch.

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

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/webappdns:feat/webappdns

git checkout feat/webappdns

Sign in to join this conversation.

No reviewers

No Label

Architecture Decision Record

Good first issue

RFC

No Milestone

No project

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: NationTech/harmony#167