Merge pull request #727 from factorial-io/claude/extract-auto-domain-helper-d02vc

Refactor domain resolution logic into ServicePortMapping method

Author: stmh

config/casbin/policy.yaml

@@ -1,17 +1,25 @@
 scopes:
-  client-a:
-    description: Client A
-    created_at: 2024-01-01T00:00:00Z
   client-b:
     description: Client B
     created_at: 2024-01-01T00:00:00Z
-  qa:
-    description: QA
+  client-a:
+    description: Client A
     created_at: 2024-01-01T00:00:00Z
   default:
     description: Default scope for unassigned apps
     created_at: 2024-01-01T00:00:00Z
+  qa:
+    description: QA
+    created_at: 2024-01-01T00:00:00Z
 roles:
+  viewer:
+    permissions:
+    - view
+    description: Read-only access
+  admin:
+    permissions:
+    - '*'
+    description: Full system access
   operator:
     permissions:
     - view
@@ -26,54 +34,46 @@ roles:
     - logs
     - create
     description: Developer access - all except destroy
-  viewer:
-    permissions:
-    - view
-    description: Read-only access
-  admin:
-    permissions:
-    - '*'
-    description: Full system access
 assignments:
-  identifier:hello-world:
+  dev:system:internal:
+  - role: admin
+    scopes:
+    - '*'
+  identifier:client-a:
   - role: developer
     scopes:
     - client-a
+  identifier:test-service:
+  - role: admin
+    scopes:
+    - client-a
     - client-b
     - qa
+    - default
   identifier:admin:
   - role: admin
     scopes:
     - client-a
     - client-b
     - qa
     - default
-  '*':
-  - role: viewer
-    scopes:
-    - default
-  identifier:client-a:
+  '@factorial.io':
   - role: developer
     scopes:
     - client-a
-  dev:system:internal:
-  - role: admin
-    scopes:
-    - '*'
-  identifier:test-service:
-  - role: admin
+    - client-b
+    - qa
+  identifier:hello-world:
+  - role: developer
     scopes:
     - client-a
     - client-b
     - qa
-    - default
   xxxstephan@factorial.io:
   - role: admin
     scopes:
     - '*'
-  '@factorial.io':
-  - role: developer
+  '*':
+  - role: viewer
     scopes:
-    - client-a
-    - client-b
-    - qa
+    - default

scotty-core/src/apps/app_data/service.rs

@@ -8,6 +8,22 @@ pub struct ServicePortMapping {
     pub domains: Vec<String>,
 }
 
+impl ServicePortMapping {
+    /// Returns the effective domains for this service.
+    ///
+    /// If custom domains are configured, returns those.
+    /// Otherwise, returns the auto-generated domain: `{service}.{app_domain}`.
+    pub fn get_domains(&self, app_domain: &str) -> Vec<String> {
+        if !self.domains.is_empty() {
+            self.domains.clone()
+        } else if !app_domain.is_empty() {
+            vec![format!("{}.{}", self.service, app_domain)]
+        } else {
+            vec![]
+        }
+    }
+}
+
 #[derive(Deserialize)]
 #[serde(untagged)]
 enum DomainField {
@@ -90,4 +106,41 @@ mod tests {
         assert_eq!(mapping.port, 3000);
         assert_eq!(mapping.domains, vec!["api1.com", "api2.com"]);
     }
+
+    #[test]
+    fn test_get_domains_with_custom_domains() {
+        let mapping = ServicePortMapping {
+            service: "web".to_string(),
+            port: 8080,
+            domains: vec!["custom.example.com".to_string()],
+        };
+        assert_eq!(
+            mapping.get_domains("myapp.apps.example.com"),
+            vec!["custom.example.com"]
+        );
+    }
+
+    #[test]
+    fn test_get_domains_auto_generated() {
+        let mapping = ServicePortMapping {
+            service: "web".to_string(),
+            port: 8080,
+            domains: vec![],
+        };
+        assert_eq!(
+            mapping.get_domains("myapp.apps.example.com"),
+            vec!["web.myapp.apps.example.com"]
+        );
+    }
+
+    #[test]
+    fn test_get_domains_empty_app_domain() {
+        let mapping = ServicePortMapping {
+            service: "web".to_string(),
+            port: 8080,
+            domains: vec![],
+        };
+        let result: Vec<String> = vec![];
+        assert_eq!(mapping.get_domains(""), result);
+    }
 }

scotty-core/src/apps/shared_app_list.rs

@@ -110,26 +110,16 @@ impl SharedAppList {
     pub async fn find_app_by_domain(&self, domain: &str) -> Option<AppData> {
         let apps = self.apps.read().await;
         for app in apps.values() {
-            // Check settings-based domains
+            // Check settings-based domains (custom and auto-generated)
             if let Some(settings) = &app.settings {
                 for service in &settings.public_services {
-                    // Check explicit custom domains
                     if service
-                        .domains
+                        .get_domains(&settings.domain)
                         .iter()
                         .any(|d| d.eq_ignore_ascii_case(domain))
                     {
                         return Some(app.clone());
                     }
-                    // Check auto-generated domain: {service_name}.{settings.domain}
-                    // Must match the Traefik label format in
-                    // scotty/src/docker/loadbalancer/traefik.rs
-                    if !settings.domain.is_empty() {
-                        let auto_domain = format!("{}.{}", service.service, settings.domain);
-                        if auto_domain.eq_ignore_ascii_case(domain) {
-                            return Some(app.clone());
-                        }
-                    }
                 }
             }
 
@@ -268,7 +258,7 @@ mod tests {
     #[tokio::test]
     async fn test_find_app_by_domain_case_insensitive() {
         let list = SharedAppList::new();
-        let app = make_app_with_settings(
+        let app_custom = make_app_with_settings(
             "myapp",
             "myapp.example.com",
             vec![ServicePortMapping {
@@ -277,16 +267,26 @@ mod tests {
                 domains: vec!["Custom.Example.COM".to_string()],
             }],
         );
-        list.add_app(app).await.unwrap();
+        let app_auto = make_app_with_settings(
+            "otherapp",
+            "otherapp.example.com",
+            vec![ServicePortMapping {
+                service: "api".to_string(),
+                port: 3000,
+                domains: vec![],
+            }],
+        );
+        list.add_app(app_custom).await.unwrap();
+        list.add_app(app_auto).await.unwrap();
 
-        // Lowercase lookup should match uppercase stored domain
+        // Lowercase lookup should match uppercase stored custom domain
         let found = list.find_app_by_domain("custom.example.com").await;
         assert!(found.is_some());
         assert_eq!(found.unwrap().name, "myapp");
 
         // Uppercase lookup should match auto-generated domain
-        let found = list.find_app_by_domain("WEB.MYAPP.EXAMPLE.COM").await;
+        let found = list.find_app_by_domain("API.OTHERAPP.EXAMPLE.COM").await;
         assert!(found.is_some());
-        assert_eq!(found.unwrap().name, "myapp");
+        assert_eq!(found.unwrap().name, "otherapp");
     }
 }

scotty/src/docker/loadbalancer/haproxy.rs

@@ -103,10 +103,7 @@ impl LoadBalancerImpl for HaproxyLoadBalancer {
             let environment = service_config.environment.as_mut().unwrap();
             environment.insert(
                 "VHOST".into(),
-                match &service.domains.is_empty() {
-                    false => service.domains.join(" "),
-                    true => format!("{}.{}", &service.service, &settings.domain),
-                },
+                service.get_domains(&settings.domain).join(" "),
             );
             environment.insert("VPORT".into(), format!("{}", &service.port));
 

scotty/src/docker/loadbalancer/traefik.rs

@@ -113,10 +113,7 @@ impl LoadBalancerImpl for TraefikLoadBalancer {
             // Add Traefik labels
             labels.insert("traefik.enable".to_string(), "true".to_string());
 
-            let domains = match &service.domains.is_empty() {
-                false => &service.domains,
-                true => &vec![format!("{}.{}", &service.service, &settings.domain)],
-            };
+            let domains = service.get_domains(&settings.domain);
             for (idx, domain) in domains.iter().enumerate() {
                 labels.insert(
                     format!("traefik.http.routers.{}-{}.rule", &service_name, idx),