diff options
Diffstat (limited to 'src/cli')
| -rw-r--r-- | src/cli/machine.rs | 97 | ||||
| -rw-r--r-- | src/cli/spec/machine.rs | 6 |
2 files changed, 93 insertions, 10 deletions
diff --git a/src/cli/machine.rs b/src/cli/machine.rs index 9328347e..52828f75 100644 --- a/src/cli/machine.rs +++ b/src/cli/machine.rs @@ -1,12 +1,12 @@ use serde::Serialize; -use crate::client::endpoint::{EndpointCatalog, ProfileId}; +use crate::client::endpoint::{EndpointCatalog, ProfileId, MAX_LABEL_BYTES}; const HELP: &str = "Usage: herdr machine list [--json] herdr machine status [<label-or-id>] [--json] herdr machine reconnect <label-or-id> - herdr machine add <ssh-target> --label <label> [--remote-session <name>] + herdr machine add <ssh-target> [--label <label>] [--remote-session <name>] herdr machine rename <profile-id> --label <label> herdr machine remove <profile-id> herdr machine enable <profile-id> @@ -205,7 +205,7 @@ fn reconnect(args: &[String]) -> std::io::Result<i32> { #[derive(Debug, PartialEq, Eq)] struct AddArgs { target: String, - label: String, + label: Option<String>, session: String, } @@ -246,9 +246,9 @@ fn parse_add_args(args: &[String]) -> Result<AddArgs, String> { } } let target = target.ok_or_else(|| { - "usage: herdr machine add <ssh-target> --label <label> [--remote-session <name>]".to_owned() + "usage: herdr machine add <ssh-target> [--label <label>] [--remote-session <name>]" + .to_owned() })?; - let label = label.ok_or_else(|| "--label is required".to_owned())?; let session = session.unwrap_or_else(|| crate::session::DEFAULT_SESSION_NAME.to_owned()); Ok(AddArgs { target, @@ -257,6 +257,41 @@ fn parse_add_args(args: &[String]) -> Result<AddArgs, String> { }) } +/// Names the machine after the SSH host, plus the session when it is not the default. +fn default_label(target: &str, session: &str) -> String { + let url = target + .strip_prefix("ssh://") + .map(|authority| authority.trim_end_matches('/')); + let authority = url.unwrap_or(target); + let host = authority + .rsplit_once('@') + .map_or(authority, |(_, host)| host); + let host = match (url, host.strip_prefix('[')) { + (Some(_), Some(bracketed)) => bracketed.split_once(']').map_or(host, |(ip, _)| ip), + (Some(_), None) => host.split_once(':').map_or(host, |(host, _)| host), + (None, _) => host, + }; + if session == crate::session::DEFAULT_SESSION_NAME { + host.to_owned() + } else { + format!("{host}/{session}") + } +} + +fn check_default_label(catalog: &EndpointCatalog, label: &str) -> Result<(), String> { + if label.len() > MAX_LABEL_BYTES { + return Err(format!( + "default machine name '{label}' is longer than {MAX_LABEL_BYTES} bytes; pass --label to choose a name" + )); + } + if catalog.ssh.iter().any(|profile| profile.label == label) { + return Err(format!( + "a machine named '{label}' already exists; pass --label to choose another name" + )); + } + Ok(()) +} + fn add(args: &[String]) -> std::io::Result<i32> { let AddArgs { target, @@ -269,7 +304,15 @@ fn add(args: &[String]) -> std::io::Result<i32> { return Ok(2); } }; + let label_is_default = label.is_none(); + let label = label.unwrap_or_else(|| default_label(&target, &session)); let mut catalog = load_catalog()?; + if label_is_default { + if let Err(error) = check_default_label(&catalog, &label) { + eprintln!("error: {error}"); + return Ok(2); + } + } match catalog.add_ssh(label.clone(), &target, session.clone()) { Ok(_) => {} Err(error) => { @@ -291,6 +334,12 @@ fn add(args: &[String]) -> std::io::Result<i32> { "remote prepared, but machine was not saved: {error}" )) })?; + if label_is_default { + if let Err(error) = check_default_label(&catalog, &label) { + eprintln!("error: {error}; machine was not saved"); + return Ok(2); + } + } let id = match catalog.add_ssh(label, &target, &session) { Ok(id) => id, Err(error) => { @@ -457,7 +506,7 @@ mod tests { parse_add_args(&args).unwrap(), AddArgs { target: "workstation.coder".into(), - label: "coder".into(), + label: Some("coder".into()), session: session.into(), }, "{args:?}" @@ -466,11 +515,45 @@ mod tests { } #[test] + fn add_parser_leaves_label_unset_without_flag() { + let parsed = parse_add_args(&["workstation.coder".to_owned()]).unwrap(); + assert_eq!(parsed.label, None); + assert_eq!(parsed.session, "default"); + } + + #[test] + fn default_label_uses_ssh_host_and_non_default_session() { + for (target, session, label) in [ + ("workbox", "default", "workbox"), + ("dev@workbox", "default", "workbox"), + ("workbox", "agents", "workbox/agents"), + ("ssh://workbox", "default", "workbox"), + ("ssh://dev@workbox:2222", "default", "workbox"), + ("ssh://dev@[::1]:2222", "agents", "::1/agents"), + ("ssh://dev@workbox/", "default", "workbox"), + ("ssh://dev@workbox:2222/", "agents", "workbox/agents"), + ] { + assert_eq!(default_label(target, session), label, "{target} {session}"); + } + } + + #[test] + fn default_label_must_be_unique_and_fit() { + let mut catalog = EndpointCatalog::default(); + catalog.add_ssh("workbox", "workbox", "default").unwrap(); + + assert!(check_default_label(&catalog, "workbox/agents").is_ok()); + let duplicate = check_default_label(&catalog, "workbox").unwrap_err(); + assert!(duplicate.contains("--label"), "{duplicate}"); + let long = check_default_label(&catalog, &"h".repeat(MAX_LABEL_BYTES + 1)).unwrap_err(); + assert!(long.contains("--label"), "{long}"); + } + + #[test] fn add_parser_rejects_incomplete_duplicate_and_extra_arguments() { for args in [ vec![], vec!["--label", "coder"], - vec!["workstation.coder"], vec!["workstation.coder", "--label"], vec!["workstation.coder", "--label", "coder", "--remote-session"], vec!["--label", "coder", "--label", "other", "workstation.coder"], diff --git a/src/cli/spec/machine.rs b/src/cli/spec/machine.rs index 82bc4c5d..19a34088 100644 --- a/src/cli/spec/machine.rs +++ b/src/cli/spec/machine.rs @@ -30,9 +30,9 @@ pub(super) fn command() -> Command { .required(true), ) .arg( - option("label", "LABEL") - .required(true) - .help("Set the machine label shown in the sidebar"), + option("label", "LABEL").help( + "Set the machine label shown in the sidebar (defaults to the SSH host, or host/session)", + ), ) .arg( option("remote-session", "NAME") |