diff --git a/src/command/dev/next/router/config/mod.rs b/src/command/dev/next/router/config/mod.rs index 0e1351280..494b438e5 100644 --- a/src/command/dev/next/router/config/mod.rs +++ b/src/command/dev/next/router/config/mod.rs @@ -149,13 +149,15 @@ impl RunRouterConfig { .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 { impl RunRouterConfig { #[allow(unused)] - pub fn listen_path(&self) -> Option<&Uri> { - self.state.listen_path.as_ref() + pub fn listen_path(&self) -> Option { + self.state.listen_path.clone() } #[allow(unused)] @@ -187,8 +189,12 @@ impl RunRouterConfig { 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 { diff --git a/src/command/dev/next/router/config/parser.rs b/src/command/dev/next/router/config/parser.rs index 368ee9216..b85c172e5 100644 --- a/src/command/dev/next/router/config/parser.rs +++ b/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, ParseRouterConfigError> { + pub fn listen_path(&self) -> Option { 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 { - 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, 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(()) } } diff --git a/src/command/dev/next/router/config/state.rs b/src/command/dev/next/router/config/state.rs index b428540a4..46572ddb7 100644 --- a/src/command/dev/next/router/config/state.rs +++ b/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, + pub listen_path: Option, #[allow(unused)] pub address: RouterAddress, pub health_check_enabled: bool, - pub health_check_endpoint: Uri, + pub health_check_endpoint: Option, + 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(), + } + } +} diff --git a/src/command/dev/next/router/run.rs b/src/command/dev/next/router/run.rs index 7ff907f97..126b8f473 100644 --- a/src/command/dev/next/router/run.rs +++ b/src/command/dev/next/router/run.rs @@ -227,8 +227,18 @@ impl RunRouter { 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 { })?; 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(),