fix(dev): router config healthcheck (#2332)

see [jira](https://apollographql.atlassian.net/browse/BGS-132) for
proof/details

src/command/dev/next/router/config/mod.rs

@@ -149,13 +149,15 @@ impl RunRouterConfig<RunRouterConfigReadConfig> {
                             .unwrap_or(self.state.router_address);
                         let health_check_enabled = router_config.health_check_enabled();
                         let health_check_endpoint = router_config.health_check_endpoint()?;
-                        let listen_path = router_config.listen_path()?;
+                        let health_check_path = router_config.health_check_path();
+                        let listen_path = router_config.listen_path();
 
                         Ok(RunRouterConfigFinal {
                             listen_path,
                             address,
                             health_check_enabled,
                             health_check_endpoint,
+                            health_check_path,
                             raw_config: contents.to_string(),
                         })
                     }
@@ -174,8 +176,8 @@ impl RunRouterConfig<RunRouterConfigReadConfig> {
 
 impl RunRouterConfig<RunRouterConfigFinal> {
     #[allow(unused)]
-    pub fn listen_path(&self) -> Option<&Uri> {
-        self.state.listen_path.as_ref()
+    pub fn listen_path(&self) -> Option<String> {
+        self.state.listen_path.clone()
     }
 
     #[allow(unused)]
@@ -187,8 +189,12 @@ impl RunRouterConfig<RunRouterConfigFinal> {
         self.state.health_check_enabled
     }
 
-    pub fn health_check_endpoint(&self) -> &Uri {
-        &self.state.health_check_endpoint
+    pub fn health_check_endpoint(&self) -> Option<&SocketAddr> {
+        self.state.health_check_endpoint.as_ref()
+    }
+
+    pub fn health_check_path(&self) -> String {
+        self.state.health_check_path.clone()
     }
 
     pub fn raw_config(&self) -> String {

src/command/dev/next/router/config/parser.rs

@@ -1,9 +1,9 @@
 use std::{
-    net::{SocketAddr, ToSocketAddrs},
+    io::Error,
+    net::{SocketAddr, SocketAddrV4, ToSocketAddrs},
     str::FromStr,
 };
 
-use http::{uri::InvalidUri, Uri};
 use thiserror::Error;
 
 #[derive(Error, Debug)]
@@ -13,13 +13,6 @@ pub enum ParseRouterConfigError {
         path: &'static str,
         source: std::io::Error,
     },
-    #[error("Invalid Uri at {}. Error: {:?}", .path, .source)]
-    ListenPath {
-        path: &'static str,
-        source: InvalidUri,
-    },
-    #[error("Invalid Uri when combining the health_check address, port, and path. Error: {:?}", .source)]
-    HealthCheckEndpoint { source: InvalidUri },
 }
 
 pub struct RouterConfigParser<'a> {
@@ -60,52 +53,58 @@ impl<'a> RouterConfigParser<'a> {
             .and_then(|enabled| enabled.as_bool())
             .unwrap_or_default()
     }
-    pub fn listen_path(&self) -> Result<Option<Uri>, ParseRouterConfigError> {
+    pub fn listen_path(&self) -> Option<String> {
         self.yaml
             .get("supergraph")
             .and_then(|supergraph| supergraph.as_mapping())
             .and_then(|supergraph| supergraph.get("path"))
-            .and_then(|path| path.as_str())
-            .and_then(|path| Some(Uri::from_str(path)))
-            .transpose()
-            .map_err(|err| ParseRouterConfigError::ListenPath {
-                path: "supergraph.path",
-                source: err,
-            })
+            .and_then(|path| path.as_str().map(|path| path.to_string()))
     }
-    pub fn health_check_endpoint(&self) -> Result<Uri, ParseRouterConfigError> {
-        let addr_and_port = self
-            .yaml
-            .get("health_check")
-            .or_else(|| self.yaml.get("health-check"))
-            .and_then(|health_check| health_check.as_mapping())
-            .and_then(|health_check| health_check.get("listen"))
-            .and_then(|addr_and_port| addr_and_port.as_str())
-            .and_then(|addr_and_port| Some(Uri::from_str(addr_and_port)))
-            // See https://www.apollographql.com/docs/graphos/routing/self-hosted/health-checks for
-            // defaults
-            .unwrap_or(Uri::from_str("http://127.0.0.1:8088"))
-            .map_err(|err| ParseRouterConfigError::ListenPath {
-                path: "health_check.listen",
-                source: err,
-            })?;
-
-        let path = self
-            .yaml
+    /// Gets the user-defined health_check_endpoint or, if missing, returns the default
+    /// 127.0.0.1:8088
+    ///
+    /// See https://www.apollographql.com/docs/graphos/routing/self-hosted/health-checks
+    pub fn health_check_endpoint(&self) -> Result<Option<SocketAddr>, ParseRouterConfigError> {
+        Ok(Some(
+            self.yaml
+                .get("health_check")
+                .or_else(|| self.yaml.get("health-check"))
+                .and_then(|health_check| health_check.as_mapping())
+                .and_then(|health_check| health_check.get("listen"))
+                .and_then(|addr_and_port| addr_and_port.as_str())
+                .and_then(|path| {
+                    path.to_string()
+                        .to_socket_addrs()
+                        .map(|mut addrs| addrs.next())
+                        .transpose()
+                })
+                .transpose()
+                .map_err(|err| ParseRouterConfigError::ParseAddress {
+                    path: "health_check.listen",
+                    source: err,
+                })?
+                .unwrap_or(SocketAddr::V4(
+                    SocketAddrV4::from_str("127.0.0.1:8088").map_err(|err| {
+                        ParseRouterConfigError::ParseAddress {
+                            path: "health_check.listen",
+                            source: Error::new(std::io::ErrorKind::InvalidInput, err.to_string()),
+                        }
+                    })?,
+                )),
+        ))
+    }
+    /// Gets the user-defined health_check_path or, if absent, returns the default `/health`
+    ///
+    /// See https://www.apollographql.com/docs/graphos/routing/self-hosted/health-checks
+    pub fn health_check_path(&self) -> String {
+        self.yaml
             .get("health_check")
             .or_else(|| self.yaml.get("health-check"))
             .and_then(|health_check| health_check.as_mapping())
             .and_then(|health_check| health_check.get("path"))
             .and_then(|path| path.as_str())
-            // See https://www.apollographql.com/docs/graphos/routing/self-hosted/health-checks for
-            // defaults
-            .unwrap_or("health");
-
-        let mut health_check_endpoint = addr_and_port.to_string();
-        health_check_endpoint.push_str(path);
-
-        Ok(Uri::from_str(&health_check_endpoint)
-            .map_err(|err| ParseRouterConfigError::HealthCheckEndpoint { source: err })?)
+            .unwrap_or("/health")
+            .to_string()
     }
 }
 
@@ -114,7 +113,6 @@ mod tests {
     use std::{net::SocketAddr, str::FromStr};
 
     use anyhow::Result;
-    use http::Uri;
     use rstest::rstest;
     use speculoos::prelude::*;
 
@@ -161,8 +159,8 @@ health_check:
         );
         let config_yaml = serde_yaml::from_str(&config_yaml_str)?;
         let router_config = RouterConfigParser { yaml: &config_yaml };
-        let health_check = router_config.health_check_enabled();
-        assert_that!(health_check).is_equal_to(is_health_check_enabled);
+        let health_check_enabled = router_config.health_check_enabled();
+        assert_that!(health_check_enabled).is_equal_to(is_health_check_enabled);
         Ok(())
     }
 
@@ -174,10 +172,9 @@ health_check:
         };
         let config_yaml = serde_yaml::from_str(&config_yaml_str)?;
         let router_config = RouterConfigParser { yaml: &config_yaml };
-        let health_check = router_config.health_check_endpoint();
-        assert_that!(health_check)
-            .is_ok()
-            .is_equal_to(Uri::from_str("http://127.0.0.1:8088/health")?);
+        let health_check = router_config.health_check_endpoint()?.unwrap().to_string();
+
+        assert_that!(health_check).is_equal_to("127.0.0.1:8088".to_string());
         Ok(())
     }
 
@@ -189,8 +186,7 @@ health_check:
         };
         let config_yaml = serde_yaml::from_str(&config_yaml_str)?;
         let router_config = RouterConfigParser { yaml: &config_yaml };
-        let health_check = router_config.listen_path();
-        assert_that!(health_check).is_ok().is_none();
+        assert_that!(router_config.listen_path()).is_none();
         Ok(())
     }
 
@@ -204,11 +200,9 @@ supergraph:
         };
         let config_yaml = serde_yaml::from_str(&config_yaml_str)?;
         let router_config = RouterConfigParser { yaml: &config_yaml };
-        let address = router_config.listen_path();
-        assert_that!(address)
-            .is_ok()
+        assert_that!(router_config.listen_path())
             .is_some()
-            .is_equal_to(Uri::from_str("/custom-path").unwrap());
+            .is_equal_to("/custom-path".to_string());
         Ok(())
     }
 }

src/command/dev/next/router/config/state.rs

@@ -1,4 +1,4 @@
-use http::Uri;
+use std::net::SocketAddr;
 
 use super::RouterAddress;
 
@@ -8,14 +8,27 @@ pub struct RunRouterConfigReadConfig {
     pub router_address: RouterAddress,
 }
 
-#[derive(Default)]
 pub struct RunRouterConfigFinal {
     #[allow(unused)]
-    pub listen_path: Option<Uri>,
+    pub listen_path: Option<String>,
     #[allow(unused)]
     pub address: RouterAddress,
     pub health_check_enabled: bool,
-    pub health_check_endpoint: Uri,
+    pub health_check_endpoint: Option<SocketAddr>,
+    pub health_check_path: String,
     #[allow(unused)]
     pub raw_config: String,
 }
+
+impl Default for RunRouterConfigFinal {
+    fn default() -> Self {
+        Self {
+            listen_path: Option::default(),
+            address: RouterAddress::default(),
+            health_check_enabled: bool::default(),
+            health_check_endpoint: Option::default(),
+            health_check_path: "/health".to_string(),
+            raw_config: String::default(),
+        }
+    }
+}

src/command/dev/next/router/run.rs

@@ -227,8 +227,18 @@ impl RunRouter<state::Run> {
             return Ok(());
         }
 
-        let healthcheck_endpoint = self.state.config.health_check_endpoint();
+        // We hardcode the endpoint and port; if they're missing now, we've lost that bit of code
+        let mut healthcheck_endpoint = match self.state.config.health_check_endpoint() {
+            Some(endpoint) => endpoint.to_string(),
+            None => {
+            return Err(RunRouterBinaryError::Internal {
+                dependency: "Router Config Validation".to_string(),
+                err: format!("Router Config passed validation incorrectly, healthchecks are enabled but missing an endpoint"),
+            })
+            }
+        };
 
+        healthcheck_endpoint.push_str(&self.state.config.health_check_path());
         let healthcheck_client = studio_client_config.get_reqwest_client().map_err(|err| {
             RunRouterBinaryError::Internal {
                 dependency: "Reqwest Client".to_string(),
@@ -237,7 +247,7 @@ impl RunRouter<state::Run> {
         })?;
 
         let healthcheck_request = healthcheck_client
-            .get(healthcheck_endpoint.to_string())
+            .get(format!("http://{healthcheck_endpoint}"))
             .build()
             .map_err(|err| RunRouterBinaryError::Internal {
                 dependency: "Reqwest Client".to_string(),