Fix/reject empty service block

docs/STACKER_YML_REFERENCE.md

@@ -1540,6 +1540,10 @@ Stacker validates your configuration both syntactically (YAML structure) and sem
 | `E002` | Server deployment requires `deploy.server.host` | `deploy.server.host` |
 | `E003` | Custom app type requires `app.image` or `app.dockerfile` | `app` |
 | `E004` | `deploy.environment` references an undefined environment key | `deploy.environment` / `environments` |
+| `E005` | `deploy.default_target` missing or naming an undefined target; invalid `deploy.cloud.public_ports` entry | `deploy.default_target` / `deploy.cloud.public_ports` |
+| `E006` | A `deploy.targets` profile defines both `server` and `cloud` | `deploy.targets.<name>` |
+| `E007` | Invalid port mapping in `app.ports` or `services.*.ports` | `app.ports` / `services.ports` |
+| `E008` | A proxy is enabled but a `proxy.domains` entry is incomplete — empty domain, or an empty/malformed upstream *(added in 0.3.3)* | `proxy.domains[N]` |
 
 ### Warnings (deployment may have issues)
 

src/cli/config_parser.rs

@@ -1305,6 +1305,20 @@ impl<'de> Deserialize<'de> for TargetConfigContract {
                 .or_insert_with(|| FieldPolicy::fixed(false));
         }
 
+        // A block that declares nothing is meaningless — a service with no
+        // policy is simply left out. In practice it means a declaration was
+        // dropped somewhere between the file and here, which is what an older
+        // CLI did with `volumes:` before that kind reached `Serialize`: the
+        // submit succeeded, the stored contract held `"my-service": {}`, and
+        // the bake reset a volume the author had asked to keep.
+        if fields.is_empty() && raw.volumes.is_empty() {
+            return Err(serde::de::Error::custom(
+                "declares neither `fields:` nor `volumes:`. A service with no policy \
+                 does not need a block at all — an empty one usually means a \
+                 declaration was lost, so it is refused rather than silently ignored.",
+            ));
+        }
+
         Ok(TargetConfigContract {
             fields,
             volumes: raw.volumes,
@@ -1793,6 +1807,78 @@ impl StackerConfig {
             }
         }
 
+        // E008 — a proxy that is switched on must be given everything it needs
+        // to route. A routing entry with no domain is not a route: NPM rejects
+        // a proxy host with no name, nginx and caddy have no server_name to
+        // match on, and traefik's Host() rule matches nothing. It used to reach
+        // the target anyway, where the NPM role answered with a censored 4xx
+        // and took the whole deploy with it — after the server was already
+        // provisioned. The usual cause is an unset `${commonDomain}`: an
+        // `env_file` line like `commonDomain=` resolves to the empty string
+        // instead of failing, so the blank travels all the way to Ansible.
+        //
+        // Checked for every proxy type but `none`, since all of them route by
+        // name. An empty `domains:` list is not an error — a proxy can be
+        // deployed to be configured through its own UI later.
+        if self.proxy.proxy_type != ProxyType::None {
+            for (index, domain_config) in self.proxy.domains.iter().enumerate() {
+                let field = format!("proxy.domains[{index}]");
+
+                if domain_config.domain.trim().is_empty() {
+                    issues.push(ValidationIssue {
+                        severity: Severity::Error,
+                        code: "E008".to_string(),
+                        message: format!(
+                            "proxy.domains[{index}] has an empty domain, but proxy.type is \
+                             '{}'. A reverse proxy routes by name, so a blank domain cannot be \
+                             configured and the deploy fails on the target. If the domain comes \
+                             from a variable such as ${{commonDomain}}, set it in the env_file or \
+                             the environment; if this route is not needed yet, remove the entry.",
+                            self.proxy.proxy_type
+                        ),
+                        field: Some(format!("{field}.domain")),
+                    });
+                }
+
+                if domain_config.upstream.trim().is_empty() {
+                    issues.push(ValidationIssue {
+                        severity: Severity::Error,
+                        code: "E008".to_string(),
+                        message: format!(
+                            "proxy.domains[{index}] has an empty upstream. The proxy needs a \
+                             target to forward to, in the form 'service:port' (e.g. 'app:8080')."
+                        ),
+                        field: Some(format!("{field}.upstream")),
+                    });
+                } else if let Some((host, port)) = domain_config.upstream.rsplit_once(':') {
+                    // The port half is only a port when the upstream is written
+                    // `host:port`; a bare hostname is accepted and defaults to 80.
+                    if host.trim().is_empty() {
+                        issues.push(ValidationIssue {
+                            severity: Severity::Error,
+                            code: "E008".to_string(),
+                            message: format!(
+                                "proxy.domains[{index}] upstream '{}' has no host before the \
+                                 port. Expected 'service:port', e.g. 'app:8080'.",
+                                domain_config.upstream
+                            ),
+                            field: Some(format!("{field}.upstream")),
+                        });
+                    } else if let Err(err) = validate_port_number(port) {
+                        issues.push(ValidationIssue {
+                            severity: Severity::Error,
+                            code: "E008".to_string(),
+                            message: format!(
+                                "proxy.domains[{index}] upstream '{}' has an invalid port: {err}.",
+                                domain_config.upstream
+                            ),
+                            field: Some(format!("{field}.upstream")),
+                        });
+                    }
+                }
+            }
+        }
+
         // W003 — a `proxy:` block deploys a *platform-managed* reverse proxy
         // that owns the ingress host ports on the target. Any app/service that
         // also publishes one of those host ports collides with it: the managed
@@ -2802,6 +2888,29 @@ config_contract:
         );
     }
 
+    /// A service block that declares nothing is almost always a declaration
+    /// that got lost on the way out — which is exactly what an older CLI did
+    /// with `volumes:` before it was added to `Serialize`. The submit
+    /// succeeded, the stored contract had `"my-service": {}`, and the bake
+    /// then reset a volume the author had asked to keep. Nothing complained.
+    ///
+    /// An empty block is also meaningless on its own: a service with no policy
+    /// simply goes unmentioned.
+    #[test]
+    fn an_empty_service_block_is_rejected() {
+        let yaml = r#"
+name: s
+config_contract:
+  services:
+    my-service: {}
+"#;
+        let err = StackerConfig::from_str(yaml).expect_err("an empty block means nothing");
+        assert!(
+            err.to_string().contains("my-service"),
+            "the error should name the service: {err}"
+        );
+    }
+
     #[test]
     fn volume_policy_survives_a_round_trip() {
         let yaml = r#"
@@ -3808,6 +3917,113 @@ app:
     // into the generated compose verbatim with no range check; a bare
     // out-of-range entry is read by Docker as a container port.
 
+    fn proxy_config_with(proxy_type: ProxyType, domains: Vec<(&str, &str)>) -> StackerConfig {
+        ConfigBuilder::new()
+            .name("proxy-validation")
+            .proxy(ProxyConfig {
+                proxy_type,
+                auto_detect: true,
+                domains: domains
+                    .into_iter()
+                    .map(|(domain, upstream)| DomainConfig {
+                        domain: domain.to_string(),
+                        ssl: SslMode::Auto,
+                        upstream: upstream.to_string(),
+                    })
+                    .collect(),
+                config: None,
+            })
+            .build()
+            .unwrap()
+    }
+
+    fn errors_with_code(config: &StackerConfig, code: &str) -> Vec<ValidationIssue> {
+        config
+            .validate_semantics()
+            .into_iter()
+            .filter(|issue| issue.severity == Severity::Error && issue.code == code)
+            .collect()
+    }
+
+    #[test]
+    fn e008_rejects_a_blank_domain_when_a_proxy_is_enabled() {
+        // The failure this guards: `${commonDomain}` resolved to "" (an
+        // `env_file` line `commonDomain=`), the blank reached the NPM role, and
+        // the deploy died with a censored error after the server existed.
+        for proxy_type in [
+            ProxyType::NginxProxyManager,
+            ProxyType::Nginx,
+            ProxyType::Caddy,
+            ProxyType::Traefik,
+        ] {
+            let config = proxy_config_with(proxy_type, vec![("", "app:8080")]);
+            let errors = errors_with_code(&config, "E008");
+            assert_eq!(
+                errors.len(),
+                1,
+                "{proxy_type} should reject a blank domain: {:?}",
+                config.validate_semantics()
+            );
+            assert_eq!(errors[0].field.as_deref(), Some("proxy.domains[0].domain"));
+        }
+    }
+
+    #[test]
+    fn e008_rejects_a_whitespace_only_domain() {
+        let config = proxy_config_with(ProxyType::NginxProxyManager, vec![("   ", "app:8080")]);
+        assert_eq!(errors_with_code(&config, "E008").len(), 1);
+    }
+
+    #[test]
+    fn e008_reports_the_index_of_each_bad_entry() {
+        let config = proxy_config_with(
+            ProxyType::NginxProxyManager,
+            vec![("app.example.com", "app:8080"), ("", "api:9000")],
+        );
+        let errors = errors_with_code(&config, "E008");
+        assert_eq!(errors.len(), 1);
+        assert_eq!(errors[0].field.as_deref(), Some("proxy.domains[1].domain"));
+    }
+
+    #[test]
+    fn e008_rejects_a_blank_or_malformed_upstream() {
+        let blank = proxy_config_with(ProxyType::Nginx, vec![("app.example.com", "")]);
+        assert_eq!(errors_with_code(&blank, "E008").len(), 1);
+
+        let no_host = proxy_config_with(ProxyType::Nginx, vec![("app.example.com", ":8080")]);
+        assert_eq!(errors_with_code(&no_host, "E008").len(), 1);
+
+        let bad_port = proxy_config_with(ProxyType::Nginx, vec![("app.example.com", "app:99999")]);
+        assert_eq!(errors_with_code(&bad_port, "E008").len(), 1);
+    }
+
+    #[test]
+    fn e008_accepts_a_complete_routing_entry() {
+        let with_port = proxy_config_with(
+            ProxyType::NginxProxyManager,
+            vec![("app.example.com", "app:8080")],
+        );
+        assert!(errors_with_code(&with_port, "E008").is_empty());
+
+        // A bare hostname is legal — the proxy defaults the port to 80.
+        let no_port = proxy_config_with(ProxyType::Nginx, vec![("app.example.com", "app")]);
+        assert!(errors_with_code(&no_port, "E008").is_empty());
+    }
+
+    #[test]
+    fn e008_ignores_domains_when_no_proxy_is_enabled() {
+        // Without a proxy nothing reads these entries, so a blank is inert.
+        let config = proxy_config_with(ProxyType::None, vec![("", "")]);
+        assert!(errors_with_code(&config, "E008").is_empty());
+    }
+
+    #[test]
+    fn e008_allows_a_proxy_with_no_routing_entries() {
+        // A proxy can be deployed to be configured through its own UI later.
+        let config = proxy_config_with(ProxyType::NginxProxyManager, vec![]);
+        assert!(errors_with_code(&config, "E008").is_empty());
+    }
+
     #[test]
     fn validate_port_mapping_accepts_every_compose_shape() {
         for good in [

src/cli/stacker_client.rs

@@ -4396,10 +4396,18 @@ pub fn build_deploy_form(config: &StackerConfig) -> serde_json::Value {
     // to the Install Service, which passes them to the proxy role as the
     // `stacker_proxy_domains` extra var. (Traefik routes via container labels
     // generated into the compose, so it does not need this.)
-    if !config.proxy.domains.is_empty() {
-        let domains: Vec<serde_json::Value> = config
-            .proxy
-            .domains
+    // A domain that resolved to nothing (an unset `${commonDomain}`, say) cannot
+    // become a routing entry: the proxy has no name to match on, and NPM refuses
+    // the request outright, failing the whole deploy over a route nobody asked
+    // for. Drop the blanks and route whatever is left.
+    let routed_domains: Vec<&crate::cli::config_parser::DomainConfig> = config
+        .proxy
+        .domains
+        .iter()
+        .filter(|d| !d.domain.trim().is_empty())
+        .collect();
+    if !routed_domains.is_empty() {
+        let domains: Vec<serde_json::Value> = routed_domains
             .iter()
             .map(|d| {
                 let ssl = match d.ssl {
@@ -5109,6 +5117,75 @@ mod tests {
         assert_eq!(domains[1]["ssl"], "off");
     }
 
+    #[test]
+    fn test_build_deploy_form_drops_blank_proxy_domains() {
+        // `${commonDomain}` that resolved to nothing leaves an entry whose domain
+        // is empty. It reached the NPM role, which answered with a censored 4xx
+        // and took the whole deploy down (install 4068). A route with no name is
+        // not a route — it must never leave the CLI.
+        let config = crate::cli::config_parser::ConfigBuilder::new()
+            .name("myproject")
+            .deploy_target(crate::cli::config_parser::DeployTarget::Cloud)
+            .proxy(crate::cli::config_parser::ProxyConfig {
+                proxy_type: crate::cli::config_parser::ProxyType::NginxProxyManager,
+                auto_detect: true,
+                domains: vec![
+                    crate::cli::config_parser::DomainConfig {
+                        domain: String::new(),
+                        ssl: crate::cli::config_parser::SslMode::Auto,
+                        upstream: "app:8080".to_string(),
+                    },
+                    crate::cli::config_parser::DomainConfig {
+                        domain: "   ".to_string(),
+                        ssl: crate::cli::config_parser::SslMode::Auto,
+                        upstream: "app:8080".to_string(),
+                    },
+                    crate::cli::config_parser::DomainConfig {
+                        domain: "app.example.com".to_string(),
+                        ssl: crate::cli::config_parser::SslMode::Auto,
+                        upstream: "app:8080".to_string(),
+                    },
+                ],
+                config: None,
+            })
+            .build()
+            .unwrap();
+
+        let form = build_deploy_form(&config);
+        let domains = form["proxy_domains"]
+            .as_array()
+            .expect("the one named domain should still be routed");
+        assert_eq!(domains.len(), 1);
+        assert_eq!(domains[0]["domain"], "app.example.com");
+    }
+
+    #[test]
+    fn test_build_deploy_form_omits_proxy_domains_when_every_domain_is_blank() {
+        // Nothing left to route: the key must be absent rather than an empty
+        // array, so the Install Service treats it as "no proxy domains".
+        let config = crate::cli::config_parser::ConfigBuilder::new()
+            .name("myproject")
+            .deploy_target(crate::cli::config_parser::DeployTarget::Cloud)
+            .proxy(crate::cli::config_parser::ProxyConfig {
+                proxy_type: crate::cli::config_parser::ProxyType::NginxProxyManager,
+                auto_detect: true,
+                domains: vec![crate::cli::config_parser::DomainConfig {
+                    domain: String::new(),
+                    ssl: crate::cli::config_parser::SslMode::Auto,
+                    upstream: "app:8080".to_string(),
+                }],
+                config: None,
+            })
+            .build()
+            .unwrap();
+
+        let form = build_deploy_form(&config);
+        assert!(
+            form.get("proxy_domains").is_none(),
+            "an all-blank domain list must not produce a proxy_domains key"
+        );
+    }
+
     #[test]
     fn test_build_deploy_form_omits_proxy_domains_when_none() {
         let config = crate::cli::config_parser::ConfigBuilder::new()