From c1ea1bc18ca3031f56125a44a5dbc03d7a5d256f Mon Sep 17 00:00:00 2001 From: Ryo Kawaguchi Date: Thu, 18 Apr 2024 20:50:50 +0900 Subject: [PATCH 1/3] Add a new client / server command to rename CIDR. --- client/src/main.rs | 50 +++++++++++++++++++++++++++++++++++- server/src/api/admin/cidr.rs | 17 ++++++++++++ server/src/db/cidr.rs | 24 ++++++++++++++++- server/src/main.rs | 33 +++++++++++++++++++++++- shared/src/prompts.rs | 48 ++++++++++++++++++++++++++++++++-- shared/src/types.rs | 15 +++++++++++ 6 files changed, 182 insertions(+), 5 deletions(-) diff --git a/client/src/main.rs b/client/src/main.rs index 70b7c093..0d529262 100644 --- a/client/src/main.rs +++ b/client/src/main.rs @@ -12,7 +12,7 @@ use shared::{ AddCidrOpts, AddDeleteAssociationOpts, AddPeerOpts, Association, AssociationContents, Cidr, CidrTree, DeleteCidrOpts, EnableDisablePeerOpts, Endpoint, EndpointContents, InstallOpts, Interface, IoErrorContext, ListenPortOpts, NatOpts, NetworkOpts, OverrideEndpointOpts, Peer, - RedeemContents, RenamePeerOpts, State, WrappedIoError, REDEEM_TRANSITION_WAIT, + RedeemContents, RenameCidrOpts, RenamePeerOpts, State, WrappedIoError, REDEEM_TRANSITION_WAIT, }; use std::{ fmt, io, @@ -193,6 +193,19 @@ enum Command { sub_opts: AddCidrOpts, }, + /// Rename a CIDR + /// + /// By default, you'll be prompted interactively to select a CIDR, but you can + /// also specify all the options in the command, eg: + /// + /// --name 'group' --new-name 'family' + RenameCidr { + interface: Interface, + + #[clap(flatten)] + sub_opts: RenameCidrOpts, + }, + /// Delete a CIDR DeleteCidr { interface: Interface, @@ -724,6 +737,37 @@ fn add_cidr(interface: &InterfaceName, opts: &Opts, sub_opts: AddCidrOpts) -> Re Ok(()) } +fn rename_cidr( + interface: &InterfaceName, + opts: &Opts, + sub_opts: RenameCidrOpts, +) -> Result<(), Error> { + let InterfaceConfig { server, .. } = + InterfaceConfig::from_interface(&opts.config_dir, interface)?; + let api = Api::new(&server); + + log::info!("Fetching CIDRs"); + let cidrs: Vec = api.http("GET", "/admin/cidrs")?; + + if let Some((cidr_request, old_name)) = prompts::rename_cidr(&cidrs, &sub_opts)? { + log::info!("Renaming CIDR..."); + + let id = cidrs + .iter() + .filter(|c| c.name == old_name) + .map(|c| c.id) + .next() + .ok_or_else(|| anyhow!("CIDR not found."))?; + + api.http_form("PUT", &format!("/admin/cidrs/{id}"), cidr_request)?; + log::info!("CIDR renamed."); + } else { + log::info!("Exited without renaming CIDR."); + } + + Ok(()) +} + fn delete_cidr( interface: &InterfaceName, opts: &Opts, @@ -1251,6 +1295,10 @@ fn run(opts: &Opts) -> Result<(), Error> { interface, sub_opts, } => add_cidr(&interface, opts, sub_opts)?, + Command::RenameCidr { + interface, + sub_opts, + } => rename_cidr(&interface, opts, sub_opts)?, Command::DeleteCidr { interface, sub_opts, diff --git a/server/src/api/admin/cidr.rs b/server/src/api/admin/cidr.rs index 57162490..74b6d6d0 100644 --- a/server/src/api/admin/cidr.rs +++ b/server/src/api/admin/cidr.rs @@ -19,6 +19,11 @@ pub async fn routes( let form = form_body(req).await?; handlers::create(form, session).await }, + (&Method::PUT, Some(id)) => { + let id: i64 = id.parse().map_err(|_| ServerError::NotFound)?; + let form = form_body(req).await?; + handlers::update(id, form, session).await + }, (&Method::DELETE, Some(id)) => { let id: i64 = id.parse().map_err(|_| ServerError::NotFound)?; handlers::delete(id, session).await @@ -43,6 +48,18 @@ mod handlers { json_status_response(cidr, StatusCode::CREATED) } + pub async fn update( + id: i64, + form: CidrContents, + session: Session, + ) -> Result, ServerError> { + let conn = session.context.db.lock(); + let cidr = DatabaseCidr::get(&conn, id)?; + DatabaseCidr::from(cidr).update(&conn, form)?; + + status_response(StatusCode::NO_CONTENT) + } + pub async fn list(session: Session) -> Result, ServerError> { let conn = session.context.db.lock(); let cidrs = DatabaseCidr::list(&conn)?; diff --git a/server/src/db/cidr.rs b/server/src/db/cidr.rs index 924f0549..ec1a9354 100644 --- a/server/src/db/cidr.rs +++ b/server/src/db/cidr.rs @@ -2,7 +2,7 @@ use crate::ServerError; use ipnet::IpNet; use rusqlite::{params, Connection}; use shared::{Cidr, CidrContents}; -use std::ops::Deref; +use std::ops::{Deref, DerefMut}; pub static CREATE_TABLE_SQL: &str = "CREATE TABLE cidrs ( id INTEGER PRIMARY KEY, @@ -35,6 +35,12 @@ impl Deref for DatabaseCidr { } } +impl DerefMut for DatabaseCidr { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.inner + } +} + impl DatabaseCidr { pub fn create(conn: &Connection, contents: CidrContents) -> Result { let CidrContents { name, cidr, parent } = &contents; @@ -106,6 +112,22 @@ impl DatabaseCidr { Ok(Cidr { id, contents }) } + /// Update self with new contents, validating them and updating the backend in the process. + pub fn update(&mut self, conn: &Connection, contents: CidrContents) -> Result<(), ServerError> { + let new_contents = CidrContents { + name: contents.name, + ..self.contents.clone() + }; + + conn.execute( + "UPDATE cidrs SET name = ?2 WHERE id = ?1", + params![self.id, &*new_contents.name,], + )?; + + self.contents = new_contents; + Ok(()) + } + pub fn delete(conn: &Connection, id: i64) -> Result<(), ServerError> { conn.execute("DELETE FROM cidrs WHERE id = ?1", params![id])?; Ok(()) diff --git a/server/src/main.rs b/server/src/main.rs index d34949ce..cb82ff62 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -10,7 +10,8 @@ use rusqlite::Connection; use serde::{Deserialize, Serialize}; use shared::{ get_local_addrs, AddCidrOpts, AddPeerOpts, DeleteCidrOpts, EnableDisablePeerOpts, Endpoint, - IoErrorContext, NetworkOpts, PeerContents, RenamePeerOpts, INNERNET_PUBKEY_HEADER, + IoErrorContext, NetworkOpts, PeerContents, RenameCidrOpts, RenamePeerOpts, + INNERNET_PUBKEY_HEADER, }; use std::{ collections::{HashMap, VecDeque}, @@ -126,6 +127,14 @@ enum Command { args: AddCidrOpts, }, + /// Rename an existing CIDR. + RenameCidr { + interface: Interface, + + #[clap(flatten)] + args: RenameCidrOpts, + }, + /// Delete a CIDR. DeleteCidr { interface: Interface, @@ -288,6 +297,7 @@ async fn main() -> Result<(), Box> { enable_or_disable_peer(&interface, &conf, true, opts.network, args)? }, Command::AddCidr { interface, args } => add_cidr(&interface, &conf, args)?, + Command::RenameCidr { interface, args } => rename_cidr(&interface, &conf, args)?, Command::DeleteCidr { interface, args } => delete_cidr(&interface, &conf, args)?, Command::Completions { shell } => { use clap::CommandFactory; @@ -460,6 +470,27 @@ fn add_cidr( Ok(()) } +fn rename_cidr( + interface: &InterfaceName, + conf: &ServerConfig, + opts: RenameCidrOpts, +) -> Result<(), Error> { + let conn = open_database_connection(interface, conf)?; + let cidrs = DatabaseCidr::list(&conn)?; + + if let Some((cidr_request, old_name)) = shared::prompts::rename_cidr(&cidrs, &opts)? { + let db_cidr = DatabaseCidr::list(&conn)? + .into_iter() + .find(|c| c.name == old_name) + .ok_or_else(|| anyhow!("CIDR not found."))?; + db::DatabaseCidr::from(db_cidr).update(&conn, cidr_request)?; + } else { + println!("exited without creating CIDR."); + } + + Ok(()) +} + fn delete_cidr( interface: &InterfaceName, conf: &ServerConfig, diff --git a/shared/src/prompts.rs b/shared/src/prompts.rs index b2b42fea..238249a1 100644 --- a/shared/src/prompts.rs +++ b/shared/src/prompts.rs @@ -2,7 +2,8 @@ use crate::{ interface_config::{InterfaceConfig, InterfaceInfo, ServerInfo}, AddCidrOpts, AddDeleteAssociationOpts, AddPeerOpts, Association, Cidr, CidrContents, CidrTree, DeleteCidrOpts, EnableDisablePeerOpts, Endpoint, Error, Hostname, IpNetExt, ListenPortOpts, - OverrideEndpointOpts, Peer, PeerContents, RenamePeerOpts, PERSISTENT_KEEPALIVE_INTERVAL_SECS, + OverrideEndpointOpts, Peer, PeerContents, RenameCidrOpts, RenamePeerOpts, + PERSISTENT_KEEPALIVE_INTERVAL_SECS, }; use anyhow::anyhow; use colored::*; @@ -111,6 +112,49 @@ pub fn add_cidr(cidrs: &[Cidr], request: &AddCidrOpts) -> Result Result, Error> { + let old_cidr = if let Some(ref name) = args.name { + cidrs + .iter() + .find(|c| &c.name == name) + .ok_or_else(|| anyhow!("CIDR '{}' does not exist", name))? + .clone() + } else { + let (cidr_index, _) = select( + "CIDR to rename", + &cidrs.iter().map(|ep| ep.name.clone()).collect::>(), + )?; + cidrs[cidr_index].clone() + }; + let old_name = old_cidr.name.clone(); + let new_name = if let Some(ref name) = args.new_name { + name.clone() + } else { + input("New Name", Prefill::None)? + }; + + let mut new_cidr = old_cidr; + new_cidr.contents.name = new_name.clone(); + + Ok( + if args.yes + || confirm(&format!( + "Rename CIDR {} to {}?", + old_name.yellow(), + new_name.yellow() + ))? + { + Some((new_cidr.contents, old_name)) + } else { + None + }, + ) +} + /// Bring up a prompt to delete a CIDR. Returns the peer request. pub fn delete_cidr(cidrs: &[Cidr], peers: &[Peer], request: &DeleteCidrOpts) -> Result { let eligible_cidrs: Vec<_> = cidrs @@ -342,7 +386,7 @@ pub fn add_peer( ) } -/// Bring up a prompt to create a new peer. Returns the peer request. +/// Bring up a prompt to rename an existing peer. Returns the peer request. pub fn rename_peer( peers: &[Peer], args: &RenamePeerOpts, diff --git a/shared/src/types.rs b/shared/src/types.rs index f482d92a..cd8f156b 100644 --- a/shared/src/types.rs +++ b/shared/src/types.rs @@ -381,6 +381,21 @@ pub struct AddCidrOpts { pub yes: bool, } +#[derive(Debug, Clone, PartialEq, Eq, Args)] +pub struct RenameCidrOpts { + /// Name of CIDR to rename + #[clap(long)] + pub name: Option, + + /// The new name of the CIDR + #[clap(long)] + pub new_name: Option, + + /// Bypass confirmation + #[clap(long)] + pub yes: bool, +} + #[derive(Debug, Clone, PartialEq, Eq, Args)] pub struct DeleteCidrOpts { /// The CIDR name (eg. 'engineers') From 29a1a15feba6af9b329360d0281ee534e007a35f Mon Sep 17 00:00:00 2001 From: Ryo Kawaguchi Date: Thu, 18 Apr 2024 23:37:43 +0900 Subject: [PATCH 2/3] Add a docker test case --- docker-tests/run-docker-tests.sh | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docker-tests/run-docker-tests.sh b/docker-tests/run-docker-tests.sh index 1eb4167f..a5f0ce78 100755 --- a/docker-tests/run-docker-tests.sh +++ b/docker-tests/run-docker-tests.sh @@ -201,6 +201,30 @@ test_simultaneous_redemption() { cmd docker exec "$PEER2_CONTAINER" ping -c3 10.66.1.1 } +test_rename_cidr() { + info "Renaming CIDR from peer1" + cmd docker exec "$PEER1_CONTAINER" innernet \ + rename-cidr evilcorp \ + --name "robots" \ + --new-name "coolbeans" \ + --yes + sleep 5 + + info "Confirming the CIDR rename from peer1" + cmd docker exec "$PEER1_CONTAINER" innernet list-cidrs evilcorp | grep coolbeans + + info "Renaming CIDR from server" + cmd docker exec "$SERVER_CONTAINER" innernet-server \ + rename-cidr evilcorp \ + --name "coolbeans" \ + --new-name "robots" \ + --yes + sleep 5 + + info "Confirming the CIDR rename from peer1" + cmd docker exec "$PEER1_CONTAINER" innernet list-cidrs evilcorp | grep robots +} + # Run tests (functions prefixed with test_) in alphabetical order. # Optional filter provided by positional arguments is applied. for func in $(declare -F | awk '{print $3}'); do From bfc575b7cd8dfa95ad31754e691b33ef76d41313 Mon Sep 17 00:00:00 2001 From: Ryo Kawaguchi Date: Mon, 22 Apr 2024 21:15:16 +0900 Subject: [PATCH 3/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matěj Laitl Co-authored-by: Jake McGinty --- client/src/main.rs | 7 +++---- server/src/db/cidr.rs | 1 + server/src/main.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client/src/main.rs b/client/src/main.rs index 0d529262..0f7f8e6e 100644 --- a/client/src/main.rs +++ b/client/src/main.rs @@ -754,10 +754,9 @@ fn rename_cidr( let id = cidrs .iter() - .filter(|c| c.name == old_name) - .map(|c| c.id) - .next() - .ok_or_else(|| anyhow!("CIDR not found."))?; + .find(|c| c.name == old_name) + .ok_or_else(|| anyhow!("CIDR not found."))? + .id; api.http_form("PUT", &format!("/admin/cidrs/{id}"), cidr_request)?; log::info!("CIDR renamed."); diff --git a/server/src/db/cidr.rs b/server/src/db/cidr.rs index ec1a9354..8e1c0196 100644 --- a/server/src/db/cidr.rs +++ b/server/src/db/cidr.rs @@ -113,6 +113,7 @@ impl DatabaseCidr { } /// Update self with new contents, validating them and updating the backend in the process. + /// Currently this only supports updating the name and ignores changes to any other field. pub fn update(&mut self, conn: &Connection, contents: CidrContents) -> Result<(), ServerError> { let new_contents = CidrContents { name: contents.name, diff --git a/server/src/main.rs b/server/src/main.rs index cb82ff62..a65ca742 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -485,7 +485,7 @@ fn rename_cidr( .ok_or_else(|| anyhow!("CIDR not found."))?; db::DatabaseCidr::from(db_cidr).update(&conn, cidr_request)?; } else { - println!("exited without creating CIDR."); + println!("exited without renaming CIDR."); } Ok(())