Merge pull request #3719 from mulkieran/bad-unlock-method-parser-error

Make specifying an invalid unlock method a parser error
stratis-storage · Nov 19, 2024 · 6486432 · 6486432
2 parents becc15e + fa3cf4b
commit 6486432
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 18 deletions.
diff --git a/src/bin/stratis-min/stratis-min.rs b/src/bin/stratis-min/stratis-min.rs
@@ -4,16 +4,17 @@
 
 use std::{error::Error, path::PathBuf};
 
-use clap::{Arg, ArgAction, ArgGroup, Command};
+use clap::{builder::PossibleValuesParser, Arg, ArgAction, ArgGroup, Command};
 use serde_json::{json, Map, Value};
+use strum::VariantNames;
 
 use stratisd::{
     engine::{
         EncryptionInfo, KeyDescription, Name, PoolIdentifier, PoolUuid, UnlockMethod,
         CLEVIS_TANG_TRUST_URL,
     },
     jsonrpc::client::{filesystem, key, pool, report},
-    stratis::{StratisError, VERSION},
+    stratis::VERSION,
 };
 
 fn parse_args() -> Command {
@@ -41,7 +42,12 @@ fn parse_args() -> Command {
                 Command::new("start")
                     .arg(Arg::new("id").required(true))
                     .arg(Arg::new("name").long("name").num_args(0))
-                    .arg(Arg::new("unlock_method").long("unlock-method").num_args(1))
+                    .arg(
+                        Arg::new("unlock_method")
+                            .long("unlock-method")
+                            .num_args(1)
+                            .value_parser(PossibleValuesParser::new(UnlockMethod::VARIANTS)),
+                    )
                     .arg(
                         Arg::new("prompt")
                             .long("prompt")
@@ -235,13 +241,9 @@ fn main() -> Result<(), String> {
                             .expect("required"),
                     )?)
                 };
-                let unlock_method =
-                    match args.get_one::<String>("unlock_method").map(|s| s.as_str()) {
-                        Some(um) => Some(UnlockMethod::try_from(um).map_err(|_| {
-                            StratisError::Msg(format!("{um} is an invalid unlock method"))
-                        })?),
-                        None => None,
-                    };
+                let unlock_method = args.get_one::<String>("unlock_method").map(|s| {
+                    UnlockMethod::try_from(s.as_str()).expect("parser ensures valid string value")
+                });
                 let prompt = args.get_flag("prompt");
                 pool::pool_start(id, unlock_method, prompt)?;
                 Ok(())

diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs
@@ -16,7 +16,7 @@ use std::{
 use libudev::EventType;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
-use strum_macros::{self, EnumString, FromRepr};
+use strum_macros::{self, EnumString, FromRepr, VariantNames};
 use uuid::Uuid;
 
 pub use crate::engine::{
@@ -131,7 +131,7 @@ impl Display for StratisUuid {
 }
 
 /// Use Clevis or keyring to unlock LUKS volume.
-#[derive(Serialize, Deserialize, Clone, Copy, Eq, PartialEq, Debug, EnumString)]
+#[derive(Serialize, Deserialize, Clone, Copy, Eq, PartialEq, Debug, EnumString, VariantNames)]
 #[strum(serialize_all = "snake_case")]
 pub enum UnlockMethod {
     Clevis,

diff --git a/tests/stratis_min.rs b/tests/stratis_min.rs
@@ -254,12 +254,7 @@ fn test_stratis_min_pool_start_invalid_unlock_method() {
         .arg("--name")
         .arg("pn")
         .arg("--unlock-method=bogus");
-    cmd.assert()
-        .failure()
-        .code(1)
-        .stderr(predicate::str::contains(
-            "bogus is an invalid unlock method",
-        ));
+    cmd.assert().failure().code(2);
 }
 
 #[test]