fix: Honor c8y.mqtt setting on tedge connect thin-edge#2787

albinsuresh · Mar 18, 2024 · f41c16f · f41c16f
1 parent 01f1cea
commit f41c16f
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 67 deletions.
diff --git a/crates/common/tedge_config/src/tedge_config_cli/error.rs b/crates/common/tedge_config/src/tedge_config_cli/error.rs
@@ -29,6 +29,9 @@ pub enum TEdgeConfigError {
 
     #[error(transparent)]
     DirNotFound(#[from] tedge_utils::paths::PathsError),
+
+    #[error(transparent)]
+    FromParseHostPortError(#[from] crate::tedge_config_cli::models::host_port::ParseHostPortError),
 }
 
 impl TEdgeConfigError {

diff --git a/crates/common/tedge_config/src/tedge_config_cli/models/host_port.rs b/crates/common/tedge_config/src/tedge_config_cli/models/host_port.rs
@@ -115,6 +115,14 @@ impl<const P: u16> TryFrom<String> for HostPort<P> {
     }
 }
 
+impl<const P: u16> TryFrom<&str> for HostPort<P> {
+    type Error = ParseHostPortError;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        Self::try_from(value.to_owned())
+    }
+}
+
 impl<const P: u16> From<ConnectUrl> for HostPort<P> {
     fn from(value: ConnectUrl) -> Self {
         HostPort {

diff --git a/crates/core/tedge/src/bridge/aws.rs b/crates/core/tedge/src/bridge/aws.rs
@@ -1,14 +1,14 @@
 use super::BridgeConfig;
 use crate::bridge::config::BridgeLocation;
 use camino::Utf8PathBuf;
-use tedge_config::ConnectUrl;
+use tedge_config::HostPort;
+use tedge_config::MQTT_TLS_PORT;
 
 const MOSQUITTO_BRIDGE_TOPIC: &str = "te/device/main/service/mosquitto-aws-bridge/status/health";
 
 #[derive(Debug, Eq, PartialEq)]
 pub struct BridgeConfigAwsParams {
-    pub connect_url: ConnectUrl,
-    pub mqtt_tls_port: u16,
+    pub mqtt_host: HostPort<MQTT_TLS_PORT>,
     pub config_file: String,
     pub remote_clientid: String,
     pub bridge_root_cert_path: Utf8PathBuf,
@@ -19,16 +19,14 @@ pub struct BridgeConfigAwsParams {
 impl From<BridgeConfigAwsParams> for BridgeConfig {
     fn from(params: BridgeConfigAwsParams) -> Self {
         let BridgeConfigAwsParams {
-            connect_url,
-            mqtt_tls_port,
+            mqtt_host,
             config_file,
             bridge_root_cert_path,
             remote_clientid,
             bridge_certfile,
             bridge_keyfile,
         } = params;
 
-        let address = format!("{}:{}", connect_url, mqtt_tls_port);
         let user_name = remote_clientid.to_string();
 
         // telemetry/command topics for use by the user
@@ -50,7 +48,7 @@ impl From<BridgeConfigAwsParams> for BridgeConfig {
             cloud_name: "aws".into(),
             config_file,
             connection: "edge_to_aws".into(),
-            address,
+            address: mqtt_host,
             remote_username: Some(user_name),
             bridge_root_cert_path,
             remote_clientid,
@@ -86,8 +84,7 @@ fn test_bridge_config_from_aws_params() -> anyhow::Result<()> {
     use std::convert::TryFrom;
 
     let params = BridgeConfigAwsParams {
-        connect_url: ConnectUrl::try_from("test.test.io")?,
-        mqtt_tls_port: 8883,
+        mqtt_host: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io")?,
         config_file: "aws-bridge.conf".into(),
         remote_clientid: "alpha".into(),
         bridge_root_cert_path: "./test_root.pem".into(),
@@ -101,7 +98,7 @@ fn test_bridge_config_from_aws_params() -> anyhow::Result<()> {
         cloud_name: "aws".into(),
         config_file: "aws-bridge.conf".to_string(),
         connection: "edge_to_aws".into(),
-        address: "test.test.io:8883".into(),
+        address: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io")?,
         remote_username: Some("alpha".into()),
         bridge_root_cert_path: Utf8PathBuf::from("./test_root.pem"),
         remote_clientid: "alpha".into(),

diff --git a/crates/core/tedge/src/bridge/azure.rs b/crates/core/tedge/src/bridge/azure.rs
@@ -1,14 +1,14 @@
 use super::BridgeConfig;
 use crate::bridge::config::BridgeLocation;
 use camino::Utf8PathBuf;
-use tedge_config::ConnectUrl;
+use tedge_config::HostPort;
+use tedge_config::MQTT_TLS_PORT;
 
 const MOSQUITTO_BRIDGE_TOPIC: &str = "te/device/main/service/mosquitto-az-bridge/status/health";
 
 #[derive(Debug, Eq, PartialEq)]
 pub struct BridgeConfigAzureParams {
-    pub connect_url: ConnectUrl,
-    pub mqtt_tls_port: u16,
+    pub mqtt_host: HostPort<MQTT_TLS_PORT>,
     pub config_file: String,
     pub remote_clientid: String,
     pub bridge_root_cert_path: Utf8PathBuf,
@@ -19,19 +19,19 @@ pub struct BridgeConfigAzureParams {
 impl From<BridgeConfigAzureParams> for BridgeConfig {
     fn from(params: BridgeConfigAzureParams) -> Self {
         let BridgeConfigAzureParams {
-            connect_url,
-            mqtt_tls_port,
+            mqtt_host,
             config_file,
             bridge_root_cert_path,
             remote_clientid,
             bridge_certfile,
             bridge_keyfile,
         } = params;
 
-        let address = format!("{}:{}", connect_url, mqtt_tls_port);
+        let address = mqtt_host.clone();
         let user_name = format!(
             "{}/{}/?api-version=2018-06-30",
-            connect_url, remote_clientid
+            mqtt_host.host().to_string(),
+            remote_clientid
         );
         let pub_msg_topic = format!("messages/events/# out 1 az/ devices/{}/", remote_clientid);
         let sub_msg_topic = format!(
@@ -84,8 +84,7 @@ fn test_bridge_config_from_azure_params() -> anyhow::Result<()> {
     use std::convert::TryFrom;
 
     let params = BridgeConfigAzureParams {
-        connect_url: ConnectUrl::try_from("test.test.io")?,
-        mqtt_tls_port: 8883,
+        mqtt_host: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io")?,
         config_file: "az-bridge.conf".into(),
         remote_clientid: "alpha".into(),
         bridge_root_cert_path: "./test_root.pem".into(),
@@ -99,7 +98,7 @@ fn test_bridge_config_from_azure_params() -> anyhow::Result<()> {
         cloud_name: "az".into(),
         config_file: "az-bridge.conf".to_string(),
         connection: "edge_to_az".into(),
-        address: "test.test.io:8883".into(),
+        address: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io")?,
         remote_username: Some("test.test.io/alpha/?api-version=2018-06-30".into()),
         bridge_root_cert_path: Utf8PathBuf::from("./test_root.pem"),
         remote_clientid: "alpha".into(),

diff --git a/crates/core/tedge/src/bridge/c8y.rs b/crates/core/tedge/src/bridge/c8y.rs
@@ -93,11 +93,7 @@ impl From<BridgeConfigC8yParams> for BridgeConfig {
             cloud_name: "c8y".into(),
             config_file,
             connection: "edge_to_c8y".into(),
-            address: format!(
-                "{host}:{port}",
-                host = mqtt_host.host(),
-                port = mqtt_host.port().0
-            ),
+            address: mqtt_host,
             remote_username: None,
             bridge_root_cert_path,
             remote_clientid,
@@ -159,7 +155,7 @@ mod tests {
     fn test_bridge_config_from_c8y_params() -> anyhow::Result<()> {
         use std::convert::TryFrom;
         let params = BridgeConfigC8yParams {
-            mqtt_host: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io".to_string())?,
+            mqtt_host: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io")?,
             config_file: C8Y_CONFIG_FILENAME.into(),
             remote_clientid: "alpha".into(),
             bridge_root_cert_path: Utf8PathBuf::from("./test_root.pem"),
@@ -176,7 +172,7 @@ mod tests {
             cloud_name: "c8y".into(),
             config_file: C8Y_CONFIG_FILENAME.into(),
             connection: "edge_to_c8y".into(),
-            address: "test.test.io:8883".into(),
+            address: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io")?,
             remote_username: None,
             bridge_root_cert_path: Utf8PathBuf::from("./test_root.pem"),
             remote_clientid: "alpha".into(),

diff --git a/crates/core/tedge/src/bridge/config.rs b/crates/core/tedge/src/bridge/config.rs
@@ -1,7 +1,8 @@
 use camino::Utf8PathBuf;
+use tedge_config::HostPort;
 use tedge_config::TEdgeConfigLocation;
+use tedge_config::MQTT_TLS_PORT;
 use tedge_utils::paths::DraftFile;
-use url::Url;
 
 use crate::ConnectError;
 
@@ -13,7 +14,7 @@ pub struct BridgeConfig {
     // XXX: having file name squished together with 20 fields which go into file content is a bit obscure
     pub config_file: String,
     pub connection: String,
-    pub address: String,
+    pub address: HostPort<MQTT_TLS_PORT>,
     pub remote_username: Option<String>,
     pub bridge_root_cert_path: Utf8PathBuf,
     pub remote_clientid: String,
@@ -91,10 +92,6 @@ impl BridgeConfig {
     }
 
     pub fn validate(&self) -> Result<(), ConnectError> {
-        // XXX: This is actually wrong. Our address looks like this: `domain:port`
-        // `Url::parse` will treat `domain` as `schema` ...
-        Url::parse(&self.address)?;
-
         if !self.bridge_root_cert_path.exists() {
             return Err(ConnectError::Certificate);
         }
@@ -141,6 +138,8 @@ impl BridgeConfig {
 #[cfg(test)]
 mod test {
 
+    use std::str::FromStr;
+
     use super::*;
     use camino::Utf8Path;
     use camino::Utf8PathBuf;
@@ -154,7 +153,7 @@ mod test {
             cloud_name: "test".into(),
             config_file: "test-bridge.conf".into(),
             connection: "edge_to_test".into(),
-            address: "test.test.io:8883".into(),
+            address: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io:8883")?,
             remote_username: None,
             bridge_root_cert_path: bridge_root_cert_path.to_owned(),
             remote_clientid: "alpha".into(),
@@ -220,7 +219,7 @@ bridge_attempt_unsubscribe false
             cloud_name: "test".into(),
             config_file: "test-bridge.conf".into(),
             connection: "edge_to_test".into(),
-            address: "test.test.io:8883".into(),
+            address: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io:8883")?,
             remote_username: None,
             bridge_root_cert_path: bridge_root_cert_path.to_owned(),
             remote_clientid: "alpha".into(),
@@ -285,7 +284,7 @@ bridge_attempt_unsubscribe false
             cloud_name: "az".into(),
             config_file: "az-bridge.conf".into(),
             connection: "edge_to_az".into(),
-            address: "test.test.io:8883".into(),
+            address: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io:8883")?,
             remote_username: Some("test.test.io/alpha/?api-version=2018-06-30".into()),
             bridge_root_cert_path: bridge_root_cert_path.to_owned(),
             remote_clientid: "alpha".into(),
@@ -355,10 +354,8 @@ bridge_attempt_unsubscribe false
         let key_file = tempfile::NamedTempFile::new()?;
         let bridge_keyfile = Utf8Path::from_path(key_file.path()).unwrap().to_owned();
 
-        let correct_url = "http://test.com";
-
         let config = BridgeConfig {
-            address: correct_url.into(),
+            address: HostPort::<MQTT_TLS_PORT>::try_from("test.test.io:8883".to_string())?,
             bridge_root_cert_path: bridge_ca_path.to_owned(),
             bridge_certfile,
             bridge_keyfile,
@@ -370,30 +367,12 @@ bridge_attempt_unsubscribe false
         Ok(())
     }
 
-    // XXX: This test is flawed as it is not clear what it tests.
-    // It can fail due to either `incorrect_url` OR `non_existent_path`.
-    #[test]
-    fn test_validate_wrong_url() {
-        let incorrect_url = "noturl";
-        let non_existent_path = Utf8PathBuf::from("/path/that/does/not/exist");
-
-        let config = BridgeConfig {
-            address: incorrect_url.into(),
-            bridge_certfile: non_existent_path.clone(),
-            bridge_keyfile: non_existent_path,
-            ..default_bridge_config()
-        };
-
-        assert!(config.validate().is_err());
-    }
-
     #[test]
     fn test_validate_wrong_cert_path() {
-        let correct_url = "http://test.com";
         let non_existent_path = Utf8PathBuf::from("/path/that/does/not/exist");
 
         let config = BridgeConfig {
-            address: correct_url.into(),
+            address: HostPort::<MQTT_TLS_PORT>::try_from("test.com").unwrap(),
             bridge_certfile: non_existent_path.clone(),
             bridge_keyfile: non_existent_path,
             ..default_bridge_config()
@@ -406,11 +385,10 @@ bridge_attempt_unsubscribe false
     fn test_validate_wrong_key_path() -> anyhow::Result<()> {
         let cert_file = tempfile::NamedTempFile::new()?;
         let bridge_certfile = Utf8Path::from_path(cert_file.path()).unwrap().to_owned();
-        let correct_url = "http://test.com";
         let non_existent_path = "/path/that/does/not/exist";
 
         let config = BridgeConfig {
-            address: correct_url.into(),
+            address: HostPort::<MQTT_TLS_PORT>::try_from("test.com".to_string())?,
             bridge_certfile,
             bridge_keyfile: non_existent_path.into(),
             ..default_bridge_config()
@@ -426,7 +404,7 @@ bridge_attempt_unsubscribe false
             cloud_name: "az/c8y".into(),
             config_file: "cfg".to_string(),
             connection: "edge_to_az/c8y".into(),
-            address: "".into(),
+            address: HostPort::<MQTT_TLS_PORT>::from_str("test.com").unwrap(),
             remote_username: None,
             bridge_root_cert_path: "".into(),
             bridge_certfile: "".into(),

diff --git a/crates/core/tedge/src/cli/connect/c8y_direct_connection.rs b/crates/core/tedge/src/cli/connect/c8y_direct_connection.rs
@@ -25,9 +25,12 @@ pub fn create_device_with_direct_connection(
     const DEVICE_CREATE_ERROR_TOPIC: &str = "s/e";
 
     let address = bridge_config.address.clone();
-    let host: Vec<&str> = address.split(':').collect();
 
-    let mut mqtt_options = MqttOptions::new(bridge_config.remote_clientid.clone(), host[0], 8883);
+    let mut mqtt_options = MqttOptions::new(
+        bridge_config.remote_clientid.clone(),
+        address.host().to_string(),
+        address.port().into(),
+    );
     mqtt_options.set_keep_alive(std::time::Duration::from_secs(5));
 
     let tls_config = create_tls_config(

diff --git a/crates/core/tedge/src/cli/connect/command.rs b/crates/core/tedge/src/cli/connect/command.rs
@@ -187,8 +187,10 @@ pub fn bridge_config(
     match cloud {
         Cloud::Azure => {
             let params = BridgeConfigAzureParams {
-                connect_url: config.az.url.or_config_not_set()?.clone(),
-                mqtt_tls_port: MQTT_TLS_PORT,
+                mqtt_host: HostPort::<MQTT_TLS_PORT>::try_from(
+                    config.az.url.or_config_not_set()?.as_str(),
+                )
+                .map_err(TEdgeConfigError::from)?,
                 config_file: AZURE_CONFIG_FILENAME.into(),
                 bridge_root_cert_path: config.az.root_cert_path.clone(),
                 remote_clientid: config.device.id.try_read(config)?.clone(),
@@ -200,8 +202,10 @@ pub fn bridge_config(
         }
         Cloud::Aws => {
             let params = BridgeConfigAwsParams {
-                connect_url: config.aws.url.or_config_not_set()?.clone(),
-                mqtt_tls_port: MQTT_TLS_PORT,
+                mqtt_host: HostPort::<MQTT_TLS_PORT>::try_from(
+                    config.aws.url.or_config_not_set()?.as_str(),
+                )
+                .map_err(TEdgeConfigError::from)?,
                 config_file: AWS_CONFIG_FILENAME.into(),
                 bridge_root_cert_path: config.aws.root_cert_path.clone(),
                 remote_clientid: config.device.id.try_read(config)?.clone(),