Merge pull request #3752 from mulkieran/add-legacy-to-stratisd-tools

Make stratis-legacy-pool a command of stratisd-tools

.githooks/pre-commit

@@ -5,7 +5,6 @@ export PROFILEDIR=debug
 make fmt-ci &&
 	make build &&
 	make stratisd-tools &&
-	make build-test-extras &&
 	make build-min &&
 	make build-no-ipc &&
 	make test &&

.github/workflows/fedora.yml

@@ -45,9 +45,6 @@ jobs:
           - task: PROFILEDIR=debug make -f Makefile build
             toolchain: 1.84.0  # CURRENT DEVELOPMENT RUST TOOLCHAIN
             components: cargo
-          - task: PROFILEDIR=debug make -f Makefile build-test-extras
-            toolchain: 1.84.0  # CURRENT DEVELOPMENT RUST TOOLCHAIN
-            components: cargo
           - task: PROFILEDIR=debug make -f Makefile build-min
             toolchain: 1.84.0  # CURRENT DEVELOPMENT RUST TOOLCHAIN
             components: cargo
@@ -72,9 +69,6 @@ jobs:
           - task: make -f Makefile build
             toolchain: 1.84.0  # CURRENT DEVELOPMENT RUST TOOLCHAIN
             components: cargo
-          - task: make -f Makefile build-test-extras
-            toolchain: 1.84.0  # CURRENT DEVELOPMENT RUST TOOLCHAIN
-            components: cargo
           - task: make -f Makefile build-min
             toolchain: 1.84.0  # CURRENT DEVELOPMENT RUST TOOLCHAIN
             components: cargo

Cargo.toml

@@ -71,10 +71,6 @@ required-features = ["udev_scripts"]
 name = "stratis-utils"
 required-features = ["engine"]
 
-[[bin]]
-name = "stratis-legacy-pool"
-required-features = ["test_extras"]
-
 [dependencies.async-trait]
 version = "0.1.51"
 optional = true
@@ -299,7 +295,6 @@ extras = ["dep:pretty-hex"]
 min = ["dep:termios"]
 systemd_compat = ["dep:bindgen"]
 udev_scripts = ["dep:data-encoding"]
-test_extras = ["engine"]
 
 [lints.rust]
 warnings = { level = "deny" }

Makefile

@@ -51,7 +51,6 @@ MIN_FEATURES = --no-default-features --features engine,min
 NO_IPC_FEATURES = --no-default-features --features engine
 SYSTEMD_FEATURES = --no-default-features --features engine,min,systemd_compat
 EXTRAS_FEATURES =  --no-default-features --features engine,extras,min
-TEST_EXTRAS_FEATURES = --no-default-features --features test_extras
 UDEV_FEATURES = --no-default-features --features udev_scripts
 UTILS_FEATURES = --no-default-features --features engine,systemd_compat
 
@@ -193,12 +192,6 @@ stratisd-tools:
 	cargo ${BUILD} ${RELEASE_FLAG} \
 	--bin=stratisd-tools ${EXTRAS_FEATURES} ${TARGET_ARGS}
 
-## Build the test extras
-build-test-extras:
-	PKG_CONFIG_ALLOW_CROSS=1 \
-	cargo build ${RELEASE_FLAG} \
-	--bin=stratis-legacy-pool ${TEST_EXTRAS_FEATURES} ${TARGET_ARGS}
-
 ## Build the stratis-dumpmetadata program
 ## Build stratis-min for early userspace
 stratis-min:
@@ -421,12 +414,8 @@ clippy-utils:
 clippy-no-ipc:
 	cargo clippy ${CLIPPY_OPTS} ${NO_IPC_FEATURES}
 
-## Run clippy on no-ipc-build
-clippy-test-extras:
-	cargo clippy ${CLIPPY_OPTS} ${TEST_EXTRAS_FEATURES}
-
 ## Run clippy on the current source tree
-clippy: clippy-macros clippy-min clippy-udev-utils clippy-no-ipc clippy-utils clippy-test-extras
+clippy: clippy-macros clippy-min clippy-udev-utils clippy-no-ipc clippy-utils
 	cargo clippy ${CLIPPY_OPTS}
 
 ## Lint Python parts of the source code
@@ -443,7 +432,6 @@ lint:
 	build-all-man
 	build-all-rust
 	build-min
-	build-test-extras
 	build-udev-utils
 	build-stratis-base32-decode
 	build-stratis-str-cmp
@@ -456,7 +444,6 @@ lint:
 	clippy-macros
 	clippy-min
 	clippy-no-ipc
-	clippy-test-extras
 	clippy-udev-utils
 	docs-ci
 	docs-rust

plans/all.fmf

@@ -50,11 +50,10 @@ execute:
 
 /python:
   prepare+:
-    - name: Build and install legacy pool script
+    - name: Make link for legacy pool utility
       how: shell
       script:
-        - PROFILEDIR=debug make build-test-extras
-        - mv target/debug/stratis-legacy-pool /usr/local/bin
+        - ln -s /usr/bin/stratisd-tools /usr/local/bin/stratis-legacy-pool
   discover+:
     filter: "tag:python"
 

src/bin/stratisd-tools.rs

@@ -49,9 +49,7 @@ fn main() {
             .arg(
                 Arg::new("executable")
                     .required(true)
-                    .value_name("EXECUTABLE")
-                    .value_parser(cmds().iter().map(|x| x.name()).collect::<Vec<_>>())
-                    .hide_possible_values(true),
+                    .value_name("EXECUTABLE"),
             )
             .arg_required_else_help(true)
             .after_help(format!(

src/bin/tools/cmds.rs

@@ -4,9 +4,9 @@
 
 use std::path::PathBuf;
 
-use clap::{Arg, ArgAction, Command};
+use clap::{Arg, ArgAction, ArgGroup, Command};
 
-use crate::tools::{check_metadata, dump_metadata};
+use crate::tools::{check_metadata, dump_metadata, legacy_pool};
 
 use stratisd::stratis::VERSION;
 
@@ -147,10 +147,72 @@ impl<'a> ToolCommand<'a> for StratisPrintMetadata {
     }
 }
 
+struct StratisLegacyPool;
+
+impl StratisLegacyPool {
+    fn cmd() -> Command {
+        Command::new("stratis-legacy-pool")
+            .version(VERSION)
+            .about("A program for facilitating testing; not to be used in production. Creates a v1 pool equivalent to a pool that would be created by stratisd 3.7.3.")
+            .arg(Arg::new("pool_name").num_args(1).required(true))
+            .arg(
+                Arg::new("blockdevs")
+                    .action(ArgAction::Append)
+                    .value_parser(clap::value_parser!(PathBuf))
+                    .required(true),
+            )
+            .arg(
+                Arg::new("key_desc")
+                    .long("key-desc")
+                    .num_args(1)
+                    .required(false),
+            )
+            .arg(
+                Arg::new("clevis")
+                    .long("clevis")
+                    .num_args(1)
+                    .required(false)
+                    .value_parser(["nbde", "tang", "tpm2"])
+                    .requires_if("nbde", "tang_args")
+                    .requires_if("tang", "tang_args"),
+            )
+            .arg(
+                Arg::new("tang_url")
+                    .long("tang-url")
+                    .num_args(1)
+                    .required_if_eq("clevis", "nbde")
+                    .required_if_eq("clevis", "tang"),
+            )
+            .arg(Arg::new("thumbprint").long("thumbprint").num_args(1))
+            .arg(Arg::new("trust_url").long("trust-url").num_args(0))
+            .group(
+                ArgGroup::new("tang_args")
+                    .arg("thumbprint")
+                    .arg("trust_url"),
+            )
+    }
+}
+
+impl<'a> ToolCommand<'a> for StratisLegacyPool {
+    fn name(&self) -> &'a str {
+        "stratis-legacy-pool"
+    }
+
+    fn run(&self, command_line_args: Vec<String>) -> Result<(), String> {
+        let matches = StratisLegacyPool::cmd().get_matches_from(command_line_args);
+        legacy_pool::run(&matches).map_err(|err| format!("{err}"))
+    }
+
+    fn show_in_after_help(&self) -> bool {
+        false
+    }
+}
+
 pub fn cmds<'a>() -> Vec<Box<dyn ToolCommand<'a>>> {
     vec![
         Box::new(StratisCheckMetadata),
         Box::new(StratisDumpMetadata),
+        Box::new(StratisLegacyPool),
         Box::new(StratisPrintMetadata),
     ]
 }

src/bin/stratis-legacy-pool.rs → src/bin/tools/legacy_pool.rs

@@ -2,9 +2,12 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-use std::{env, path::PathBuf};
+use std::{
+    io::{stdin, stdout, Write},
+    path::PathBuf,
+};
 
-use clap::{Arg, ArgAction, ArgGroup, Command};
+use clap::ArgMatches;
 use serde_json::{json, Map, Value};
 
 use stratisd::{
@@ -15,65 +18,22 @@ use stratisd::{
     stratis::StratisResult,
 };
 
-fn stratis_legacy_pool_args() -> Command {
-    Command::new("stratis-legacy-pool")
-        .arg(Arg::new("pool_name").num_args(1).required(true))
-        .arg(
-            Arg::new("blockdevs")
-                .action(ArgAction::Append)
-                .required(true),
-        )
-        .arg(
-            Arg::new("key_desc")
-                .long("key-desc")
-                .num_args(1)
-                .required(false),
-        )
-        .arg(
-            Arg::new("clevis")
-                .long("clevis")
-                .num_args(1)
-                .required(false)
-                .value_parser(["nbde", "tang", "tpm2"])
-                .requires_if("nbde", "tang_args")
-                .requires_if("tang", "tang_args"),
-        )
-        .arg(
-            Arg::new("tang_url")
-                .long("tang-url")
-                .num_args(1)
-                .required_if_eq("clevis", "nbde")
-                .required_if_eq("clevis", "tang"),
-        )
-        .arg(Arg::new("thumbprint").long("thumbprint").num_args(1))
-        .arg(Arg::new("trust_url").long("trust-url").num_args(0))
-        .group(
-            ArgGroup::new("tang_args")
-                .arg("thumbprint")
-                .arg("trust_url"),
-        )
-}
-
 type ParseReturn = StratisResult<(
     String,
     Vec<PathBuf>,
     Option<KeyDescription>,
     Option<(String, Value)>,
 )>;
 
-fn parse_args() -> ParseReturn {
-    let args = env::args().collect::<Vec<_>>();
-    let parser = stratis_legacy_pool_args();
-    let matches = parser.get_matches_from(args);
-
+fn parse_args(matches: &ArgMatches) -> ParseReturn {
     let pool_name = matches
         .get_one::<String>("pool_name")
         .expect("required")
         .clone();
     let blockdevs = matches
-        .get_many::<String>("blockdevs")
+        .get_many::<PathBuf>("blockdevs")
         .expect("required")
-        .map(PathBuf::from)
+        .cloned()
         .collect::<Vec<_>>();
     let key_desc = match matches.get_one::<String>("key_desc") {
         Some(kd) => Some(KeyDescription::try_from(kd)?),
@@ -107,10 +67,21 @@ fn parse_args() -> ParseReturn {
     Ok((pool_name, blockdevs, key_desc, clevis_info))
 }
 
-fn main() -> StratisResult<()> {
-    env_logger::init();
+pub fn run(matches: &ArgMatches) -> StratisResult<()> {
+    let (name, devices, key_desc, clevis_info) = parse_args(matches)?;
+
+    println!("This program's purpose is to create v1 pools that can be used for testing. Under no circumstances should such pools be used in production.");
+    print!("Do you want to continue? [Y/n] ");
+    stdout().flush()?;
+
+    let mut answer = String::new();
+    stdin().read_line(&mut answer)?;
+    let answer = answer.trim_end();
+
+    if answer != "y" && answer != "Y" && answer != "yes" && answer != "Yes" {
+        return Ok(());
+    }
 
-    let (name, devices, key_desc, clevis_info) = parse_args()?;
     let unowned = ProcessedPathInfos::try_from(
         devices
             .iter()

src/bin/tools/mod.rs

@@ -5,5 +5,6 @@
 mod check_metadata;
 mod cmds;
 mod dump_metadata;
+mod legacy_pool;
 
 pub use cmds::cmds;

src/engine/mod.rs

@@ -2,11 +2,8 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-#[cfg(feature = "test_extras")]
-pub use self::strat_engine::{ProcessedPathInfos, StratPool};
-
 #[cfg(feature = "extras")]
-pub use self::strat_engine::pool_inspection;
+pub use self::strat_engine::{pool_inspection, ProcessedPathInfos, StratPool};
 
 pub use self::{
     engine::{BlockDev, Engine, Filesystem, KeyActions, Pool, Report},

src/engine/strat_engine/backstore/backstore/v1.rs

@@ -281,7 +281,7 @@ impl Backstore {
     /// be encrypted only with a kernel keyring and without Clevis information.
     ///
     /// WARNING: metadata changing event
-    #[cfg(any(test, feature = "test_extras"))]
+    #[cfg(any(test, feature = "extras"))]
     pub fn initialize(
         pool_name: Name,
         pool_uuid: PoolUuid,

src/engine/strat_engine/backstore/data_tier.rs

@@ -49,7 +49,7 @@ impl DataTier<v1::StratBlockDev> {
     /// Initially 0 segments are allocated.
     ///
     /// WARNING: metadata changing event
-    #[cfg(any(test, feature = "test_extras"))]
+    #[cfg(any(test, feature = "extras"))]
     pub fn new(block_mgr: BlockDevMgr<v1::StratBlockDev>) -> DataTier<v1::StratBlockDev> {
         DataTier {
             block_mgr,

src/engine/strat_engine/mod.rs

@@ -22,11 +22,11 @@ mod types;
 mod udev;
 mod writing;
 
-#[cfg(feature = "test_extras")]
-pub use self::{backstore::ProcessedPathInfos, pool::v1::StratPool};
-
 #[cfg(feature = "extras")]
-pub use self::pool::inspection as pool_inspection;
+pub use self::{
+    backstore::ProcessedPathInfos,
+    pool::{inspection as pool_inspection, v1::StratPool},
+};
 
 pub use self::{
     backstore::integrity_meta_space,

src/engine/strat_engine/pool/v1.rs

@@ -15,7 +15,7 @@ use serde_json::{Map, Value};
 use devicemapper::{Bytes, DmNameBuf, Sectors};
 use stratisd_proc_macros::strat_pool_impl_gen;
 
-#[cfg(any(test, feature = "test_extras"))]
+#[cfg(any(test, feature = "extras"))]
 use crate::engine::{
     strat_engine::{
         backstore::UnownedDevices,
@@ -183,7 +183,7 @@ impl StratPool {
     /// 1. Initialize the block devices specified by paths.
     /// 2. Set up thinpool device to back filesystems.
     /// Precondition: p.is_absolute() is true for all p in paths
-    #[cfg(any(test, feature = "test_extras"))]
+    #[cfg(any(test, feature = "extras"))]
     pub fn initialize(
         name: &str,
         devices: UnownedDevices,

src/engine/strat_engine/thinpool/thinpool.rs

@@ -702,7 +702,7 @@ impl<B> ThinPool<B> {
 
 impl ThinPool<v1::Backstore> {
     /// Make a new thin pool.
-    #[cfg(any(test, feature = "test_extras"))]
+    #[cfg(any(test, feature = "extras"))]
     pub fn new(
         pool_uuid: PoolUuid,
         thin_pool_size: &ThinPoolSizeParams,