Skip to content

Commit 6a7e291

Browse files
cbgbtcbgbt
authored
Merge pull request #25 from cbgbt/we-clap-for-argh
refactor: migrate from clap to argh
2 parents efd1ebe + 07b24bc commit 6a7e291

File tree

7 files changed

+145
-82
lines changed

7 files changed

+145
-82
lines changed

‎bottlerocket-settings-sdk/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
[package]
22
name = "bottlerocket-settings-sdk"
3-
version = "0.1.0-alpha.0"
3+
version = "0.1.0-alpha.1"
44
license = "Apache-2.0 OR MIT"
55
edition = "2021"
66
repository = "https://github.com/bottlerocket-os/bottlerocket-settings-sdk"
77
readme = "../README.md"
88

99
[dependencies]
10+
argh = "0.1"
1011
bottlerocket-template-helper = { path = "../bottlerocket-template-helper", version = "0.1.0-alpha" }
11-
clap = { version = "4", features = ["derive"] }
1212
serde = { version = "1", features = ["derive"] }
1313
serde_json = "1.0"
1414
snafu = "0.7"

‎bottlerocket-settings-sdk/src/cli/mod.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,20 @@
55
#![allow(missing_docs)]
66
pub mod proto1;
77

8+
use argh::FromArgs;
89
use std::fmt::Display;
910

10-
use clap::{Parser, Subcommand};
11-
12-
#[derive(Parser, Debug)]
13-
#[command(author, version, about, long_about = None)]
14-
#[command(propagate_version = true)]
11+
/// Top-level CLI command.
12+
#[derive(FromArgs, Debug)]
1513
pub struct Cli {
16-
/// The Bottlerocket Settings CLI protocol to use
17-
#[command(subcommand)]
14+
/// the Bottlerocket Settings CLI protocol to use
15+
#[argh(subcommand)]
1816
pub protocol: Protocol,
1917
}
2018

21-
#[derive(Subcommand, Debug)]
19+
/// The CLI protocol to use when invoking the extension.
20+
#[derive(FromArgs, Debug)]
21+
#[argh(subcommand)]
2222
pub enum Protocol {
2323
#[cfg(feature = "proto1")]
2424
/// Settings extension protocol 1

‎bottlerocket-settings-sdk/src/cli/proto1.rs

Lines changed: 70 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
//! Bottlerocket Settings Extension CLI proto1 definition.
22
#![allow(missing_docs)]
3-
use clap::{Args, Subcommand};
3+
use argh::FromArgs;
44

5-
#[derive(Args, Debug)]
5+
/// Use Settings Extension CLI protocol proto1.
6+
#[derive(FromArgs, Debug)]
7+
#[argh(subcommand, name = "proto1")]
68
pub struct Protocol1 {
7-
#[command(subcommand)]
9+
/// the command to invoke against the settings extension
10+
#[argh(subcommand)]
811
pub command: Proto1Command,
912
}
1013

11-
#[derive(Subcommand, Debug)]
14+
/// The command to invoke against the settings extension.
15+
#[derive(FromArgs, Debug)]
16+
#[argh(subcommand)]
1217
pub enum Proto1Command {
1318
/// Modify values owned by this setting
1419
Set(SetCommand),
@@ -22,92 +27,109 @@ pub enum Proto1Command {
2227
/// Migrate this setting from one given version to another
2328
Migrate(MigrateCommand),
2429

30+
/// Migrate this setting from one given version to all other known versions
31+
FloodMigrate(FloodMigrateCommand),
32+
2533
/// Execute a helper. Typically this is used to render config templates
2634
Helper(TemplateHelperCommand),
2735
}
2836

2937
impl Proto1Command {}
3038

31-
#[derive(Args, Debug)]
39+
/// Validates that a new setting value can be persisted to the Bottlerocket datastore.
40+
#[derive(FromArgs, Debug)]
41+
#[argh(subcommand, name = "set")]
3242
pub struct SetCommand {
33-
/// The version of the setting which should be used
34-
#[arg(long)]
43+
/// the version of the setting which should be used
44+
#[argh(option)]
3545
pub setting_version: String,
3646

37-
/// The requested value to be set for the incoming setting
38-
#[arg(long, value_parser = parse_json)]
47+
/// the requested value to be set for the incoming setting
48+
#[argh(option)]
3949
pub value: serde_json::Value,
4050

41-
/// The current value of this settings tree
42-
#[arg(long, value_parser = parse_json)]
51+
/// the current value of this settings tree
52+
#[argh(option)]
4353
pub current_value: Option<serde_json::Value>,
4454
}
4555

46-
#[derive(Args, Debug)]
56+
/// Dynamically generates a value for this setting given, possibly from other settings.
57+
#[derive(FromArgs, Debug)]
58+
#[argh(subcommand, name = "generate")]
4759
pub struct GenerateCommand {
48-
/// The version of the setting which should be used
49-
#[arg(long)]
60+
/// the version of the setting which should be used
61+
#[argh(option)]
5062
pub setting_version: String,
5163

52-
/// A json value containing any partially generated data for this setting
53-
#[arg(long, value_parser = parse_json)]
64+
/// a json value containing any partially generated data for this setting
65+
#[argh(option)]
5466
pub existing_partial: Option<serde_json::Value>,
5567

56-
/// A json value containing any requested settings partials needed to generate this one
57-
#[arg(long, value_parser = parse_json)]
68+
/// a json value containing any requested settings partials needed to generate this one
69+
#[argh(option)]
5870
pub required_settings: Option<serde_json::Value>,
5971
}
6072

61-
#[derive(Args, Debug)]
73+
/// Validates an incoming setting, possibly cross-validated with other settings.
74+
#[derive(FromArgs, Debug)]
75+
#[argh(subcommand, name = "validate")]
6276
pub struct ValidateCommand {
63-
/// The version of the setting which should be used
64-
#[arg(long)]
77+
/// the version of the setting which should be used
78+
#[argh(option)]
6579
pub setting_version: String,
6680

67-
/// A json value containing any partially generated data for this setting
68-
#[arg(long, value_parser = parse_json)]
81+
/// a json value containing any partially generated data for this setting
82+
#[argh(option)]
6983
pub value: serde_json::Value,
7084

71-
/// A json value containing any requested settings partials needed to generate this one
72-
#[arg(long, value_parser = parse_json)]
85+
/// a json value containing any requested settings partials needed to generate this one
86+
#[argh(option)]
7387
pub required_settings: Option<serde_json::Value>,
7488
}
7589

76-
#[derive(Args, Debug)]
90+
/// Migrates a setting value from one version to another.
91+
#[derive(FromArgs, Debug)]
92+
#[argh(subcommand, name = "migrate")]
7793
pub struct MigrateCommand {
78-
/// A json value containing the current value of the setting
79-
#[arg(long, value_parser = parse_json)]
94+
/// a json value containing the current value of the setting
95+
#[argh(option)]
8096
pub value: serde_json::Value,
8197

82-
/// The version of the settings data being migrated
83-
#[arg(long)]
98+
/// the version of the settings data being migrated
99+
#[argh(option)]
84100
pub from_version: String,
85101

86-
/// The desired resulting version for the settings data
87-
#[arg(long, group = "migration-type")]
88-
pub target_version: Option<String>,
102+
/// the desired resulting version for the settings data
103+
#[argh(option)]
104+
pub target_version: String,
105+
}
106+
107+
/// Migrates a setting value from one version to all other known versions.
108+
#[derive(FromArgs, Debug)]
109+
#[argh(subcommand, name = "flood-migrate")]
110+
pub struct FloodMigrateCommand {
111+
/// a json value containing the current value of the setting
112+
#[argh(option)]
113+
pub value: serde_json::Value,
89114

90-
/// Triggers a batch migration to all known setting versions
91-
#[arg(long, group = "migration-type")]
92-
pub flood: bool,
115+
/// the version of the settings data being migrated
116+
#[argh(option)]
117+
pub from_version: String,
93118
}
94119

95-
#[derive(Args, Debug)]
120+
/// Executes a template helper to assist in rendering values to a configuration file.
121+
#[derive(FromArgs, Debug)]
122+
#[argh(subcommand, name = "helper")]
96123
pub struct TemplateHelperCommand {
97-
/// The version of the setting which should be used
98-
#[arg(long)]
124+
/// the version of the setting which should be used
125+
#[argh(option)]
99126
pub setting_version: String,
100127

101-
/// The name of the helper to call
102-
#[arg(long)]
128+
/// the name of the helper to call
129+
#[argh(option)]
103130
pub helper_name: String,
104131

105-
/// The arguments for the given helper
106-
#[arg(long, value_parser = parse_json)]
132+
/// the arguments for the given helper
133+
#[argh(option)]
107134
pub arg: Vec<serde_json::Value>,
108135
}
109-
110-
/// Helper for `clap` to parse JSON values.
111-
fn parse_json(arg: &str) -> Result<serde_json::Value, serde_json::Error> {
112-
serde_json::from_str(arg)
113-
}

‎bottlerocket-settings-sdk/src/extension/mod.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
use crate::cli;
44
use crate::migrate::{Migrator, ModelStore};
55
use crate::model::erased::AsTypeErasedModel;
6-
use clap::Parser;
7-
use snafu::{ensure, ResultExt};
6+
use argh::FromArgs;
7+
use snafu::{ensure, OptionExt, ResultExt};
88
use std::collections::{HashMap, HashSet};
99
use std::ffi::OsString;
1010
use std::process::ExitCode;
@@ -103,7 +103,7 @@ where
103103
/// Users of this method should not separately write to `stdout`, as this could break adherence
104104
/// to the settings extension CLI protocol.
105105
pub fn run(self) -> ExitCode {
106-
let args = cli::Cli::parse();
106+
let args: cli::Cli = argh::from_env();
107107
info!(extension = ?self, protocol = ?args.protocol, "Starting settings extensions");
108108
debug!(?args, "CLI arguments");
109109

@@ -122,7 +122,21 @@ where
122122
I: IntoIterator<Item = T>,
123123
T: Into<OsString> + Clone,
124124
{
125-
let args = cli::Cli::try_parse_from(iter).context(error::ParseCLIArgsSnafu)?;
125+
let all_inputs: Vec<String> = iter
126+
.into_iter()
127+
.map(|s| s.into().to_string_lossy().into_owned())
128+
.collect();
129+
130+
let mut input_iter = all_inputs.iter().map(AsRef::as_ref);
131+
let command_name = [input_iter.next().context(error::ParseCLICommandSnafu)?];
132+
let args: Vec<&str> = input_iter.collect();
133+
134+
let args = cli::Cli::from_args(&command_name, &args).map_err(|e| {
135+
error::SettingsExtensionError::ParseCLIArgs {
136+
parser_output: e.output,
137+
}
138+
})?;
139+
126140
info!(cli_protocol = %args.protocol, "Starting settings extensions.");
127141

128142
match args.protocol {
@@ -227,8 +241,11 @@ pub mod error {
227241
#[snafu(display("Requested model version '{}' not found", setting_version))]
228242
NoSuchModel { setting_version: String },
229243

230-
#[snafu(display("Failed to parse CLI arguments: {}", source))]
231-
ParseCLIArgs { source: clap::Error },
244+
#[snafu(display("Failed to parse CLI arguments: No CLI command given"))]
245+
ParseCLICommand,
246+
247+
#[snafu(display("Failed to parse CLI arguments: {}", parser_output))]
248+
ParseCLIArgs { parser_output: String },
232249

233250
#[snafu(display("Failed to write settings extension output as JSON: {}", source))]
234251
SerializeResult { source: serde_json::Error },

‎bottlerocket-settings-sdk/src/extension/proto1.rs

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
//! with function name collisions if needed.
55
use super::{error, SettingsExtensionError};
66
use crate::cli::proto1::{
7-
GenerateCommand, MigrateCommand, Proto1Command, SetCommand, TemplateHelperCommand,
8-
ValidateCommand,
7+
FloodMigrateCommand, GenerateCommand, MigrateCommand, Proto1Command, SetCommand,
8+
TemplateHelperCommand, ValidateCommand,
99
};
1010
use crate::migrate::Migrator;
1111
use crate::model::erased::AsTypeErasedModel;
@@ -51,6 +51,7 @@ where
5151
Proto1Command::Set(s) => extension.set(s).map(|_| String::new()),
5252
Proto1Command::Generate(g) => extension.generate(g).and_then(json_stringify),
5353
Proto1Command::Migrate(m) => extension.migrate(m).and_then(json_stringify),
54+
Proto1Command::FloodMigrate(m) => extension.flood_migrate(m).and_then(json_stringify),
5455
Proto1Command::Validate(v) => extension.validate(v).map(|_| String::new()),
5556
Proto1Command::Helper(h) => extension.template_helper(h).and_then(json_stringify),
5657
}
@@ -71,6 +72,10 @@ pub trait Proto1: Debug {
7172
&self,
7273
args: MigrateCommand,
7374
) -> Result<serde_json::Value, SettingsExtensionError<Self::MigratorErrorKind>>;
75+
fn flood_migrate(
76+
&self,
77+
args: FloodMigrateCommand,
78+
) -> Result<serde_json::Value, SettingsExtensionError<Self::MigratorErrorKind>>;
7479
fn validate(
7580
&self,
7681
args: ValidateCommand,
@@ -135,19 +140,39 @@ where
135140
setting_version: args.from_version.clone(),
136141
})?;
137142

138-
match (args.target_version, args.flood) {
139-
(Some(target_version), _) => self
140-
.migrator
141-
.perform_migration(self, starting_value, &args.from_version, &target_version)
142-
.context(error::MigrateSnafu),
143-
144-
// target_version is None, flood *must* be true.
145-
(None, _flood) => self
146-
.migrator
147-
.perform_flood_migrations(self, starting_value, &args.from_version)
148-
.context(error::MigrateSnafu)
149-
.and_then(|value| serde_json::to_value(value).context(error::SerializeResultSnafu)),
150-
}
143+
self.migrator
144+
.perform_migration(
145+
self,
146+
starting_value,
147+
&args.from_version,
148+
&args.target_version,
149+
)
150+
.context(error::MigrateSnafu)
151+
}
152+
153+
#[instrument(err)]
154+
fn flood_migrate(
155+
&self,
156+
args: FloodMigrateCommand,
157+
) -> Result<serde_json::Value, SettingsExtensionError<Self::MigratorErrorKind>> {
158+
let model = self
159+
.model(&args.from_version)
160+
.context(error::NoSuchModelSnafu {
161+
setting_version: args.from_version.clone(),
162+
})?;
163+
164+
let starting_value =
165+
model
166+
.as_model()
167+
.parse_erased(args.value)
168+
.context(error::ModelParseSnafu {
169+
setting_version: args.from_version.clone(),
170+
})?;
171+
172+
self.migrator
173+
.perform_flood_migrations(self, starting_value, &args.from_version)
174+
.context(error::MigrateSnafu)
175+
.and_then(|value| serde_json::to_value(value).context(error::SerializeResultSnafu))
151176
}
152177

153178
#[instrument(err)]

‎bottlerocket-settings-sdk/tests/sample_extensions.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,12 +179,11 @@ mod helpers {
179179
let args = vec![
180180
"extension",
181181
"proto1",
182-
"migrate",
182+
"flood-migrate",
183183
"--value",
184184
&value,
185185
"--from-version",
186186
from_version,
187-
"--flood",
188187
];
189188

190189
extension

‎deny.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ confidence-threshold = 0.93
1313
allow = [
1414
"Apache-2.0",
1515
# "BSD-2-Clause",
16-
# "BSD-3-Clause",
16+
"BSD-3-Clause",
1717
"BSL-1.0",
1818
# "CC0-1.0",
1919
# "ISC",

0 commit comments

Comments
 (0)