Repository: tonarino/innernet (language: rust)

Issue:
Allow deleting CIDR that has no peers
If you add a CIDR and realize that you made a mistake it would be good to allow removing the entry without having to edit the sqlite database.


Discussion / hints:
Makes sense
For anyone experiencing this, if the CIDR has no children (neither peers nor other CIDRs), you can directly delete it in the SQLite database in the mean time (backing up the database before-hand is always encouraged of course).

We'll eventually add a CLI command to make this easier as well.
> For anyone experiencing this, if the CIDR has no children (neither peers nor other CIDRs), you can directly delete it in the SQLite database in the mean time (backing up the database before-hand is always encouraged of course).

Yeah, that's what I ended up doing. Along with other edits >.>
Yeah, we started with a pretty minimal set of operations, which can of course feel very rigid, but this also gives us flexibility to design the API better without as much cruft after seeing how people are using it :).

Oh yes please.. A delete CIDR would be great :)

Fix patch:
diff --git a/client/src/main.rs b/client/src/main.rs
index f4724c63..b24f08a2 100644
--- a/client/src/main.rs
+++ b/client/src/main.rs
@@ -4,9 +4,9 @@ use hostsfile::HostsBuilder;
 use indoc::printdoc;
 use shared::{
     interface_config::InterfaceConfig, prompts, AddAssociationOpts, AddCidrOpts, AddPeerOpts,
-    Association, AssociationContents, Cidr, CidrTree, EndpointContents, InstallOpts, Interface,
-    IoErrorContext, NetworkOpt, Peer, RedeemContents, State, CLIENT_CONFIG_DIR,
-    REDEEM_TRANSITION_WAIT,
+    Association, AssociationContents, Cidr, CidrTree, DeleteCidrOpts, EndpointContents,
+    InstallOpts, Interface, IoErrorContext, NetworkOpt, Peer, RedeemContents, State,
+    CLIENT_CONFIG_DIR, REDEEM_TRANSITION_WAIT,
 };
 use std::{
     fmt,
@@ -145,6 +145,14 @@ enum Command {
         opts: AddCidrOpts,
     },
 
+    /// Delete a CIDR.
+    DeleteCidr {
+        interface: Interface,
+
+        #[structopt(flatten)]
+        opts: DeleteCidrOpts,
+    },
+
     /// Disable an enabled peer.
     DisablePeer { interface: Interface },
 
@@ -563,6 +571,23 @@ fn add_cidr(interface: &InterfaceName, opts: AddCidrOpts) -> Result<(), Error> {
     Ok(())
 }
 
+fn delete_cidr(interface: &InterfaceName, opts: DeleteCidrOpts) -> Result<(), Error> {
+    let InterfaceConfig { server, .. } = InterfaceConfig::from_interface(interface)?;
+    println!("Fetching eligible CIDRs");
+    let api = Api::new(&server);
+    let cidrs: Vec<Cidr> = api.http("GET", "/admin/cidrs")?;
+    let peers: Vec<Peer> = api.http("GET", "/admin/peers")?;
+
+    let cidr_id = prompts::delete_cidr(&cidrs, &peers, &opts)?;
+
+    println!("Deleting CIDR...");
+    let _ = api.http("DELETE", &*format!("/admin/cidrs/{}", cidr_id))?;
+
+    println!("CIDR deleted.");
+
+    Ok(())
+}
+
 fn add_peer(interface: &InterfaceName, opts: AddPeerOpts) -> Result<(), Error> {
     let InterfaceConfig { server, .. } = InterfaceConfig::from_interface(interface)?;
     let api = Api::new(&server);
@@ -962,6 +987,7 @@ fn run(opt: Opts) -> Result<(), Error> {
         Command::Uninstall { interface } => uninstall(&interface, opt.network)?,
         Command::AddPeer { interface, opts } => add_peer(&interface, opts)?,
         Command::AddCidr { interface, opts } => add_cidr(&interface, opts)?,
+        Command::DeleteCidr { interface, opts } => delete_cidr(&interface, opts)?,
         Command::DisablePeer { interface } => enable_or_disable_peer(&interface, false)?,
         Command::EnablePeer { interface } => enable_or_disable_peer(&interface, true)?,
         Command::AddAssociation { interface, opts } => add_association(&interface, opts)?,
diff --git a/server/src/main.rs b/server/src/main.rs
index 8d5267a6..f9bc15ab 100644
--- a/server/src/main.rs
+++ b/server/src/main.rs
@@ -6,7 +6,7 @@ use ipnetwork::IpNetwork;
 use parking_lot::{Mutex, RwLock};
 use rusqlite::Connection;
 use serde::{Deserialize, Serialize};
-use shared::{AddCidrOpts, AddPeerOpts, IoErrorContext, NetworkOpt, INNERNET_PUBKEY_HEADER};
+use shared::{AddCidrOpts, DeleteCidrOpts, AddPeerOpts, IoErrorContext, NetworkOpt, INNERNET_PUBKEY_HEADER};
 use std::{
     collections::{HashMap, VecDeque},
     convert::TryInto,
@@ -85,6 +85,15 @@ enum Command {
         #[structopt(flatten)]
         args: AddCidrOpts,
     },
+
+    /// Delete a CIDR.
+    DeleteCidr {
+        interface: Interface,
+
+        #[structopt(flatten)]
+        args: DeleteCidrOpts,
+    },
+
 }
 
 pub type Db = Arc<Mutex<Connection>>;
@@ -221,6 +230,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         } => serve(*interface, &conf, routing).await?,
         Command::AddPeer { interface, args } => add_peer(&interface, &conf, args, opt.network)?,
         Command::AddCidr { interface, args } => add_cidr(&interface, &conf, args)?,
+        Command::DeleteCidr { interface, args } => delete_cidr(&interface, &conf, args)?,
     }
 
     Ok(())
@@ -317,6 +327,25 @@ fn add_cidr(
     Ok(())
 }
 
+fn delete_cidr(interface: &InterfaceName, conf: &ServerConfig, args: DeleteCidrOpts) -> Result<(), Error> {
+    println!("Fetching eligible CIDRs");
+    let conn = open_database_connection(interface, conf)?;
+    let cidrs = DatabaseCidr::list(&conn)?;
+    let peers = DatabasePeer::list(&conn)?
+        .into_iter()
+        .map(|dp| dp.inner)
+        .collect::<Vec<_>>();
+
+    let cidr_id = prompts::delete_cidr(&cidrs, &peers, &args)?;
+
+    println!("Deleting CIDR...");
+    let _ = DatabaseCidr::delete(&conn, cidr_id)?;
+
+    println!("CIDR deleted.");
+
+    Ok(())
+}
+
 fn uninstall(
     interface: &InterfaceName,
     conf: &ServerConfig,
diff --git a/shared/src/prompts.rs b/shared/src/prompts.rs
index 05239228..6832a1eb 100644
--- a/shared/src/prompts.rs
+++ b/shared/src/prompts.rs
@@ -1,7 +1,7 @@
 use crate::{
     interface_config::{InterfaceConfig, InterfaceInfo, ServerInfo},
-    AddCidrOpts, AddPeerOpts, Association, Cidr, CidrContents, CidrTree, Endpoint, Error, Peer,
-    PeerContents, PERSISTENT_KEEPALIVE_INTERVAL_SECS,
+    AddCidrOpts, AddPeerOpts, Association, Cidr, CidrContents, CidrTree, DeleteCidrOpts, Endpoint,
+    Error, Peer, PeerContents, PERSISTENT_KEEPALIVE_INTERVAL_SECS,
 };
 use colored::*;
 use dialoguer::{theme::ColorfulTheme, Confirm, Input, Select};
@@ -58,6 +58,42 @@ pub fn add_cidr(cidrs: &[Cidr], request: &AddCidrOpts) -> Result<Option<CidrCont
     )
 }
 
+/// Bring up a prompt to delete a CIDR. Returns the peer request.
+pub fn delete_cidr(cidrs: &[Cidr], peers: &[Peer], request: &DeleteCidrOpts) -> Result<i64, Error> {
+    let eligible_cidrs: Vec<_> = cidrs
+        .iter()
+        .filter(|cidr| {
+            !peers.iter().any(|peer| peer.contents.cidr_id == cidr.id) &&
+            !cidrs.iter().any(
+                |cidr2| matches!(cidr2.contents.parent, Some(parent_id) if parent_id == cidr.id)
+            )
+        })
+        .collect();
+    let cidr = if let Some(ref name) = request.name {
+        cidrs
+            .iter()
+            .find(|cidr| &cidr.name == name)
+            .ok_or_else(|| format!("CIDR {} doesn't exist or isn't eligible for deletion", name))?
+    } else {
+        let cidr_index = Select::with_theme(&*THEME)
+            .with_prompt("Delete CIDR")
+            .items(&eligible_cidrs)
+            .interact()?;
+        &eligible_cidrs[cidr_index]
+    };
+
+    if request.yes
+        || Confirm::with_theme(&*THEME)
+            .with_prompt(&format!("Delete CIDR \"{}\"?", cidr.name))
+            .default(false)
+            .interact()?
+    {
+        Ok(cidr.id)
+    } else {
+        Err("Canceled".into())
+    }
+}
+
 pub fn choose_cidr<'a>(cidrs: &'a [Cidr], text: &'static str) -> Result<&'a Cidr, Error> {
     let eligible_cidrs: Vec<_> = cidrs
         .iter()
diff --git a/shared/src/types.rs b/shared/src/types.rs
index 0f84579c..40e97e49 100644
--- a/shared/src/types.rs
+++ b/shared/src/types.rs
@@ -337,6 +337,17 @@ pub struct AddCidrOpts {
     pub yes: bool,
 }
 
+#[derive(Debug, Clone, PartialEq, StructOpt)]
+pub struct DeleteCidrOpts {
+    /// The CIDR name (eg. "engineers")
+    #[structopt(long)]
+    pub name: Option<String>,
+
+    /// Bypass confirmation
+    #[structopt(long)]
+    pub yes: bool,
+}
+
 #[derive(Debug, Clone, PartialEq, StructOpt)]
 pub struct AddAssociationOpts {
     /// The first cidr to associate


Test patch: