From 53d803abf3d8d7fd1d6af6281a6d80c4f5c8470a Mon Sep 17 00:00:00 2001 From: Steffen Eiden Date: Thu, 7 Nov 2024 11:03:41 +0100 Subject: [PATCH] rust: Streamline and cleanup verbosity handling Create one implementation for the verbose option to be used by all tools. While at it, add a quiet option to decrease the verbosity. Signed-off-by: Marc Hartmayer Signed-off-by: Steffen Eiden --- rust/cpacfinfo/src/main.rs | 2 +- rust/pvapconfig/src/main.rs | 2 +- rust/pvattest/src/cli.rs | 69 +++--------------------------- rust/pvattest/src/main.rs | 18 ++++---- rust/pvsecret/src/cli.rs | 9 ++-- rust/pvsecret/src/main.rs | 21 +++------- rust/utils/src/cli.rs | 83 ++++++++++++++++++++++++++++++++++--- rust/utils/src/lib.rs | 30 +++++++++----- rust/utils/src/log.rs | 14 +------ 9 files changed, 125 insertions(+), 123 deletions(-) diff --git a/rust/cpacfinfo/src/main.rs b/rust/cpacfinfo/src/main.rs index 24ab5fce..ff82f60a 100644 --- a/rust/cpacfinfo/src/main.rs +++ b/rust/cpacfinfo/src/main.rs @@ -113,7 +113,7 @@ fn main() -> anyhowRes<()> { /* ---- PRINT VERSION STRING ---- */ if args.version { - print_version!(0, "2024"); + print_version!("2024"); return Ok(()); } diff --git a/rust/pvapconfig/src/main.rs b/rust/pvapconfig/src/main.rs index f480c69e..8654ba02 100644 --- a/rust/pvapconfig/src/main.rs +++ b/rust/pvapconfig/src/main.rs @@ -58,7 +58,7 @@ macro_rules! on_error_print_and_exit { fn main() -> ExitCode { // handle version option if cli::ARGS.version { - print_version!(0, "2023"); + print_version!("2023"); return ExitCode::SUCCESS; } diff --git a/rust/pvattest/src/cli.rs b/rust/pvattest/src/cli.rs index 919bf643..08b894d3 100644 --- a/rust/pvattest/src/cli.rs +++ b/rust/pvattest/src/cli.rs @@ -3,25 +3,19 @@ // Copyright IBM Corp. 2024 use clap::{Args, Parser, Subcommand, ValueEnum, ValueHint}; -use log::warn; -use utils::CertificateOptions; +use utils::{CertificateOptions, DeprecatedVerbosityOptions}; /// create, perform, and verify attestation measurements /// /// Create, perform, and verify attestation measurements for IBM Secure Execution guest systems. #[derive(Parser, Debug)] pub struct CliOptions { - /// Provide more detailed output - #[arg(short='v', long, action = clap::ArgAction::Count)] - verbose: u8, - - /// Deprecated short verbose flag (-V) form the C implementation. - /// - /// If specified a deprecation warning is emitted, - #[arg(short = 'V', hide = true, action = clap::ArgAction::Count)] - verbose_deprecated: u8, + #[clap(flatten)] + pub verbosity: DeprecatedVerbosityOptions, /// Print version information and exit + // Implemented for the help message only. Actual parsing happens in the + // version command. #[arg(long)] pub version: bool, @@ -29,29 +23,6 @@ pub struct CliOptions { pub cmd: Command, } -impl CliOptions { - pub fn verbosity(&self) -> u8 { - let verbose_deprecated = self.verbose_deprecated - + match &self.cmd { - Command::Create(cmd) => cmd.verbose_deprecated, - Command::Perform(cmd) => cmd.verbose_deprecated, - Command::Verify(cmd) => cmd.verbose_deprecated, - Command::Version => 0, - }; - if verbose_deprecated > 0 { - warn!("WARNING: Use of deprecated flag '-V'. Use '-v' or '--verbose' instead.") - } - verbose_deprecated - + self.verbose - + match &self.cmd { - Command::Create(cmd) => cmd.verbose, - Command::Perform(cmd) => cmd.verbose, - Command::Verify(cmd) => cmd.verbose, - Command::Version => 0, - } - } -} - #[derive(Subcommand, Debug)] pub enum Command { /// Create an attestation measurement request @@ -107,16 +78,6 @@ pub struct CreateAttOpt { /// Optional. #[arg(long, value_name = "FLAGS")] pub add_data: Vec, - - /// Provide more detailed output. - #[arg(short='v', long, action = clap::ArgAction::Count)] - verbose: u8, - - /// Deprecated short verbose flag (-V) form the C implementation. - /// - /// If specified a deprecation warning is emitted, - #[arg(short = 'V', hide = true, action = clap::ArgAction::Count)] - verbose_deprecated: u8, } #[derive(Debug, ValueEnum, Clone, Copy)] @@ -158,16 +119,6 @@ pub struct PerformAttOpt { /// May be any arbitrary data, as long as it is less or equal to 256 bytes #[arg(short, long, value_name = "File", value_hint = ValueHint::FilePath,)] pub user_data: Option, - - /// Provide more detailed output. - #[arg(short='v', long, action = clap::ArgAction::Count)] - verbose: u8, - - /// Deprecated short verbose flag (-V) form the C implementation. - /// - /// If specified a deprecation warning is emitted, - #[arg(short = 'V', hide = true, action = clap::ArgAction::Count)] - verbose_deprecated: u8, } #[cfg(target_arch = "s390x")] @@ -237,16 +188,6 @@ pub struct VerifyOpt { /// Emits a warning if the response contains no user-data #[arg(long, short ,value_name = "FILE", value_hint = ValueHint::FilePath,)] pub user_data: Option, - - /// Provide more detailed output. - #[arg(short='v', long, action = clap::ArgAction::Count)] - verbose: u8, - - /// Deprecated short verbose flag (-V) form the C implementation. - /// - /// If specified a deprecation warning is emitted, - #[arg(short = 'V', hide = true, action = clap::ArgAction::Count)] - verbose_deprecated: u8, } #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum, Debug, Default)] diff --git a/rust/pvattest/src/main.rs b/rust/pvattest/src/main.rs index 435ddd12..6c3d8fd3 100644 --- a/rust/pvattest/src/main.rs +++ b/rust/pvattest/src/main.rs @@ -7,12 +7,11 @@ mod cmd; mod exchange; use clap::{CommandFactory, Parser}; -use cli::CliOptions; +use cli::{CliOptions, Command}; use log::trace; use std::process::ExitCode; use utils::{print_cli_error, print_error, print_version, PvLogger}; -use crate::cli::Command; use crate::cmd::*; static LOGGER: PvLogger = PvLogger; @@ -20,11 +19,6 @@ const FEATURES: &[&[&str]] = &[cmd::CMD_FN, cmd::UV_CMD_FN]; const EXIT_CODE_ATTESTATION_FAIL: u8 = 2; const EXIT_CODE_LOGGER_FAIL: u8 = 3; -fn print_version(verbosity: u8) -> anyhow::Result { - print_version!(verbosity, "2024", FEATURES.concat()); - Ok(ExitCode::SUCCESS) -} - fn main() -> ExitCode { let cli: CliOptions = match CliOptions::try_parse() { Ok(cli) => cli, @@ -32,7 +26,8 @@ fn main() -> ExitCode { }; // set up logger/stderr - if let Err(e) = LOGGER.start(cli.verbosity()) { + let log_level = cli.verbosity.to_level_filter(); + if let Err(e) = LOGGER.start(log_level) { // should(TM) never happen eprintln!("Logger error: {e:?}"); return EXIT_CODE_LOGGER_FAIL.into(); @@ -45,10 +40,13 @@ fn main() -> ExitCode { Command::Create(opt) => create(opt), Command::Perform(opt) => perform(opt), Command::Verify(opt) => verify(opt), - Command::Version => print_version(cli.verbosity()), + Command::Version => { + print_version!("2024", log_level; FEATURES.concat()); + Ok(ExitCode::SUCCESS) + } }; match res { Ok(c) => c, - Err(e) => print_error(&e, cli.verbosity()), + Err(e) => print_error(&e, log_level), } } diff --git a/rust/pvsecret/src/cli.rs b/rust/pvsecret/src/cli.rs index 43c9cf25..7387a754 100644 --- a/rust/pvsecret/src/cli.rs +++ b/rust/pvsecret/src/cli.rs @@ -3,18 +3,19 @@ // Copyright IBM Corp. 2023 use clap::{ArgGroup, Args, CommandFactory, Parser, Subcommand, ValueEnum, ValueHint}; -use utils::{CertificateOptions, STDOUT}; +use utils::{CertificateOptions, DeprecatedVerbosityOptions, STDOUT}; /// Manage secrets for IBM Secure Execution guests. /// /// Use to create and send add-secret requests, list the added secrets and lock the Secret Store. #[derive(Parser, Debug)] pub struct CliOptions { - /// Provide more detailed output. - #[arg(short='v', long, action = clap::ArgAction::Count, short_alias('V'))] - pub verbose: u8, + #[clap(flatten)] + pub verbosity: DeprecatedVerbosityOptions, /// Print version information and exit. + // Implemented for the help message only. Actual parsing happens in the + // version command. #[arg(long)] pub version: bool, diff --git a/rust/pvsecret/src/main.rs b/rust/pvsecret/src/main.rs index 4c1c6198..71f8dab1 100644 --- a/rust/pvsecret/src/main.rs +++ b/rust/pvsecret/src/main.rs @@ -6,22 +6,15 @@ mod cli; mod cmd; use clap::{CommandFactory, Parser}; -use cli::{CliOptions, Command}; +use cli::{validate_cli, CliOptions, Command}; use log::trace; use std::process::ExitCode; use utils::{print_cli_error, print_error, print_version, PvLogger}; -use crate::cli::validate_cli; - static LOGGER: PvLogger = PvLogger; static EXIT_LOGGER: u8 = 3; const FEATURES: &[&[&str]] = &[cmd::CMD_FN, cmd::UV_CMD_FN]; -fn print_version(verbosity: u8) -> anyhow::Result<()> { - print_version!(verbosity, "2024", FEATURES.concat()); - Ok(()) -} - fn main() -> ExitCode { let cli: CliOptions = match CliOptions::try_parse() { Ok(cli) => match validate_cli(&cli) { @@ -32,7 +25,8 @@ fn main() -> ExitCode { }; // set up logger/std(out,err) - if let Err(e) = LOGGER.start(cli.verbose) { + let log_level = cli.verbosity.to_level_filter(); + if let Err(e) = LOGGER.start(log_level) { // should(TM) never happen eprintln!("Logger error: {e:?}"); return EXIT_LOGGER.into(); @@ -42,23 +36,18 @@ fn main() -> ExitCode { trace!("Trace verbosity, may leak secrets to command-line"); trace!("Options {cli:?}"); - if cli.version { - let _ = print_version(cli.verbose); - return ExitCode::SUCCESS; - } - // perform the command selected by the user let res = match &cli.cmd { Command::Add(opt) => cmd::add(opt), Command::List(opt) => cmd::list(opt), Command::Lock => cmd::lock(), Command::Create(opt) => cmd::create(opt), - Command::Version => print_version(cli.verbose), + Command::Version => Ok(print_version!("2024", log_level; FEATURES.concat())), Command::Verify(opt) => cmd::verify(opt), }; match res { Ok(_) => ExitCode::SUCCESS, - Err(e) => print_error(&e, cli.verbose), + Err(e) => print_error(&e, log_level), } } diff --git a/rust/utils/src/cli.rs b/rust/utils/src/cli.rs index 77260d33..f64f7af3 100644 --- a/rust/utils/src/cli.rs +++ b/rust/utils/src/cli.rs @@ -2,8 +2,8 @@ // // Copyright IBM Corp. 2023, 2024 -use clap::{ArgGroup, Args, Command, ValueHint}; -use log::{info, warn}; +use clap::{ArgAction, ArgGroup, Args, Command, ValueHint}; +use log::{info, warn, LevelFilter}; use pv::misc::read_file; use pv::{ misc::{create_file, open_file, read_certs}, @@ -174,7 +174,7 @@ pub fn get_reader_from_cli_file_arg>(path: P) -> Result ExitCode { let ret = if e.use_stderr() { ExitCode::FAILURE @@ -187,12 +187,12 @@ pub fn print_cli_error(e: clap::Error, mut cmd: Command) -> ExitCode { } /// Print an error to stderr -pub fn print_error(e: &E, verbosity: u8) -> ExitCode +pub fn print_error(e: &E, verbosity: LevelFilter) -> ExitCode where // Error trait is not required, but here to limit the usage to errors E: AsRef + std::fmt::Debug + std::fmt::Display, { - if verbosity > 0 { + if verbosity > LevelFilter::Warn { // Debug formatter also prints the whole error stack // So only print it when on verbose eprintln!("error: {e:?}") @@ -202,6 +202,79 @@ where ExitCode::FAILURE } +#[derive(Args, Debug, Clone, Default)] +pub struct VerbosityOptions { + #[arg( + long, + short = 'v', + action = ArgAction::Count, + global = true, + )] + /// Provide more detailed output. + verbose: u8, + + #[arg( + long, + short = 'q', + action = ArgAction::Count, + global = true, + conflicts_with = "verbose", + )] + /// Provide less output. + quiet: u8, +} + +const fn to_level_filter(v: u8) -> LevelFilter { + match v { + 0 => LevelFilter::Off, + 1 => LevelFilter::Error, + 2 => LevelFilter::Warn, + 3 => LevelFilter::Info, + 4 => LevelFilter::Debug, + 5.. => LevelFilter::Trace, + } +} + +impl VerbosityOptions { + fn verbosity(&self) -> u8 { + (LevelFilter::Warn as i16 + self.verbose as i16 - self.quiet as i16) + .clamp(u8::MIN.into(), u8::MAX.into()) as u8 + } + + pub fn to_level_filter(&self) -> LevelFilter { + to_level_filter(self.verbosity()) + } +} + +#[derive(Args, Debug, Clone, Default)] +pub struct DeprecatedVerbosityOptions { + #[clap(flatten)] + verbosity: VerbosityOptions, + + #[arg( + short = 'V', + action = ArgAction::Count, + global = true, + hide = true, + )] + /// Provide more detailed output. + deprecated_verbose: u8, +} + +impl DeprecatedVerbosityOptions { + pub fn to_level_filter(&self) -> LevelFilter { + if self.deprecated_verbose > 0 { + // Use eprintln as the logger is most likely not yet initialized. + eprintln!("WARNING: Use of deprecated flag '-V'. Use '-v' or '--verbose' instead.") + } + to_level_filter( + self.verbosity + .verbosity() + .saturating_add(self.deprecated_verbose), + ) + } +} + #[cfg(test)] mod test { use clap::Parser; diff --git a/rust/utils/src/lib.rs b/rust/utils/src/lib.rs index 6c39314d..af4cbc4b 100644 --- a/rust/utils/src/lib.rs +++ b/rust/utils/src/lib.rs @@ -8,13 +8,14 @@ mod hexslice; mod log; mod tmpfile; -pub use crate::cli::CertificateOptions; pub use crate::cli::{get_reader_from_cli_file_arg, get_writer_from_cli_file_arg}; pub use crate::cli::{print_cli_error, print_error}; +pub use crate::cli::{CertificateOptions, DeprecatedVerbosityOptions, VerbosityOptions}; pub use crate::cli::{STDIN, STDOUT}; pub use crate::hexslice::HexSlice; pub use crate::log::PvLogger; pub use crate::tmpfile::TemporaryDirectory; +pub use ::log::LevelFilter; /// Get the s390-tools release string /// @@ -41,23 +42,18 @@ macro_rules! release_string { } #[macro_export] -/// Print the version to stdout -/// -/// verbosity: integer if >0 more and more details printed -/// feat: (optional) list of features -/// rel_str: a string containig the release name -macro_rules! print_version { - ($verbosity: expr, $year: expr $( ,$feat: expr)?) => {{ +macro_rules! __print_version { + ($year: expr, $verbosity: expr $( ,$feat: expr)?) => {{ println!( "{} version {}\nCopyright IBM Corp. {}", env!("CARGO_PKG_NAME"), $crate::release_string!(), $year, ); - if $verbosity > 0 { + if $verbosity > $crate::LevelFilter::Warn { $($feat.iter().for_each(|f| print!("{f} ")); println!("(compiled)");)? } - if $verbosity > 1 { + if $verbosity > $crate::LevelFilter::Info { println!( "\n{}-crate {}", env!("CARGO_PKG_NAME"), @@ -67,6 +63,20 @@ macro_rules! print_version { }}; } +#[macro_export] +/// Print the version to stdout +/// +/// `verbosity` (optional): `LogLevel` +/// `feat` (optional): list of features +/// `rel_str`: a string containig the release name +macro_rules! print_version { + ($year: expr $( ;$feat: expr)?) => { + $crate::__print_version!($year, $crate::LevelFilter::Warn $(, $feat)*) + }; + ($year: expr, $verbosity: expr $( ;$feat: expr)?) => { + $crate::__print_version!($year, $verbosity $(, $feat)*) + }; +} /// Asserts a constant expression evaluates to `true`. /// /// If the expression is not evaluated to `true` the compilation will fail. diff --git a/rust/utils/src/log.rs b/rust/utils/src/log.rs index 70204e8c..fa0cf993 100644 --- a/rust/utils/src/log.rs +++ b/rust/utils/src/log.rs @@ -9,24 +9,14 @@ use log::{self, Level, LevelFilter, Log, Metadata, Record}; #[derive(Clone, Default, Debug)] pub struct PvLogger; -fn to_level(verbosity: u8) -> LevelFilter { - match verbosity { - // Error and Warn on by default - 0 => LevelFilter::Warn, - 1 => LevelFilter::Info, - 2 => LevelFilter::Debug, - _ => LevelFilter::Trace, - } -} - impl PvLogger { /// Set self as the logger for this application. /// /// # Errors /// /// An error is returned if a logger has already been set. - pub fn start(&'static self, verbosity: u8) -> Result<(), log::SetLoggerError> { - log::set_logger(self).map(|()| log::set_max_level(to_level(verbosity))) + pub fn start(&'static self, filter: LevelFilter) -> Result<(), log::SetLoggerError> { + log::set_logger(self).map(|()| log::set_max_level(filter)) } }