Merge pull request #2789 from albinsuresh/fix/2787/use-c8y-mqtt-port

fix: Honor c8y.mqtt setting on tedge connect

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 {

crates/common/tedge_config/src/tedge_config_cli/models/host_port.rs

@@ -58,14 +58,6 @@ impl<const P: u16> HostPort<P> {
     pub fn port(&self) -> Port {
         self.port
     }
-
-    /// Returns a string representation of the host.
-    ///
-    /// In practice, it just returns the input string used to construct the
-    /// struct.
-    pub fn as_str(&self) -> &str {
-        &self.input
-    }
 }
 
 impl<const P: u16> From<HostPort<P>> for String {
@@ -76,7 +68,7 @@ impl<const P: u16> From<HostPort<P>> for String {
 
 impl<const P: u16> fmt::Display for HostPort<P> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.input.fmt(f)
+        write!(f, "{}:{}", self.hostname, self.port)
     }
 }
 
@@ -115,6 +107,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 {
@@ -138,3 +138,18 @@ pub enum ParseHostPortError {
     #[error("Could not parse port")]
     ParsePort(#[from] ParseIntError),
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::HTTPS_PORT;
+    use crate::MQTT_TLS_PORT;
+    use test_case::test_case;
+
+    #[test_case(HostPort::<HTTPS_PORT>::try_from("test.com").unwrap(), "test.com:443")]
+    #[test_case(HostPort::<MQTT_TLS_PORT>::try_from("test.com").unwrap(), "test.com:8883")]
+    #[test_case(HostPort::<HTTPS_PORT>::try_from("test.com:8443").unwrap(), "test.com:8443")]
+    fn to_string<const P: u16>(input: HostPort<P>, expected: &str) {
+        assert_eq!(input.to_string(), expected);
+    }
+}

crates/common/tedge_config/src/tedge_config_cli/models/port.rs

@@ -1,5 +1,5 @@
 use std::convert::TryFrom;
-use std::convert::TryInto;
+use std::fmt::Display;
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 pub struct Port(pub u16);
@@ -22,11 +22,9 @@ impl TryFrom<String> for Port {
     }
 }
 
-impl TryInto<String> for Port {
-    type Error = std::convert::Infallible;
-
-    fn try_into(self) -> Result<String, Self::Error> {
-        Ok(format!("{}", self.0))
+impl Display for Port {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.fmt(f)
     }
 }
 
@@ -53,5 +51,5 @@ fn conversion_from_longer_integer_fails() {
 
 #[test]
 fn conversion_from_port_to_string() {
-    assert_matches!(TryInto::<String>::try_into(Port(1234)), Ok(port_str) if port_str == "1234");
+    assert_eq!(Port(1234).to_string(), "1234");
 }

crates/core/c8y_api/src/http_proxy.rs

@@ -101,12 +101,19 @@ impl C8yEndPoint {
         // * <tenant_id>.<domain> eg: t12345.c8y.io
         // These URLs may be both equivalent and point to the same tenant.
         // We are going to remove that and only check if the domain is the same.
-        let tenant_uri = &self.c8y_host;
+        let (tenant_host, _port) = self
+            .c8y_host
+            .split_once(':')
+            .unwrap_or((&self.c8y_host, ""));
         let url = Url::parse(url).ok()?;
         let url_host = url.domain()?;
 
         let (_, host) = url_host.split_once('.').unwrap_or((url_host, ""));
-        let (_, c8y_host) = tenant_uri.split_once('.').unwrap();
+        let (_, c8y_host) = tenant_host.split_once('.').unwrap_or((tenant_host, ""));
+
+        // The configured `c8y.http` setting may have a port value specified,
+        // but the incoming URL is less likely to have any port specified.
+        // Hence just matching the host prefix.
         (host == c8y_host).then_some(url)
     }
 }
@@ -237,11 +244,28 @@ mod tests {
     #[test_case("http://test.co.te")]
     #[test_case("http://test.com:123456")]
     #[test_case("http://test.com::12345")]
+    #[test_case("http://localhost")]
+    #[test_case("http://abc.com")]
     fn url_is_my_tenant_incorrect_urls(url: &str) {
         let c8y = C8yEndPoint::new("test.test.com", "test_device");
         assert!(c8y.maybe_tenant_url(url).is_none());
     }
 
+    #[test]
+    fn url_is_my_tenant_with_hostname_without_commas() {
+        let c8y = C8yEndPoint::new("custom-domain", "test_device");
+        let url = "http://custom-domain/path";
+        assert_eq!(c8y.maybe_tenant_url(url), Some(url.parse().unwrap()));
+    }
+
+    #[ignore = "Until #2804 is fixed"]
+    #[test]
+    fn url_is_my_tenant_check_not_too_broad() {
+        let c8y = C8yEndPoint::new("abc.com", "test_device");
+        dbg!(c8y.maybe_tenant_url("http://xyz.com"));
+        assert!(c8y.maybe_tenant_url("http://xyz.com").is_none());
+    }
+
     #[test]
     fn check_non_cached_internal_id_for_a_device() {
         let mut c8y = C8yEndPoint::new("test_host", "test_device");

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(),

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(),
+            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(),

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(),