Refactor create_peer API (#389)

* Refactor create_peer()

* Add a custom error type

* Clarify that things exist

* Take ownership of addresses

* Expect that `ip_net_for()` errors should not happen

client-core/examples/add_peer.rs

@@ -2,9 +2,9 @@ use anyhow::{Context, Error};
 use env_logger::Env;
 use innernet_client_core::{
     interface::{InterfaceConfig, InterfaceName},
-    peer::{self, NewPeerInfo},
+    peer::{create_peer, NewPeerInfo},
     rest_client::RestClient,
-    CidrTree, DEFAULT_CONFIG_DIR,
+    DEFAULT_CONFIG_DIR,
 };
 use innernet_shared::prompts;
 use std::{
@@ -22,12 +22,8 @@ fn main() -> Result<(), Error> {
     let interface: InterfaceName = interface.parse()?;
     let interface_config = InterfaceConfig::from_interface(config_dir, &interface)?;
     let rest_client = RestClient::new(&interface_config.server);
-
-    let peers = rest_client.get_peers()?;
-    let server_peer = peers.iter().find(|p| p.id == 1).unwrap();
-
     let cidrs = rest_client.get_cidrs()?;
-    let cidr_tree = CidrTree::new(&cidrs);
+    let peers = rest_client.get_peers()?;
 
     let new_peer_info = NewPeerInfo {
         name: "joe".parse().unwrap(),
@@ -38,17 +34,7 @@ fn main() -> Result<(), Error> {
     };
 
     let target_path = "invitation.toml";
-    let server_api_addr = &interface_config.server.internal_endpoint;
-
-    let (peer, invitation) = peer::create_peer_and_invitation(
-        &rest_client,
-        &interface,
-        &cidr_tree,
-        server_peer,
-        new_peer_info,
-        server_api_addr,
-    )?;
-
+    let (peer, invitation) = create_peer(config_dir, &interface, &cidrs, &peers, new_peer_info)?;
     invitation.save_new(target_path)?;
     prompts::print_invitation_info(&peer, target_path);
 

client-core/src/lib.rs

@@ -3,7 +3,8 @@
 //! This is a work in progress but the final goal is to match the `innernet` CLI API surface.
 
 pub use innernet_shared::{
-    CidrTree, Endpoint, HostsOpts, NatOpts, NetworkOpts, WrappedIoError, DEFAULT_HOSTS_PATH,
+    interface_config::PeerInvitation, Cidr, CidrTree, Endpoint, HostsOpts, NatOpts, NetworkOpts,
+    Peer, WrappedIoError, DEFAULT_HOSTS_PATH,
 };
 pub use wireguard_control::Backend;
 

client-core/src/peer.rs

@@ -1,36 +1,51 @@
 pub use innernet_shared::peer::NewPeerInfo;
 
-use crate::rest_client::RestClient;
-use anyhow::Error;
-use innernet_shared::{
-    interface_config::PeerInvitation, peer::make_peer_contents_and_key_pair, CidrTree, Peer,
+use crate::{
+    rest_client::{RestClient, RestError},
+    Cidr, Peer, PeerInvitation,
 };
-use std::net::SocketAddr;
-use wireguard_control::InterfaceName;
+use anyhow::Result;
+use innernet_shared::{
+    interface_config::{InterfaceConfig, InterfaceInfo, ServerInfo},
+    CidrTree,
+};
+use std::path::Path;
+use thiserror::Error;
+use wireguard_control::{InterfaceName, KeyPair};
+
+#[derive(Debug, Error)]
+pub enum CreatePeerError {
+    // TODO(mbernat): Use a custom error type in the InterfaceConfig methods.
+    #[error("Error accessing innernet interface config file: {0}")]
+    InterfaceConfigAccess(anyhow::Error),
+    #[error("Error making a REST request: {0}")]
+    RestRequest(#[from] RestError),
+}
 
 /// Create a new innernet [`Peer`] and a [`PeerInvitation`] they can use to join the network.
-//
-//  TODO(mbernat): The shape of this API is only provisional, it reflects the client-side `add-peer`
-//                 CLI, where it was pulled from.
-//                 See https://github.com/tonarino/innernet/pull/382#discussion_r2859409122
-pub fn create_peer_and_invitation(
-    rest_client: &RestClient,
+pub fn create_peer(
+    config_dir: &Path,
     interface: &InterfaceName,
-    cidr_tree: &CidrTree,
-    server_peer: &Peer,
+    existing_cidrs: &[Cidr],
+    existing_peers: &[Peer],
     new_peer_info: NewPeerInfo,
-    server_api_addr: &SocketAddr,
-) -> Result<(Peer, PeerInvitation), Error> {
-    let (peer_contents, keypair) = make_peer_contents_and_key_pair(new_peer_info);
-    let peer = rest_client.create_peer(&peer_contents)?;
-    let invitation = PeerInvitation::new(
-        interface,
-        &peer,
-        server_peer,
-        cidr_tree,
-        keypair,
-        server_api_addr,
-    )?;
+) -> Result<(Peer, PeerInvitation), CreatePeerError> {
+    let interface_config = InterfaceConfig::from_interface(config_dir, interface)
+        .map_err(CreatePeerError::InterfaceConfigAccess)?;
+    let rest_client = RestClient::new(&interface_config.server);
 
-    Ok((peer, invitation))
+    let keypair = KeyPair::generate();
+    let peer_contents = new_peer_info.into_peer_contents(&keypair);
+    let peer = rest_client.create_peer(&peer_contents)?;
+
+    let cidr_tree = CidrTree::new(existing_cidrs);
+    let address = &cidr_tree
+        .ip_net_for(peer.ip)
+        .expect("Peer's IpNet address to be valid because the peer was created successfully.");
+    let interface_info = InterfaceInfo::new(interface, &keypair, *address);
+
+    let server_peer = existing_peers.iter().find(|p| p.id == 1).unwrap();
+    let server_info = ServerInfo::new(server_peer, interface_config.server.internal_endpoint);
+
+    Ok((peer, PeerInvitation::new(interface_info, server_info)))
 }

client/src/main.rs

@@ -7,7 +7,7 @@ use indoc::eprintdoc;
 use innernet_client_core::{
     data_store::DataStore,
     interface::{fetch, redeem_invite},
-    peer::create_peer_and_invitation,
+    peer::create_peer,
     rest_client::{RestClient, RestError},
     DEFAULT_CONFIG_DIR, DEFAULT_DATA_DIR,
 };
@@ -558,17 +558,9 @@ fn add_peer(interface: &InterfaceName, opts: &Opts, sub_opts: AddPeerOpts) -> Re
     if let Some((new_peer_info, target_path)) =
         prompts::gather_new_peer_info(&peers, &cidr_tree, &sub_opts)?
     {
-        let server_peer = peers.iter().find(|p| p.id == 1).unwrap();
-        let server_api_addr = &server.internal_endpoint;
         log::info!("Creating peer...");
-        let (peer, invitation) = create_peer_and_invitation(
-            &rest_client,
-            interface,
-            &cidr_tree,
-            server_peer,
-            new_peer_info,
-            server_api_addr,
-        )?;
+        let (peer, invitation) =
+            create_peer(&opts.config_dir, interface, &cidrs, &peers, new_peer_info)?;
 
         invitation.save_new(&target_path)?;
         prompts::print_invitation_info(&peer, &target_path);

server/src/lib.rs

@@ -5,10 +5,11 @@ use dialoguer::Confirm;
 use hyper::{http, server::conn::AddrStream, Body, Request, Response};
 use indoc::printdoc;
 use innernet_shared::{
-    get_local_addrs, interface_config::PeerInvitation, peer, prompts, update_hosts_file, wg,
-    AddCidrOpts, AddPeerOpts, CidrTree, DeleteCidrOpts, EnableDisablePeerOpts, Endpoint, Error,
-    HostsOpts, Interface, IoErrorContext, NetworkOpts, PeerContents, RenameCidrOpts,
-    RenamePeerOpts, INNERNET_PUBKEY_HEADER,
+    get_local_addrs,
+    interface_config::{InterfaceInfo, PeerInvitation, ServerInfo},
+    prompts, update_hosts_file, wg, AddCidrOpts, AddPeerOpts, CidrTree, DeleteCidrOpts,
+    EnableDisablePeerOpts, Endpoint, Error, HostsOpts, Interface, IoErrorContext, NetworkOpts,
+    PeerContents, RenameCidrOpts, RenamePeerOpts, INNERNET_PUBKEY_HEADER,
 };
 use ipnet::IpNet;
 use parking_lot::{Mutex, RwLock};
@@ -27,7 +28,9 @@ use std::{
     time::Duration,
 };
 use subtle::ConstantTimeEq;
-use wireguard_control::{Backend, Device, DeviceUpdate, InterfaceName, Key, PeerConfigBuilder};
+use wireguard_control::{
+    Backend, Device, DeviceUpdate, InterfaceName, Key, KeyPair, PeerConfigBuilder,
+};
 
 mod api;
 mod db;
@@ -189,7 +192,8 @@ pub fn add_peer(
     if let Some((new_peer_info, target_path)) =
         innernet_shared::prompts::gather_new_peer_info(&peers, &cidr_tree, &opts)?
     {
-        let (peer_contents, keypair) = peer::make_peer_contents_and_key_pair(new_peer_info);
+        let keypair = KeyPair::generate();
+        let peer_contents = new_peer_info.into_peer_contents(&keypair);
         let peer = DatabasePeer::create(&conn, peer_contents)?;
         if cfg!(not(test)) && Device::get(interface, network.backend).is_ok() {
             // Update the current WireGuard interface with the new peers.
@@ -201,16 +205,16 @@ pub fn add_peer(
             println!("adding to WireGuard interface: {}", &*peer);
         }
 
-        let server_peer = DatabasePeer::get(&conn, 1)?;
-        let invitation = PeerInvitation::new(
-            interface,
-            &peer,
-            &server_peer,
-            &cidr_tree,
-            keypair,
-            &SocketAddr::new(config.address, config.listen_port),
-        )?;
+        let address = cidr_tree
+            .ip_net_for(peer.ip)
+            .expect("Peer's IpNet address to be valid because the peer was created successfully.");
+        let interface_info = InterfaceInfo::new(interface, &keypair, address);
 
+        let internal_endpoint = SocketAddr::new(config.address, config.listen_port);
+        let server_peer = DatabasePeer::get(&conn, 1)?;
+        let server_info = ServerInfo::new(&server_peer, internal_endpoint);
+
+        let invitation = PeerInvitation::new(interface_info, server_info);
         invitation.save_new(target_path)?;
     } else {
         println!("exited without creating peer.");

shared/src/interface_config.rs

@@ -1,6 +1,4 @@
-use crate::{
-    chmod, ensure_dirs_exist, Cidr, Endpoint, Error, IoErrorContext, Peer, WrappedIoError,
-};
+use crate::{chmod, ensure_dirs_exist, Endpoint, Error, IoErrorContext, Peer, WrappedIoError};
 use indoc::writedoc;
 use ipnet::IpNet;
 use serde::{Deserialize, Serialize};
@@ -41,6 +39,17 @@ pub struct InterfaceInfo {
     pub listen_port: Option<u16>,
 }
 
+impl InterfaceInfo {
+    pub fn new(network_name: &InterfaceName, keypair: &KeyPair, address: IpNet) -> Self {
+        Self {
+            network_name: network_name.to_string(),
+            private_key: keypair.private.to_base64(),
+            address,
+            listen_port: None,
+        }
+    }
+}
+
 #[derive(Clone, Deserialize, Serialize, Debug)]
 #[serde(rename_all = "kebab-case")]
 pub struct ServerInfo {
@@ -54,6 +63,19 @@ pub struct ServerInfo {
     pub internal_endpoint: SocketAddr,
 }
 
+impl ServerInfo {
+    pub fn new(server_peer: &Peer, internal_endpoint: SocketAddr) -> Self {
+        Self {
+            external_endpoint: server_peer
+                .endpoint
+                .clone()
+                .expect("The innernet server should have a WireGuard endpoint"),
+            internal_endpoint,
+            public_key: server_peer.public_key.clone(),
+        }
+    }
+}
+
 impl InterfaceConfig {
     /// Save a new config file, failing if it already exists.
     pub fn save_new(&self, path: impl AsRef<Path>, mode: u32) -> Result<(), WrappedIoError> {
@@ -111,32 +133,8 @@ impl InterfaceConfig {
         Ok(Self::get_path(config_dir, interface))
     }
 
-    fn new(
-        network_name: &InterfaceName,
-        peer: &Peer,
-        server_peer: &Peer,
-        root_cidr: &Cidr,
-        keypair: KeyPair,
-        server_api_addr: &SocketAddr,
-    ) -> Result<InterfaceConfig, Error> {
-        let invitation = InterfaceConfig {
-            interface: InterfaceInfo {
-                network_name: network_name.to_string(),
-                private_key: keypair.private.to_base64(),
-                address: IpNet::new(peer.ip, root_cidr.prefix_len())?,
-                listen_port: None,
-            },
-            server: ServerInfo {
-                external_endpoint: server_peer
-                    .endpoint
-                    .clone()
-                    .expect("The innernet server should have a WireGuard endpoint"),
-                internal_endpoint: *server_api_addr,
-                public_key: server_peer.public_key.clone(),
-            },
-        };
-
-        Ok(invitation)
+    fn new(interface: InterfaceInfo, server: ServerInfo) -> Self {
+        InterfaceConfig { interface, server }
     }
 }
 
@@ -154,24 +152,10 @@ pub struct PeerInvitation {
 }
 
 impl PeerInvitation {
-    pub fn new(
-        network_name: &InterfaceName,
-        peer: &Peer,
-        server_peer: &Peer,
-        root_cidr: &Cidr,
-        keypair: KeyPair,
-        server_api_addr: &SocketAddr,
-    ) -> Result<Self, Error> {
-        let interface_config = InterfaceConfig::new(
-            network_name,
-            peer,
-            server_peer,
-            root_cidr,
-            keypair,
-            server_api_addr,
-        )?;
-
-        Ok(Self { interface_config })
+    pub fn new(interface: InterfaceInfo, server: ServerInfo) -> Self {
+        Self {
+            interface_config: InterfaceConfig::new(interface, server),
+        }
     }
 
     /// Save a new invitation file, failing if it already exists.

shared/src/peer.rs

@@ -10,21 +10,20 @@ pub struct NewPeerInfo {
     pub invite_expires: Timestring,
 }
 
-pub fn make_peer_contents_and_key_pair(info: NewPeerInfo) -> (PeerContents, KeyPair) {
-    let default_keypair = KeyPair::generate();
-    let peer_contents = PeerContents {
-        name: info.name,
-        ip: info.ip,
-        cidr_id: info.cidr_id,
-        public_key: default_keypair.public.to_base64(),
-        endpoint: None,
-        is_admin: info.is_admin,
-        is_disabled: false,
-        is_redeemed: false,
-        persistent_keepalive_interval: Some(PERSISTENT_KEEPALIVE_INTERVAL_SECS),
-        invite_expires: Some(SystemTime::now() + info.invite_expires.into()),
-        candidates: vec![],
-    };
-
-    (peer_contents, default_keypair)
+impl NewPeerInfo {
+    pub fn into_peer_contents(self, keypair: &KeyPair) -> PeerContents {
+        PeerContents {
+            name: self.name,
+            ip: self.ip,
+            cidr_id: self.cidr_id,
+            public_key: keypair.public.to_base64(),
+            endpoint: None,
+            is_admin: self.is_admin,
+            is_disabled: false,
+            is_redeemed: false,
+            persistent_keepalive_interval: Some(PERSISTENT_KEEPALIVE_INTERVAL_SECS),
+            invite_expires: Some(SystemTime::now() + self.invite_expires.into()),
+            candidates: vec![],
+        }
+    }
 }

shared/src/types.rs

@@ -4,7 +4,7 @@ use clap::{
     builder::{PossibleValuesParser, TypedValueParser},
     Args,
 };
-use ipnet::IpNet;
+use ipnet::{IpNet, PrefixLenError};
 use once_cell::sync::Lazy;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
@@ -291,6 +291,10 @@ impl<'a> CidrTree<'a> {
             self.children().flat_map(|child| child.leaves()).collect()
         }
     }
+
+    pub fn ip_net_for(&self, ip: IpAddr) -> Result<IpNet, PrefixLenError> {
+        IpNet::new(ip, self.contents.prefix_len())
+    }
 }
 
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]