Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate old TEdgeConfig #2143

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bin/c8y-device-management/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ async fn main() -> anyhow::Result<()> {
let config_dir = config_plugin_opt.config_dir;
let tedge_config_location = TEdgeConfigLocation::from_custom_root(&config_dir);
let config_repository = TEdgeConfigRepository::new(tedge_config_location);
let tedge_config = config_repository.load_new()?;
let tedge_config = config_repository.load()?;

let c8y_http_config = (&tedge_config).try_into()?;
let mqtt_config = tedge_config.mqtt_config()?;
Expand Down
9 changes: 6 additions & 3 deletions crates/common/tedge_config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ pub mod tedge_config_cli;
pub use self::tedge_config_cli::config_setting::*;
pub use self::tedge_config_cli::error::*;
pub use self::tedge_config_cli::models::*;
pub use self::tedge_config_cli::new;
pub use self::tedge_config_cli::settings::*;
pub use self::tedge_config_cli::tedge_config::*;
pub use self::tedge_config_cli::tedge_config_defaults::*;
pub use self::tedge_config_cli::tedge_config_location::*;
pub use self::tedge_config_cli::tedge_config_repository::*;
pub use camino::Utf8Path as Path;
pub use camino::Utf8PathBuf as PathBuf;
pub use certificate::CertificateError;

/// loads the new tedge config from system default
pub fn get_new_tedge_config() -> Result<TEdgeConfig, TEdgeConfigError> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be deprecated as using a default location.

This is used in a single place: crates/core/tedge/src/cli/certificate/upload.rs.

I will fix that in a follow-up PR.

let tedge_config_location = TEdgeConfigLocation::default();
TEdgeConfigRepository::new(tedge_config_location).load()
}
48 changes: 0 additions & 48 deletions crates/common/tedge_config/src/mqtt_config.rs
Original file line number Diff line number Diff line change
@@ -1,54 +1,6 @@
use crate::ConfigSettingAccessor;
use crate::ConfigSettingError;
use crate::MqttClientAuthCertSetting;
use crate::MqttClientAuthKeySetting;
use crate::MqttClientCafileSetting;
use crate::MqttClientCapathSetting;
use crate::MqttClientHostSetting;
use crate::MqttClientPortSetting;
use crate::TEdgeConfig;
use certificate::CertificateError;

// TODO!: Remove this after replacing tedge config API by the new one.
impl TEdgeConfig {
pub fn mqtt_config(&self) -> Result<mqtt_channel::Config, MqttConfigBuildError> {
let host = self.query(MqttClientHostSetting)?;
let port = self.query(MqttClientPortSetting)?;
let mut mqtt_config = mqtt_channel::Config::default()
.with_host(host)
.with_port(port.into());

// If these options are not set, just dont use them
let ca_file = self.query(MqttClientCafileSetting).ok();
let ca_path = self.query(MqttClientCapathSetting).ok();

// Both these options have to either be set or not set, so we keep
// original error to rethrow when only one is set
let client_cert = self.query(MqttClientAuthCertSetting);
let client_key = self.query(MqttClientAuthKeySetting);

// Configure certificate authentication
if let Some(ca_file) = ca_file {
mqtt_config.with_cafile(ca_file)?;
}

if let Some(ca_path) = ca_path {
mqtt_config.with_cadir(ca_path)?;
}

// TODO (Marcel): remove unnecessary error checks once tedge_config
// refactor lands
if client_cert.is_ok() || client_key.is_ok() {
let client_cert = client_cert?;
let client_key = client_key?;

mqtt_config.with_client_auth(client_cert, client_key)?;
}

Ok(mqtt_config)
}
}

#[derive(Debug, thiserror::Error)]
pub enum MqttConfigBuildError {
#[error("Invalid tedge config")]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,50 +1,3 @@
pub trait ConfigSetting {
/// This is something like `device.id`.
const KEY: &'static str;

const DESCRIPTION: &'static str;

/// The underlying value type of the configuration setting.
type Value;
}

pub trait ConfigSettingAccessor<T: ConfigSetting> {
/// Read a configuration setting
fn query(&self, setting: T) -> ConfigSettingResult<T::Value>;

fn query_optional(&self, setting: T) -> ConfigSettingResult<Option<T::Value>> {
match self.query(setting) {
Ok(value) => Ok(Some(value)),
Err(ConfigSettingError::ConfigNotSet { .. }) => Ok(None),
Err(err) => Err(err),
}
}

/// Update a configuration setting
fn update(&mut self, _setting: T, _value: T::Value) -> ConfigSettingResult<()>;

/// Unset a configuration setting / reset to default
fn unset(&mut self, _setting: T) -> ConfigSettingResult<()>;
}

/// Extension trait that provides methods to query a setting as a String or
/// update a setting provided a String value.
pub trait ConfigSettingAccessorStringExt<T: ConfigSetting>: ConfigSettingAccessor<T> {
/// Read a configuration setting and convert it into a String.
fn query_string(&self, setting: T) -> ConfigSettingResult<String>;

fn query_string_optional(&self, setting: T) -> ConfigSettingResult<Option<String>> {
match self.query_string(setting) {
Ok(value) => Ok(Some(value)),
Err(ConfigSettingError::ConfigNotSet { .. }) => Ok(None),
Err(err) => Err(err),
}
}

/// Update a configuration setting from a String value
fn update_string(&mut self, setting: T, value: String) -> ConfigSettingResult<()>;
}

pub type ConfigSettingResult<T> = Result<T, ConfigSettingError>;

#[derive(thiserror::Error, Debug)]
Expand Down Expand Up @@ -77,5 +30,5 @@ pub enum ConfigSettingError {
Other { msg: &'static str },

#[error(transparent)]
Write(#[from] crate::new::WriteError),
Write(#[from] crate::WriteError),
}
4 changes: 2 additions & 2 deletions crates/common/tedge_config/src/tedge_config_cli/figment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,11 @@ impl TEdgeEnv {
Uncased::new(
tracing::subscriber::with_default(
tracing::subscriber::NoSubscriber::default(),
|| lowercase_name.parse::<crate::new::WritableKey>(),
|| lowercase_name.parse::<crate::WritableKey>(),
)
.map(|key| key.as_str().to_owned())
.map_err(|err| {
let is_read_only_key = matches!(err, crate::new::ParseKeyError::ReadOnly(_));
let is_read_only_key = matches!(err, crate::ParseKeyError::ReadOnly(_));
if is_read_only_key && !WARNINGS.lock().unwrap().insert(lowercase_name.clone()) {
tracing::error!(
"Failed to configure tedge with environment variable `TEDGE_{name}`: {}",
Expand Down
5 changes: 1 addition & 4 deletions crates/common/tedge_config/src/tedge_config_cli/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
pub mod config_setting;
pub mod error;
pub mod settings;
pub mod tedge_config;
pub mod tedge_config_defaults;
pub mod tedge_config_location;
pub mod tedge_config_repository;

mod figment;
pub mod models;
pub mod new;
pub mod tedge_config;
Loading