cli: fix and refactor

Clap uses new primitive `get_*` functions and deprecated the
`value_of_*` variants which started triggering runtime errors.

Also allow completions subcommand to work without cluster, provider and
domain args, fixing the Nix package.

Author: nrdxp

nix/cli/packages/cli/default.nix

@@ -50,15 +50,14 @@ in
     doCheck = false;
 
     postInstall = ''
-      export BITTE_CLUSTER=b
-      export BITTE_PROVIDER=aws
-      export BITTE_DOMAIN=b.b.b
-
       mkdir -p $out/share/zsh/site-functions
-      $out/bin/bitte comp zsh > $out/share/zsh/site-functions/_bitte
+      $out/bin/bitte completions --shell zsh > $out/share/zsh/site-functions/_bitte
 
       mkdir -p $out/share/bash-completion/completions
-      $out/bin/bitte comp bash > $out/share/bash-completion/completions/bitte
+      $out/bin/bitte completions --shell bash > $out/share/bash-completion/completions/bitte
+
+      mkdir -p $out/share/fish/vendor_completions.d
+      $out/bin/bitte completions --shell fish > $out/share/fish/vendor_completions.d/bitte.fish
     '';
 
     passthru = {

nix/cli/packages/cli/src/cli.rs

@@ -1,11 +1,10 @@
-mod args;
 pub mod opts;
 pub mod subs;
 
 use crate::types::{BitteFind, ClusterHandle};
 use anyhow::{anyhow, Context, Result};
 use clap::{App, ArgMatches, FromArgMatches};
-use clap_complete::{generate, shells};
+use clap_complete::{generate, Generator};
 use deploy::cli as deployCli;
 use deploy::cli::Opts as ExtDeployOpts;
 use log::*;
@@ -27,11 +26,14 @@ pub fn init_log(level: u64) {
 }
 
 pub(crate) async fn ssh(sub: &ArgMatches, cluster: ClusterHandle) -> Result<()> {
-    let mut args = sub.values_of_lossy("args").unwrap_or_default();
-    let job: Vec<String> = sub.values_of_t("job").unwrap_or_default();
-    let delay = Duration::from_secs(sub.value_of_t::<u64>("delay").unwrap_or(0));
+    let mut args: Vec<String> = sub.get_many("args").unwrap_or_default().cloned().collect();
+    let job: Vec<String> = sub.get_many("job").unwrap_or_default().cloned().collect();
+    let delay = Duration::from_secs(*sub.get_one::<u64>("delay").unwrap_or(&0));
 
-    let namespace: String = sub.value_of_t("namespace").unwrap_or_default();
+    let namespace = sub
+        .get_one::<String>("namespace")
+        .unwrap_or(&"default".to_string())
+        .to_owned();
 
     let ip: IpAddr;
 
@@ -76,18 +78,17 @@ pub(crate) async fn ssh(sub: &ArgMatches, cluster: ClusterHandle) -> Result<()>
 
         return Ok(());
     } else if sub.is_present("job") {
-        let (name, group, index) = (&*job[0], &*job[1], &job[2]);
+        let (name, group, index) = (&job[0], &job[1], &job[2]);
 
         let nodes = cluster.nodes;
-        let (node, alloc) = nodes.find_with_job(name, group, index, namespace.as_ref())?;
+        let (node, alloc) = nodes.find_with_job(name, group, index, &namespace.clone())?;
+        ip = node.pub_ip;
         if args.is_empty() {
             args.extend(vec![
                 "-t".into(),
-                format!("cd /var/lib/nomad/alloc/{}; bash", alloc.id),
+                format!("cd /var/lib/nomad/alloc/{} && exec $SHELL", alloc.id),
             ]);
-        }
-
-        ip = node.pub_ip;
+        };
     } else {
         let needle = args.first();
 
@@ -123,8 +124,9 @@ async fn init_ssh(ip: IpAddr, args: Vec<String>, cluster: String) -> Result<()>
     flags.push(user_host);
 
     if !args.is_empty() {
-        flags.append(&mut args.iter().map(|string| string.as_str()).collect());
-    }
+        flags.append(&mut args.iter().map(AsRef::as_ref).collect())
+    };
+
     let ssh_args = flags.into_iter();
 
     let mut cmd = Command::new("ssh");
@@ -290,12 +292,7 @@ async fn info_print(cluster: ClusterHandle, json: bool) -> Result<()> {
     Ok(())
 }
 
-pub(crate) async fn completions(sub: &ArgMatches, mut app: App<'_>) -> Result<()> {
-    match sub.subcommand() {
-        Some(("bash", _)) => generate(shells::Bash, &mut app, "bitte", &mut std::io::stdout()),
-        Some(("zsh", _)) => generate(shells::Zsh, &mut app, "bitte", &mut std::io::stdout()),
-        Some(("fish", _)) => generate(shells::Fish, &mut app, "bitte", &mut std::io::stdout()),
-        _ => (),
-    };
-    Ok(())
+pub(crate) async fn completions<G: Generator>(gen: G, mut app: App<'_>) {
+    let cli = &mut app;
+    generate(gen, cli, cli.get_name().to_string(), &mut std::io::stdout())
 }

nix/cli/packages/cli/src/cli/args.rs

@@ -1,37 +1,9 @@
 use super::subs::SubCommands;
-use crate::types::BitteProvider;
 use clap::Parser;
 
 #[derive(Parser)]
 #[clap(author, version, about)]
 pub struct Bitte {
-    #[clap(arg_enum, long, env = "BITTE_PROVIDER", ignore_case = true)]
-    /// The cluster infrastructure provider
-    provider: BitteProvider,
-    #[clap(long, env = "BITTE_DOMAIN", value_name = "NAME")]
-    /// The public domain of the cluster
-    domain: String,
-    #[clap(long = "cluster", env = "BITTE_CLUSTER", value_name = "TITLE")]
-    /// The unique name of the cluster
-    name: String,
-    #[clap(
-        long,
-        env = "AWS_DEFAULT_REGION",
-        value_name = "REGION",
-        required_if_eq("provider", "AWS")
-    )]
-    /// The default AWS region
-    aws_region: Option<String>,
-    #[clap(
-        long,
-        env = "AWS_ASG_REGIONS",
-        value_name = "REGIONS",
-        required_if_eq("provider", "AWS"),
-        value_delimiter(':'),
-        require_delimiter = true
-    )]
-    /// Regions containing Nomad clients
-    aws_asg_regions: Option<Vec<String>>,
     #[clap(short, long, parse(from_occurrences), global = true, env = "RUST_LOG")]
     /// set log level: 'unset' is 'warn', '-v' is 'info', '-vv' is 'debug', ...
     verbose: i32,

nix/cli/packages/cli/src/cli/opts.rs

@@ -1,5 +1,6 @@
-use anyhow::{Context, Result};
-use clap::{AppSettings, ArgSettings, Parser};
+use crate::types::BitteProvider;
+use clap::{ArgSettings, Parser};
+use clap_complete::Shell;
 use deploy::data as deployData;
 use deploy::settings as deploySettings;
 use uuid::Uuid;
@@ -9,13 +10,14 @@ pub enum SubCommands {
     Info(Info),
     Ssh(Ssh),
     Deploy(Deploy),
-    #[clap(setting = AppSettings::Hidden)]
     Completions(Completions),
 }
 
 #[derive(Parser)]
 /// Show information about instances and auto-scaling groups
 pub struct Info {
+    #[clap(flatten)]
+    globals: Globals,
     #[clap(short, long)]
     /// output as JSON
     json: bool,
@@ -24,6 +26,8 @@ pub struct Info {
 #[derive(Parser, Default)]
 /// Deploy core and client nodes
 pub struct Deploy {
+    #[clap(flatten)]
+    globals: Globals,
     #[clap(long, short = 'l')]
     /// (re-)deploy all client nodes
     pub clients: bool,
@@ -36,10 +40,50 @@ pub struct Deploy {
     /// private & public ip, node name and aws client id
     pub nodes: Vec<String>,
 }
+#[derive(Parser)]
+/// Generate completions for the given shell
+pub struct Completions {
+    // Shell to generate completions for
+    #[clap(long, value_enum, value_name = "SHELL", value_parser = clap::value_parser!(Shell))]
+    shell: Shell,
+}
+
+#[derive(Parser, Default)]
+struct Globals {
+    #[clap(arg_enum, long, env = "BITTE_PROVIDER", ignore_case = true, value_parser = clap::value_parser!(BitteProvider))]
+    /// The cluster infrastructure provider
+    provider: BitteProvider,
+    #[clap(long, env = "BITTE_DOMAIN", value_name = "NAME")]
+    /// The public domain of the cluster
+    domain: String,
+    #[clap(long = "cluster", env = "BITTE_CLUSTER", value_name = "TITLE")]
+    /// The unique name of the cluster
+    name: String,
+    #[clap(
+        long,
+        env = "AWS_DEFAULT_REGION",
+        value_name = "REGION",
+        required_if_eq("provider", "AWS")
+    )]
+    /// The default AWS region
+    aws_region: Option<String>,
+    #[clap(
+        long,
+        env = "AWS_ASG_REGIONS",
+        value_name = "REGIONS",
+        required_if_eq("provider", "AWS"),
+        value_delimiter(':'),
+        require_delimiter = true
+    )]
+    /// Regions containing Nomad clients
+    aws_asg_regions: Option<Vec<String>>,
+}
 
 #[derive(Parser)]
 /// SSH to instances
 pub struct Ssh {
+    #[clap(flatten)]
+    globals: Globals,
     #[clap(
         short,
         long,
@@ -54,7 +98,7 @@ pub struct Ssh {
         long,
         value_name = "TOKEN",
         env = "NOMAD_TOKEN",
-        parse(try_from_str = token_context),
+        value_parser = clap::value_parser!(Uuid),
         setting = ArgSettings::HideEnvValues
     )]
     /// for '-j': The Nomad token used to query node information
@@ -90,22 +134,3 @@ pub struct Ssh {
     /// arguments to ssh
     args: Option<String>,
 }
-
-#[derive(Parser)]
-#[clap(alias = "comp")]
-/// Generate CLI completions
-pub struct Completions {
-    #[clap(subcommand)]
-    shells: Shells,
-}
-
-#[derive(Parser)]
-pub enum Shells {
-    Bash,
-    Zsh,
-    Fish,
-}
-
-fn token_context(string: &str) -> Result<Uuid> {
-    Uuid::parse_str(string).with_context(|| format!("'{}' is not a valid UUID", string))
-}

nix/cli/packages/cli/src/cli/subs.rs

@@ -3,7 +3,8 @@ mod types;
 mod utils;
 
 use anyhow::Result;
-use clap::{App, IntoApp};
+use clap::{App, ArgMatches, IntoApp};
+use clap_complete::Shell;
 use cli::opts::Bitte;
 use types::BitteCluster;
 use uuid::Uuid;
@@ -16,26 +17,32 @@ async fn main() -> Result<()> {
 
     let matches = app.clone().get_matches();
 
-    let run = |init_log: bool, token| {
+    let run = |sub: &ArgMatches, init_log: bool, token| {
         if init_log {
             cli::init_log(matches.occurrences_of("verbose"))
         };
-        BitteCluster::init(matches.clone(), token)
+        BitteCluster::init(sub.clone(), token)
     };
 
     match matches.subcommand() {
-        Some(("deploy", sub)) => cli::deploy(sub, run(false, None)).await?,
-        Some(("info", sub)) => cli::info(sub, run(true, None)).await?,
+        Some(("deploy", sub)) => cli::deploy(sub, run(sub, false, None)).await?,
+        Some(("info", sub)) => cli::info(sub, run(sub, true, None)).await?,
         Some(("ssh", sub)) => {
             let token: Option<Uuid> = if sub.is_present("job") {
-                sub.value_of_t("nomad").ok()
+                sub.get_one::<Uuid>("nomad").copied()
             } else {
                 None
             };
-            cli::ssh(sub, run(true, token)).await?
+            cli::ssh(sub, run(sub, true, token)).await?
         }
-        Some(("completions", sub)) => cli::completions(sub, app).await?,
+        Some(("completions", sub)) => {
+            if let Some(shell) = sub.get_one::<Shell>("shell").copied() {
+                cli::completions(shell, app).await;
+            }
+        }
+
         _ => (),
     }
+
     Ok(())
 }

nix/cli/packages/cli/src/main.rs

@@ -26,8 +26,6 @@ use reqwest::{
     Client,
 };
 
-use error::Error;
-
 use regex::Regex;
 
 #[derive(Serialize, Deserialize)]
@@ -78,6 +76,12 @@ pub enum BitteProvider {
     AWS,
 }
 
+impl Default for BitteProvider {
+    fn default() -> Self {
+        BitteProvider::AWS
+    }
+}
+
 impl Display for BitteProvider {
     fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
         let provider = match *self {
@@ -340,11 +344,17 @@ impl BitteNode {
     ) -> Result<BitteNodes> {
         match provider {
             BitteProvider::AWS => {
-                let regions: HashSet<String> = {
-                    let mut result = args.values_of_t("aws-asg-regions")?;
-                    let default = args.value_of_t("aws-region")?;
-                    result.push(default);
-                    result.into_iter().collect()
+                let regions = {
+                    let mut result: HashSet<String> = args
+                        .get_many("aws-asg-regions")
+                        .unwrap_or_default()
+                        .cloned()
+                        .collect();
+                    let default = args.get_one::<String>("aws-region");
+                    if default.is_some() {
+                        result.insert(default.unwrap().to_owned());
+                    }
+                    result
                 };
 
                 let mut handles = Vec::with_capacity(regions.len());
@@ -365,7 +375,10 @@ impl BitteNode {
                     ]));
                     let response = tokio::spawn(async move {
                         request.send().await.with_context(|| {
-                            format!("failed to connect to ec2.{}.amazonaws.com", region_str)
+                            format!(
+                                "failed to connect to ec2.{}.amazonaws.com",
+                                region_str.to_owned()
+                            )
                         })
                     });
                     handles.push(response);
@@ -511,15 +524,12 @@ type AllocHandle = JoinHandle<Result<NomadAllocs>>;
 
 impl BitteCluster {
     pub async fn new(args: &ArgMatches, token: Option<Uuid>) -> Result<Self> {
-        let name: String = args.value_of_t("name")?;
-        let domain: String = args.value_of_t("domain")?;
-        let provider: BitteProvider = {
-            let provider: String = args.value_of_t("provider")?;
-            match provider.parse() {
-                Ok(v) => Ok(v),
-                Err(_) => Err(Error::Provider { provider }),
-            }?
-        };
+        let name = args.get_one::<String>("name").unwrap().to_owned();
+        let domain = args.get_one::<String>("domain").unwrap().to_owned();
+        let provider: BitteProvider = args
+            .get_one::<BitteProvider>("provider")
+            .unwrap()
+            .to_owned();
 
         let nomad_api_client = match token {
             Some(token) => {