fix(iroh): handle rpc args in any position (n0-computer#1739)

It is now possible to do any of these ``` > iroh start --rpc-port 1234 > iroh --rpc-port 1234 start > iroh console --rpc-port 1234 > iroh --rpc-port 1234 console ``` Closes n0-computer#1639
divagant-martian · Oct 31, 2023 · 67b14f7 · 67b14f7
1 parent 89e7a10
commit 67b14f7
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 61 deletions.
diff --git a/iroh/src/commands.rs b/iroh/src/commands.rs
@@ -28,7 +28,7 @@ use iroh_net::{
 
 use crate::config::{ConsoleEnv, NodeConfig};
 
-use self::node::{RpcPort, StartOptions};
+use self::node::StartOptions;
 use self::sync::{AuthorCommands, DocCommands};
 
 const DEFAULT_RPC_PORT: u16 = 0x1337;
@@ -53,23 +53,15 @@ pub struct Cli {
     #[clap(subcommand)]
     pub command: Commands,
 
-    #[clap(flatten)]
-    #[clap(next_help_heading = "Options for console, doc, author, blob, node, tag")]
-    pub rpc_args: RpcArgs,
+    /// RPC port of the Iroh node.
+    #[clap(long, global = true, default_value_t = DEFAULT_RPC_PORT)]
+    pub rpc_port: u16,
 
     #[clap(flatten)]
     #[clap(next_help_heading = "Options for start, get, doctor")]
     pub full_args: FullArgs,
 }
 
-/// Options for commands that talk to a running Iroh node over RPC
-#[derive(Args, Debug, Clone)]
-pub struct RpcArgs {
-    /// RPC port of the Iroh node
-    #[clap(long, default_value_t = DEFAULT_RPC_PORT)]
-    pub rpc_port: u16,
-}
-
 /// Options for commands that may start an Iroh node
 #[derive(Args, Debug, Clone)]
 pub struct FullArgs {
@@ -88,12 +80,12 @@ impl Cli {
     pub async fn run(self, rt: runtime::Handle) -> Result<()> {
         match self.command {
             Commands::Console => {
-                let iroh = iroh::client::quic::connect(self.rpc_args.rpc_port, Some(rt)).await?;
+                let iroh = iroh::client::quic::connect(self.rpc_port, Some(rt)).await?;
                 let env = ConsoleEnv::for_console()?;
                 repl::run(&iroh, &env).await
             }
             Commands::Rpc(command) => {
-                let iroh = iroh::client::quic::connect(self.rpc_args.rpc_port, Some(rt)).await?;
+                let iroh = iroh::client::quic::connect(self.rpc_port, Some(rt)).await?;
                 let env = ConsoleEnv::for_cli()?;
                 command.run(&iroh, &env).await
             }
@@ -109,7 +101,7 @@ impl Cli {
                 #[cfg(feature = "metrics")]
                 let metrics_fut = start_metrics_server(metrics_addr, &rt);
 
-                let res = command.run(&rt, &config, keylog).await;
+                let res = command.run(&rt, &config, keylog, self.rpc_port).await;
 
                 #[cfg(feature = "metrics")]
                 if let Some(metrics_fut) = metrics_fut {
@@ -152,9 +144,6 @@ pub enum FullCommands {
         /// Listening address to bind to
         #[clap(long, short, default_value_t = SocketAddr::from(iroh::node::DEFAULT_BIND_ADDR))]
         addr: SocketAddr,
-        /// RPC port, set to "disabled" to disable RPC
-        #[clap(long, default_value_t = RpcPort::Enabled(DEFAULT_RPC_PORT))]
-        rpc_port: RpcPort,
         /// Use a token to authenticate requests for data
         ///
         /// Pass "random" to generate a random token, or base32-encoded bytes to use as a token
@@ -217,11 +206,16 @@ pub enum FullCommands {
 }
 
 impl FullCommands {
-    pub async fn run(self, rt: &runtime::Handle, config: &NodeConfig, keylog: bool) -> Result<()> {
+    pub async fn run(
+        self,
+        rt: &runtime::Handle,
+        config: &NodeConfig,
+        keylog: bool,
+        rpc_port: u16,
+    ) -> Result<()> {
         match self {
             FullCommands::Start {
                 addr,
-                rpc_port,
                 request_token,
                 add_options,
             } => {

diff --git a/iroh/src/commands/node.rs b/iroh/src/commands/node.rs
@@ -1,8 +1,6 @@
 use std::{
-    fmt,
     net::{Ipv4Addr, SocketAddr, SocketAddrV4},
     path::PathBuf,
-    str::FromStr,
     sync::Arc,
 };
 
@@ -28,7 +26,7 @@ use super::{BlobAddOptions, MAX_RPC_CONNECTIONS, MAX_RPC_STREAMS};
 #[derive(Debug)]
 pub struct StartOptions {
     pub addr: SocketAddr,
-    pub rpc_port: RpcPort,
+    pub rpc_port: u16,
     pub keylog: bool,
     pub request_token: Option<RequestToken>,
     pub derp_map: Option<DerpMap>,
@@ -175,39 +173,3 @@ fn make_rpc_endpoint(
         QuinnServerEndpoint::<ProviderRequest, ProviderResponse>::new(rpc_quinn_endpoint)?;
     Ok(rpc_endpoint)
 }
-
-#[derive(Debug, Clone)]
-pub enum RpcPort {
-    Enabled(u16),
-    Disabled,
-}
-
-impl From<RpcPort> for Option<u16> {
-    fn from(value: RpcPort) -> Self {
-        match value {
-            RpcPort::Enabled(port) => Some(port),
-            RpcPort::Disabled => None,
-        }
-    }
-}
-
-impl fmt::Display for RpcPort {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            RpcPort::Enabled(port) => write!(f, "{port}"),
-            RpcPort::Disabled => write!(f, "disabled"),
-        }
-    }
-}
-
-impl FromStr for RpcPort {
-    type Err = anyhow::Error;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if s == "disabled" {
-            Ok(RpcPort::Disabled)
-        } else {
-            Ok(RpcPort::Enabled(s.parse()?))
-        }
-    }
-}
diff --git a/iroh/tests/cli.rs b/iroh/tests/cli.rs
@@ -342,7 +342,7 @@ fn cli_provide_persistence() -> anyhow::Result<()> {
                 "--addr",
                 ADDR,
                 "--rpc-port",
-                "disabled",
+                "0",
                 path.to_str().unwrap(),
                 "--wrap",
             ],
@@ -478,7 +478,7 @@ fn make_provider_in(
         "--addr",
         addr.unwrap_or(ADDR),
         "--rpc-port",
-        rpc_port.unwrap_or("disabled"),
+        rpc_port.unwrap_or("0"),
     ];
     if wrap {
         args.push("--wrap");