diff options
author | Michael Neumann <mneumann@ntecs.de> | 2021-04-13 15:04:30 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-04-13 15:04:30 +0200 |
commit | 697eacb6e9e851cd856b9eb792bbca6e400dd331 (patch) | |
tree | 37c2b7e236eff8519a8d66ff1879ef2e9f836003 | |
parent | e69dea81e85ec977c95339646aac3585c47a5a5f (diff) |
[CIT-304] Migrate tedge config to use the new tedge_config crate (#168)
- Disentangle TEdgeConfig from `tedge config` CLI
- Disentangle `tedge config` commands from CLI parsing
- Do all the parsing in `config/cli.rs`.
- Commands are defined in `config/commands/*.rs`. One (sub-)command per file.
- The implementation of the commands become easier to read
and understand, and you can test each command independently.
- Do not list valid keys when running `tedge config [get|set|unset] --help`
- When running `tedge config get --help`, the output is now:
ARGS:
<key> Configuration key. Run `tedge config list --doc` for available keys
- Prior to this commit, a list of keys was shown after the `<key>`, but
formatting was pretty ugly.
- The list of valid keys was generated by the macro. Get rid of it.
IMHO this was a mistake in the first place.
- Instead, point the user at running `tedge config list --doc` for a list
of valid keys. TODO: Currently the `tedge config list --doc` output
does not mention whether a given key is read-only or not.
- If we want the previous behaviour, it's better to just maintain the
list of valid keys in the structopt comment.
- tedge_config - Fix KEY's of some ConfigSettings
These are the keys that are used in tedge/tests/main.rs.
- Use crate tedge_config in `tedge config` CLI
- Add `ConfigSettingAccessorStringExt` trait to `tedge_config`. We need
that in order to get/set keys as strings. It provides a
`query_string`, `query_string_optional` and `update_string` methods.
- Provide a default implementation for `ConfigSettingAccessorStringExt`
for all ConfigSettings on `TEdgeConfig`.
- Introduce newtype `FilePath` and get rid of `PathBuf` in `TEdgeConfig`
and `TEdgeConfigDto`. We need this because `PathBuf` does not
implement `TryInto<String>` which we need for the default
implementation mentioned in the previous step.
- Make `TEdgeConfig` clonable. This is required when we update or unset
a config setting in cli/config/commands/{set,unset}.rs. In the command
we receive a `config: TEdgeConfig`, but as we are accessing it through
the `&self` in `execute`, we cannot modify it. To get a mutable
version of `config`, simply clone it.
- For those `tedge config` commands that need to write a configuration
back, inject a `TEdgeConfigRepository`. TODO: We should already inject
the `TEdgeConfigRepository` in `build_command`.
- Get rid of parts of the macro. We still need to keep parts of it
around for the other commands.
- Pass BuildCommandContext to BuildCommand::build_command
- This allows to inject TEdgeConfigRepository for commands that need to
write a config.
- Especially useful during the migration period, where we can still
provide `build_command` with an old config::TEdgeConfig.
- TODO: Some tests failing. I think some of the test conditions are wrong!
- Fix test
- We changed the default value
- When writing tedge.toml, create parent directory
- This fixes the tests.
- Create $HOME/.tedge or /etc/tedge if it does not exist yet.
- This needs some more love. Permissions should be set correctly.
- Remove underscored from trait definition
- Split TEdgeConfigDefaults from TEdgeConfigLocation
- `TEdgeConfigLocation` only tells us something about where the
`tedge.config` is located (and about it's root directory).
- `TEdgeConfigDefaults` only tells us something about the default values
used in `TEdgeConfig` as fallbacks.
- Both of them are independent entities.
- You can derive `TEdgeConfigDefaults::from(&TEdgeConfigLocation)`.
- Drop `Clone` - Use `config_repo.load()` instead of `config.clone()`
- Silence a few clippy warnings
- Rename BuildCommandContext to just BuildContext
28 files changed, 786 insertions, 624 deletions
diff --git a/tedge/src/certificate.rs b/tedge/src/certificate.rs index 85f92a85..17fc7b2d 100644 --- a/tedge/src/certificate.rs +++ b/tedge/src/certificate.rs @@ -5,7 +5,7 @@ use crate::utils::users; use crate::utils::users::UserManager; use crate::utils::{paths, paths::PathsError}; use crate::{ - command::{BuildCommand, Command, ExecutionContext}, + command::{BuildCommand, BuildContext, Command, ExecutionContext}, utils, }; use certificate::{KeyCertPair, NewCertificateConfig, PemCertificate}; @@ -78,7 +78,9 @@ pub enum UploadCertOpt { } impl BuildCommand for UploadCertOpt { - fn build_command(self, config: TEdgeConfig) -> Result<Box<dyn Command>, ConfigError> { + fn build_command(self, context: BuildContext) -> Result<Box<dyn Command>, ConfigError> { + let config = context.config; + match self { UploadCertOpt::C8y { username } => { let device_id = config.device.id.ok_or_else(|| ConfigError::ConfigNotSet { @@ -309,55 +311,54 @@ impl CertError { } impl BuildCommand for TEdgeCertOpt { - fn build_command(self, config: TEdgeConfig) -> Result<Box<dyn Command>, ConfigError> { - let cmd = - match self { - TEdgeCertOpt::Create { id } => { - let cmd = CreateCertCmd { - id, - cert_path: config.device.cert_path.ok_or_else(|| { - ConfigError::ConfigNotSet { - key: String::from(DEVICE_CERT_PATH), - } - })?, - key_path: config.device.key_path.ok_or_else(|| { - ConfigError::ConfigNotSet { - key: String::from(DEVICE_KEY_PATH), - } - })?, - }; - cmd.into_boxed() - } + fn build_command(self, context: BuildContext) -> Result<Box<dyn Command>, ConfigError> { + let cmd = match self { + TEdgeCertOpt::Create { id } => { + let cmd = CreateCertCmd { + id, + cert_path: context.config.device.cert_path.ok_or_else(|| { + ConfigError::ConfigNotSet { + key: String::from(DEVICE_CERT_PATH), + } + })?, + key_path: context.config.device.key_path.ok_or_else(|| { + ConfigError::ConfigNotSet { + key: String::from(DEVICE_KEY_PATH), + } + })?, + }; + cmd.into_boxed() + } - TEdgeCertOpt::Show => { - let cmd = ShowCertCmd { - cert_path: config.device.cert_path.ok_or_else(|| { - ConfigError::ConfigNotSet { - key: String::from(DEVICE_CERT_PATH), - } - })?, - }; - cmd.into_boxed() - } + TEdgeCertOpt::Show => { + let cmd = ShowCertCmd { + cert_path: context.config.device.cert_path.ok_or_else(|| { + ConfigError::ConfigNotSet { + key: String::from(DEVICE_CERT_PATH), + } + })?, + }; + cmd.into_boxed() + } - TEdgeCertOpt::Remove => { - let cmd = RemoveCertCmd { - cert_path: config.device.cert_path.ok_or_else(|| { - ConfigError::ConfigNotSet { - key: String::from(DEVICE_CERT_PATH), - } - })?, - key_path: config.device.key_path.ok_or_else(|| { - ConfigError::ConfigNotSet { - key: String::from(DEVICE_KEY_PATH), - } - })?, - }; - cmd.into_boxed() - } + TEdgeCertOpt::Remove => { + let cmd = RemoveCertCmd { + cert_path: context.config.device.cert_path.ok_or_else(|| { + ConfigError::ConfigNotSet { + key: String::from(DEVICE_CERT_PATH), + } + })?, + key_path: context.config.device.key_path.ok_or_else(|| { + ConfigError::ConfigNotSet { + key: String::from(DEVICE_KEY_PATH), + } + })?, + }; + cmd.into_boxed() + } - TEdgeCertOpt::Upload(cmd) => cmd.build_command(config)?, - }; + TEdgeCertOpt::Upload(cmd) => cmd.build_command(context)?, + }; Ok(cmd) } diff --git a/tedge/src/cli/config/cli.rs b/tedge/src/cli/config/cli.rs new file mode 100644 index 00000000..8806503a --- /dev/null +++ b/tedge/src/cli/config/cli.rs @@ -0,0 +1,73 @@ +use crate::cli::config::{commands::*, config_key::*}; +use crate::command::*; +use crate::config::*; +use structopt::StructOpt; +use tedge_config::ConfigRepository; + +#[derive(StructOpt, Debug)] +pub enum ConfigCmd { + /// Get the value of the provided configuration key + Get { + /// Configuration key. Run `tedge config list --doc` for available keys + key: ConfigKey, + }, + + /// Set or update the provided configuration key with the given value + Set { + /// Configuration key. Run `tedge config list --doc` for available keys + key: ConfigKey, + + /// Configuration value. + value: String, + }, + + /// Unset the provided configuration key + Unset { + /// Configuration key. Run `tedge config list --doc` for available keys + key: ConfigKey, + }, + + /// Print the configuration keys and their values + List { + /// Prints all the configuration keys, even those without a configured value + #[structopt(long = "all")] + is_all: bool, + + /// Prints all keys and descriptions with example values + #[structopt(long = "doc")] + is_doc: bool, + }, +} + +impl BuildCommand for ConfigCmd { + fn build_command(self, context: BuildContext) -> Result<Box<dyn Command>, ConfigError> { + let config = context.config_repository.load()?; + let config_repository = context.config_repository; + + match self { + ConfigCmd::Get { key } => Ok(GetConfigCommand { + config_key: key, + config, + } + .into_boxed()), + ConfigCmd::Set { key, value } => Ok(SetConfigCommand { + config_key: key, + value, + config_repository, + } + .into_boxed()), + ConfigCmd::Unset { key } => Ok(UnsetConfigCommand { + config_key: key, + config_repository, + } + .into_boxed()), + ConfigCmd::List { is_all, is_doc } => Ok(ListConfigCommand { + is_all, + is_doc, + config_keys: ConfigKey::list_all(), + config, + } + .into_boxed()), + } + } +} diff --git a/tedge/src/cli/config/commands/get.rs b/tedge/src/cli/config/commands/get.rs new file mode 100644 index 00000000..8788f203 --- /dev/null +++ b/tedge/src/cli/config/commands/get.rs @@ -0,0 +1,33 @@ +use crate::cli::config::ConfigKey; +use crate::command::{Command, ExecutionContext}; + +pub struct GetConfigCommand { + pub config_key: ConfigKey, + pub config: tedge_config::TEdgeConfig, +} + +impl Command for GetConfigCommand { + fn description(&self) -> String { + format!( + "get the configuration value for key: {}", + self.config_key.key + ) + } + + fn execute(&self, _context: &ExecutionContext) -> Result<(), anyhow::Error> { + match (self.config_key.get)(&self.config) { + Ok(value) => { + println!("{}", value); + } + Err(tedge_config::ConfigSettingError::ConfigNotSet { .. }) => { + println!( + "The provided config key: '{}' is not set", + self.config_key.key + ); + } + Err(err) => return Err(err.into()), + } + + Ok(()) + } +} diff --git a/tedge/src/cli/config/commands/list.rs b/tedge/src/cli/config/commands/list.rs new file mode 100644 index 00000000..0a31315a --- /dev/null +++ b/tedge/src/cli/config/commands/list.rs @@ -0,0 +1,59 @@ +use crate::cli::config::config_key::*; +use crate::command::{Command, ExecutionContext}; +use crate::config::ConfigError; +use tedge_config::*; + +pub struct ListConfigCommand { + pub is_all: bool, + pub is_doc: bool, + pub config: TEdgeConfig, + pub config_keys: Vec<ConfigKey>, +} + +impl Command for ListConfigCommand { + fn description(&self) -> String { + "list the configuration keys and values".into() + } + + fn execute(&self, _context: &ExecutionContext) -> Result<(), anyhow::Error> { + if self.is_doc { + print_config_doc(&self.config_keys); + } else { + print_config_list(&self.config_keys, &self.config, self.is_all)?; + } + + Ok(()) + } +} + +fn print_config_list( + config_keys: &[ConfigKey], + config: &TEdgeConfig, + all: bool, +) -> Result<(), ConfigError> { + let mut keys_without_values: Vec<String> = Vec::new(); + for config_key in config_keys { + match (config_key.get)(config) { + Ok(value) => { + println!("{}={}", config_key.key, value); + } + Err(tedge_config::ConfigSettingError::ConfigNotSet { .. }) => { + keys_without_values.push(config_key.key.into()); + } + Err(err) => return Err(err.into()), + } + } + if all && !keys_without_values.is_empty() { + println!(); + for key in keys_without_values { + println!("{}=", key); + } + } + Ok(()) +} + +fn print_config_doc(config_keys: &[ConfigKey]) { + for config_key in config_keys { + println!("{:<30} {}", config_key.key, config_key.description); + } +} diff --git a/tedge/src/cli/config/commands/mod.rs b/tedge/src/cli/config/commands/mod.rs new file mode 100644 index 00000000..e08b70da --- /dev/null +++ b/tedge/src/cli/config/commands/mod.rs @@ -0,0 +1,6 @@ +mod get; +mod list; +mod set; +mod unset; + +pub use self::{get::*, list::*, set::*, unset::*}; diff --git a/tedge/src/cli/config/commands/set.rs b/tedge/src/cli/config/commands/set.rs new file mode 100644 index 00000000..04d302da --- /dev/null +++ b/tedge/src/cli/config/commands/set.rs @@ -0,0 +1,25 @@ +use crate::cli::config::ConfigKey; +use crate::command::{Command, ExecutionContext}; +use tedge_config::*; + +pub struct SetConfigCommand { + pub config_key: ConfigKey, + pub value: String, + pub config_repository: TEdgeConfigRepository, +} + +impl Command for SetConfigCommand { + fn description(&self) -> String { + format!( + "set the configuration key: {} with value: {}.", + self.config_key.key, self.value + ) + } + + fn execute(&self, _context: &ExecutionContext) -> Result<(), anyhow::Error> { + let mut config = self.config_repository.load()?; + let () = (self.config_key.set)(&mut config, self.value.to_string())?; + self.config_repository.store(config)?; + Ok(()) + } +} diff --git a/tedge/src/cli/config/commands/unset.rs b/tedge/src/cli/config/commands/unset.rs new file mode 100644 index 00000000..b32b97d4 --- /dev/null +++ b/tedge/src/cli/config/commands/unset.rs @@ -0,0 +1,24 @@ +use crate::cli::config::ConfigKey; +use crate::command::{Command, ExecutionContext}; +use tedge_config::*; + +pub struct UnsetConfigCommand { + pub config_key: ConfigKey, + pub config_repository: TEdgeConfigRepository, +} + +impl Command for UnsetConfigCommand { + fn description(&self) -> String { + format!( + "unset the configuration value for key: {}", + self.config_key.key + ) + } + + fn execute(&self, _context: &ExecutionContext) -> Result<(), anyhow::Error> { + let mut config = self.config_repository.load()?; + let () = (self.config_key.unset)(&mut config)?; + self.config_repository.store(config)?; + Ok(()) + } +} diff --git a/tedge/src/cli/config/config_key.rs b/tedge/src/cli/config/config_key.rs new file mode 100644 index 00000000..4a557f73 --- /dev/null +++ b/tedge/src/cli/config/config_key.rs @@ -0,0 +1,68 @@ +use std::str::FromStr; +use tedge_config::*; + +/// Wrapper type for configuration keys. +/// +/// This is used to reject invalid config keys during CLI parsing. +pub struct ConfigKey { + pub key: &'static str, + pub description: &'static str, + pub get: GetConfigStringValue<TEdgeConfig>, + pub set: SetConfigStringValue<TEdgeConfig>, + pub unset: UnsetConfigValue<TEdgeConfig>, +} + +type GetConfigStringValue<C> = Box<dyn Fn(&C) -> ConfigSettingResult<String>>; +type SetConfigStringValue<C> = Box<dyn Fn(&mut C, String) -> ConfigSettingResult<()>>; +type UnsetConfigValue<C> = Box<dyn Fn(&mut C) -> ConfigSettingResult<()>>; + +impl std::fmt::Debug for ConfigKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "ConfigKey({})", self.key) + } +} + +macro_rules! config_key { + ($setting:tt) => { + ConfigKey { + key: $setting::KEY, + description: $setting::DESCRIPTION, + get: Box::new(move |config: &TEdgeConfig| config.query_string($setting)), + set: Box::new(move |config: &mut TEdgeConfig, value: String| { + config.update_string($setting, value) + }), + unset: Box::new(move |config: &mut TEdgeConfig| config.unset($setting)), + } + }; +} + +impl ConfigKey { + pub fn list_all() -> Vec<ConfigKey> { + vec![ + config_key!(DeviceIdSetting), + config_key!(DeviceKeyPathSetting), + config_key!(DeviceCertPathSetting), + config_key!(C8yUrlSetting), + config_key!(C8yUrlSetting), + config_key!(C8yRootCertPathSetting), + config_key!(AzureUrlSetting), + config_key!(AzureRootCertPathSetting), + ] + } +} + +impl FromStr for ConfigKey { + type Err = String; + + fn from_str(key: &str) -> Result<Self, Self::Err> { + ConfigKey::list_all() + .into_iter() + .find(|config_key| config_key.key == key) + .ok_or_else(|| { + format!( + "Invalid key `{}'. See `tedge config list --doc` for a list of valid keys.", + key, + ) + }) + } +} diff --git a/tedge/src/cli/config/mod.rs b/tedge/src/cli/config/mod.rs new file mode 100644 index 00000000..712fc380 --- /dev/null +++ b/tedge/src/cli/config/mod.rs @@ -0,0 +1,5 @@ +mod cli; +mod commands; +mod config_key; + +pub use self::{cli::*, commands::*, config_key::*}; diff --git a/tedge/src/cli/connect/mod.rs b/tedge/src/cli/connect/mod.rs index cabd7d2f..0a7884dc 100644 --- a/tedge/src/cli/connect/mod.rs +++ b/tedge/src/cli/connect/mod.rs @@ -1,5 +1,5 @@ use crate::cli::connect::{az::Azure, c8y::C8y}; -use crate::command::{BuildCommand, Command, ExecutionContext}; +use crate::command::{BuildCommand, BuildContext, Command, ExecutionContext}; use crate::config::{ConfigError, TEdgeConfig}; use crate::services::{ self, mosquitto::MosquittoService, tedge_mapper::TedgeMapperService, SystemdService, @@ -43,15 +43,15 @@ pub enum TEdgeConnectOpt { impl BuildCommand for TEdgeConnectOpt { fn build_command( self, - tedge_config: crate::config::TEdgeConfig, + context: BuildContext, ) -> Result<Box<dyn Command>, crate::config::ConfigError> { let cmd = match self { TEdgeConnectOpt::C8y => BridgeCommand { - bridge_config: C8y::c8y_bridge_config(tedge_config)?, + bridge_config: C8y::c8y_bridge_config(context.config)?, check_connection: Box::new(C8y {}), }, TEdgeConnectOpt::Az => BridgeCommand { - bridge_config: Azure::azure_bridge_config(tedge_config)?, + bridge_config: Azure::azure_bridge_config(context.config)?, check_connection: Box::new(Azure {}), }, }; diff --git a/tedge/src/cli/disconnect/mod.rs b/tedge/src/cli/disconnect/mod.rs index 58050386..48273bc9 100644 --- a/tedge/src/cli/disconnect/mod.rs +++ b/tedge/src/cli/disconnect/mod.rs @@ -1,7 +1,7 @@ use crate::cli::connect::{ az::AZURE_CONFIG_FILENAME, c8y::C8Y_CONFIG_FILENAME, TEDGE_BRIDGE_CONF_DIR_PATH, }; -use crate::command::{BuildCommand, Command, ExecutionContext}; +use crate::command::{BuildCommand, BuildContext, Command, ExecutionContext}; use crate::config::ConfigError; use crate::services::{ self, mosquitto::MosquittoService, tedge_mapper::TedgeMapperService, SystemdService, @@ -22,7 +22,7 @@ pub enum TedgeDisconnectBridgeOpt { impl BuildCommand for TedgeDisconnectBridgeOpt { fn build_command( self, - _tedge_config: crate::config::TEdgeConfig, + _context: BuildContext, ) -> Result<Box<dyn Command>, crate::config::ConfigError> { let cmd = match self { TedgeDisconnectBridgeOpt::C8y => DisconnectBridge { diff --git a/tedge/src/cli.rs b/tedge/src/cli/mod.rs index fa94aad3..486960a7 100644 --- a/tedge/src/cli.rs +++ b/tedge/src/cli/mod.rs @@ -1,11 +1,11 @@ use crate::certificate; -use crate::command::{BuildCommand, Command}; -use crate::config; -use crate::config::{ConfigError, TEdgeConfig}; +use crate::command::{BuildCommand, BuildContext, Command}; +use crate::config::ConfigError; use crate::mqtt; use structopt::clap; use structopt::StructOpt; +mod config; mod connect; mod disconnect; @@ -39,13 +39,13 @@ pub enum TEdgeOpt { } impl BuildCommand for TEdgeOpt { - fn build_command(self, config: TEdgeConfig) -> Result<Box<dyn Command>, ConfigError> { + fn build_command(self, context: BuildContext) -> Result<Box<dyn Command>, ConfigError> { match self { - TEdgeOpt::Cert(opt) => opt.build_command(config), - TEdgeOpt::Config(opt) => opt.build_command(config), - TEdgeOpt::Connect(opt) => opt.build_command(config), - TEdgeOpt::Disconnect(opt) => opt.build_command(config), - TEdgeOpt::Mqtt(opt) => opt.build_command(config), + TEdgeOpt::Cert(opt) => opt.build_command(context), + TEdgeOpt::Config(opt) => opt.build_command(context), + TEdgeOpt::Connect(opt) => opt.build_command(context), + TEdgeOpt::Disconnect(opt) => opt.build_command(context), + TEdgeOpt::Mqtt(opt) => opt.build_command(context), } } } diff --git a/tedge/src/command.rs b/tedge/src/command.rs index d7bb871c..78939637 100644 --- a/tedge/src/command.rs +++ b/tedge/src/command.rs @@ -138,15 +138,15 @@ pub trait Command { /// } /// /// impl BuildCommand for ConfigCmd { -/// fn build_command(self, config: TEdgeConfig) -> Result<Box<dyn Command>, ConfigError> { +/// fn build_command(self, context: BuildContext) -> Result<Box<dyn Command>, ConfigError> { /// let cmd = match self { /// ConfigCmd::Set { key, value } => SetConfigKey { -/// config, +/// config: context.config, /// key, /// value, /// }, /// ConfigCmd::Get { key } => GetConfigKey { -/// config, +/// config: context.config, /// key, /// }, /// } @@ -155,10 +155,14 @@ pub trait Command { /// } /// ``` pub trait BuildCommand { - fn build_command( - self, - config: config::TEdgeConfig, - ) -> Result<Box<dyn Command>, config::ConfigError>; + fn build_command(self, context: BuildContext) -> Result<Box<dyn Command>, config::ConfigError>; +} + +/// The context for `BuildCommand` +/// +pub struct BuildContext { + pub config: config::TEdgeConfig, + pub config_repository: tedge_config::TEdgeConfigRepository, } /// The execution context of a command. diff --git a/tedge/src/config.rs b/tedge/src/config.rs index 9e9aec80..3b53e65f 100644 --- a/tedge/src/config.rs +++ b/tedge/src/config.rs @@ -1,13 +1,8 @@ -use crate::{ - command::{BuildCommand, Command, ExecutionContext}, - utils::paths, -}; - +use crate::utils::paths; use serde::{Deserialize, Serialize}; use std::fs::{create_dir_all, read_to_string}; use std::io::{ErrorKind, Write}; use std::path::{Path, PathBuf}; -use structopt::StructOpt; use tempfile::NamedTempFile; const TEDGE_CONFIG_FILE: &str = "tedge.toml"; @@ -25,166 +20,6 @@ pub const C8Y_ROOT_CERT_PATH: &str = "c8y.root.cert.path"; pub const AZURE_URL: &str = "azure.url"; pub const AZURE_ROOT_CERT_PATH: &str = "azure.root.cert.path"; -/// Wrapper type for configuration keys. -#[derive(Debug, Clone)] -pub struct ConfigKey(pub String); - -impl ConfigKey { - fn as_str(&self) -> &str { - self.0.as_str() - } -} - -impl std::str::FromStr for ConfigKey { - type Err = String; - - fn from_str(key: &str) -> Result<Self, Self::Err> { - match TEdgeConfig::get_key_properties(key) { - Some(_) => Ok(ConfigKey(key.into())), |