Skip to content

Commit

Permalink
ROVER-305 Update address precedence logic (#2374)
Browse files Browse the repository at this point in the history
The current address deciding logic, cannot differentiate between a value
in the config that has been overriden by the config file or by another
source. As such it's impossible to tell if it should update that value
when the config file changes and so it makes the wrong choice.

This fixes that by expanding the expressiveness of the RouterAddress
struct, and thus produces something that makes the correct choice in
this and other situations.
  • Loading branch information
jonathanrainer authored Jan 30, 2025
1 parent 72622af commit c370d74
Show file tree
Hide file tree
Showing 5 changed files with 232 additions and 139 deletions.
15 changes: 11 additions & 4 deletions src/command/dev/do_dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rover_std::{errln, infoln, warnln};
use semver::Version;
use tower::ServiceExt;

use crate::command::dev::router::config::RouterAddress;
use crate::command::dev::router::config::{RouterAddress, RouterHost, RouterPort};
use crate::command::dev::router::hot_reload::HotReloadConfigOverrides;
use crate::command::dev::router::run::RunRouter;
use crate::command::dev::{OVERRIDE_DEV_COMPOSITION_VERSION, OVERRIDE_DEV_ROUTER_VERSION};
Expand Down Expand Up @@ -204,8 +204,14 @@ impl Dev {
// default, but we still have to reckon with the config-set address (if one exists). See
// the reassignment of the variable below for details
let router_address = RouterAddress::new(
self.opts.supergraph_opts.supergraph_address,
self.opts.supergraph_opts.supergraph_port,
self.opts
.supergraph_opts
.supergraph_address
.map(RouterHost::CliOption),
self.opts
.supergraph_opts
.supergraph_port
.map(RouterPort::CliOption),
);

let run_router = RunRouter::default()
Expand Down Expand Up @@ -251,7 +257,8 @@ impl Dev {
"Do not run this command in production! It is intended for local development only."
);

infoln!("Your supergraph is running! head to {router_address} to query your supergraph");
let pretty_router_addr_string = router_address.pretty_string();
infoln!("Your supergraph is running! head to {pretty_router_addr_string} to query your supergraph");

loop {
tokio::select! {
Expand Down
95 changes: 65 additions & 30 deletions src/command/dev/router/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ pub mod parser;
pub mod remote;
mod state;

const DEFAULT_ROUTER_IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1));
const DEFAULT_ROUTER_PORT: u16 = 4000;
const DEFAULT_ROUTER_IP_ADDR: RouterHost =
RouterHost::Default(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)));
const DEFAULT_ROUTER_PORT: RouterPort = RouterPort::Default(4000);

#[derive(Error, Debug)]
pub enum ReadRouterConfigError {
Expand All @@ -36,32 +37,84 @@ pub enum ReadRouterConfigError {
Expansion { path: Utf8PathBuf },
}

#[derive(Copy, Clone, derive_getters::Getters)]
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum RouterHost {
CliOption(IpAddr),
ConfigFile(IpAddr),
Default(IpAddr),
}

impl RouterHost {
fn get_ip_addr(&self) -> IpAddr {
match self {
RouterHost::CliOption(ip_addr)
| RouterHost::ConfigFile(ip_addr)
| RouterHost::Default(ip_addr) => *ip_addr,
}
}
}

impl Display for RouterHost {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
self.get_ip_addr().fmt(f)
}
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum RouterPort {
CliOption(u16),
ConfigFile(u16),
Default(u16),
}

impl RouterPort {
fn get_port(&self) -> u16 {
match self {
RouterPort::CliOption(port)
| RouterPort::ConfigFile(port)
| RouterPort::Default(port) => *port,
}
}
}

impl Display for RouterPort {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
self.get_port().fmt(f)
}
}

#[derive(Copy, Clone, Debug, derive_getters::Getters, PartialEq, Eq)]
pub struct RouterAddress {
host: IpAddr,
port: u16,
host: RouterHost,
port: RouterPort,
}

#[buildstructor]
impl RouterAddress {
#[builder]
pub fn new(host: Option<IpAddr>, port: Option<u16>) -> RouterAddress {
pub fn new(host: Option<RouterHost>, port: Option<RouterPort>) -> RouterAddress {
let host = host.unwrap_or(DEFAULT_ROUTER_IP_ADDR);
let port = port.unwrap_or(DEFAULT_ROUTER_PORT);
RouterAddress { host, port }
}
}

impl Display for RouterAddress {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
impl RouterAddress {
pub(crate) fn pretty_string(&self) -> String {
let host = self
.host
.to_string()
.replace("127.0.0.1", "localhost")
.replace("0.0.0.0", "localhost")
.replace("[::]", "localhost")
.replace("[::1]", "localhost");
write!(f, "http://{}:{}", host, self.port)
format!("http://{}:{}", host, self.port)
}
}

impl Display for RouterAddress {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "{}:{}", self.host, self.port)
}
}

Expand All @@ -78,31 +131,15 @@ impl From<RouterAddress> for SocketAddr {
fn from(value: RouterAddress) -> Self {
let host = value.host;
let port = value.port;
SocketAddr::new(host, port)
SocketAddr::new(host.get_ip_addr(), port.get_port())
}
}

impl From<&RouterAddress> for SocketAddr {
fn from(value: &RouterAddress) -> Self {
let host = value.host;
let port = value.port;
SocketAddr::new(host, port)
}
}

impl From<Option<SocketAddr>> for RouterAddress {
fn from(value: Option<SocketAddr>) -> Self {
let host = value.map(|addr| addr.ip());
let port = value.map(|addr| addr.port());
RouterAddress::new(host, port)
}
}

impl From<SocketAddr> for RouterAddress {
fn from(value: SocketAddr) -> Self {
let host = value.ip();
let port = value.port();
RouterAddress { host, port }
SocketAddr::new(host.get_ip_addr(), port.get_port())
}
}

Expand Down Expand Up @@ -159,10 +196,8 @@ impl RunRouterConfig<RunRouterConfigReadConfig> {
Err(_) => Err(ReadRouterConfigError::Expansion { path: path.clone() }),
}?;

let router_config =
RouterConfigParser::new(&yaml, self.state.router_address.into());
let router_config = RouterConfigParser::new(&yaml, self.state.router_address);
let address = router_config.address()?;
let address = address.into();
let health_check_enabled = router_config.health_check_enabled();
let health_check_endpoint = router_config.health_check_endpoint()?;
let health_check_path = router_config.health_check_path();
Expand Down
Loading

0 comments on commit c370d74

Please sign in to comment.