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

fix: Honor c8y.mqtt setting on tedge connect #2789

Merged
merged 2 commits into from
Mar 28, 2024
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
3 changes: 3 additions & 0 deletions crates/common/tedge_config/src/tedge_config_cli/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}
}
12 changes: 5 additions & 7 deletions crates/common/tedge_config/src/tedge_config_cli/models/port.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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)
}
}

Expand All @@ -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");
}
28 changes: 26 additions & 2 deletions crates/core/c8y_api/src/http_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +114 to +116
Copy link
Contributor Author

@albinsuresh albinsuresh Mar 22, 2024

Choose a reason for hiding this comment

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

The other option was to change the C8yEndPoint::c8y_host field into a HostPort and just validate the host part of it against the incoming host. But, that would be have been a huge change needing the generic port parameter <P> to be added to C8yEndPoint as well, since HostPort<P> is generic.

(host == c8y_host).then_some(url)
}
}
Expand Down Expand Up @@ -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");
Expand Down
17 changes: 7 additions & 10 deletions crates/core/tedge/src/bridge/aws.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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")?,
albinsuresh marked this conversation as resolved.
Show resolved Hide resolved
remote_username: Some("alpha".into()),
bridge_root_cert_path: Utf8PathBuf::from("./test_root.pem"),
remote_clientid: "alpha".into(),
Expand Down
19 changes: 9 additions & 10 deletions crates/core/tedge/src/bridge/azure.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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!(
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
10 changes: 3 additions & 7 deletions crates/core/tedge/src/bridge/c8y.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
Expand All @@ -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(),
Expand Down
Loading