From 3c8c62f287dc26b98b9ee48069974190dbdfbbe0 Mon Sep 17 00:00:00 2001 From: Johnson Shih Date: Thu, 22 Jun 2023 09:31:50 -0700 Subject: [PATCH] correct onvif filters (#622) * correct filters Signed-off-by: Johnson Shih * Update patch version Signed-off-by: Johnson Shih * correct how scope is matched Signed-off-by: Johnson Shih * add more test cases Signed-off-by: Johnson Shih * case-insensitive comparison for ip address Signed-off-by: Johnson Shih * utility functions for ut Signed-off-by: Johnson Shih * Update patch version Signed-off-by: Johnson Shih * rename test function name Signed-off-by: Johnson Shih --------- Signed-off-by: Johnson Shih --- Cargo.lock | 28 +-- agent/Cargo.toml | 2 +- controller/Cargo.toml | 2 +- deployment/helm/Chart.yaml | 4 +- .../debug-echo-discovery-handler/Cargo.toml | 2 +- .../onvif-discovery-handler/Cargo.toml | 2 +- .../opcua-discovery-handler/Cargo.toml | 2 +- .../udev-discovery-handler/Cargo.toml | 2 +- discovery-handlers/debug-echo/Cargo.toml | 2 +- discovery-handlers/onvif/Cargo.toml | 2 +- .../onvif/src/discovery_handler.rs | 63 +++++ .../onvif/src/discovery_impl.rs | 222 ++++++++++++++++-- discovery-handlers/opcua/Cargo.toml | 2 +- discovery-handlers/udev/Cargo.toml | 2 +- discovery-utils/Cargo.toml | 2 +- samples/brokers/udev-video-broker/Cargo.toml | 2 +- shared/Cargo.toml | 2 +- version.txt | 2 +- webhooks/validating/configuration/Cargo.toml | 2 +- 19 files changed, 298 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dfbf7d5a7..3e14b1aa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -333,7 +333,7 @@ checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" [[package]] name = "agent" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-debug-echo", "akri-discovery-utils", @@ -402,7 +402,7 @@ dependencies = [ [[package]] name = "akri-debug-echo" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-discovery-utils", "akri-shared", @@ -422,7 +422,7 @@ dependencies = [ [[package]] name = "akri-discovery-utils" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-shared", "anyhow", @@ -444,7 +444,7 @@ dependencies = [ [[package]] name = "akri-onvif" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-discovery-utils", "akri-shared", @@ -472,7 +472,7 @@ dependencies = [ [[package]] name = "akri-opcua" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-discovery-utils", "akri-shared", @@ -496,7 +496,7 @@ dependencies = [ [[package]] name = "akri-shared" -version = "0.10.13" +version = "0.10.14" dependencies = [ "anyhow", "async-trait", @@ -525,7 +525,7 @@ dependencies = [ [[package]] name = "akri-udev" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-discovery-utils", "anyhow", @@ -1043,7 +1043,7 @@ checksum = "fbdcdcb6d86f71c5e97409ad45898af11cbc995b4ee8112d59095a28d376c935" [[package]] name = "controller" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-shared", "anyhow", @@ -1243,7 +1243,7 @@ dependencies = [ [[package]] name = "debug-echo-discovery-handler" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-debug-echo", "akri-discovery-utils", @@ -2540,7 +2540,7 @@ checksum = "b7e5500299e16ebb147ae15a00a942af264cf3688f47923b8fc2cd5858f23ad3" [[package]] name = "onvif-discovery-handler" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-discovery-utils", "akri-onvif", @@ -2590,7 +2590,7 @@ dependencies = [ [[package]] name = "opcua-discovery-handler" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-discovery-utils", "akri-opcua", @@ -4206,7 +4206,7 @@ dependencies = [ [[package]] name = "udev-discovery-handler" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-discovery-utils", "akri-udev", @@ -4217,7 +4217,7 @@ dependencies = [ [[package]] name = "udev-video-broker" -version = "0.10.13" +version = "0.10.14" dependencies = [ "akri-shared", "env_logger", @@ -4494,7 +4494,7 @@ dependencies = [ [[package]] name = "webhook-configuration" -version = "0.10.13" +version = "0.10.14" dependencies = [ "actix", "actix-rt 2.7.0", diff --git a/agent/Cargo.toml b/agent/Cargo.toml index 90d9d7ed0..858aad2b7 100644 --- a/agent/Cargo.toml +++ b/agent/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "agent" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring ", ""] edition = "2018" rust-version = "1.68.1" diff --git a/controller/Cargo.toml b/controller/Cargo.toml index 58ed3302e..d576a3b11 100644 --- a/controller/Cargo.toml +++ b/controller/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "controller" -version = "0.10.13" +version = "0.10.14" authors = ["", ""] edition = "2018" rust-version = "1.68.1" diff --git a/deployment/helm/Chart.yaml b/deployment/helm/Chart.yaml index f19dc6c80..55dba676f 100644 --- a/deployment/helm/Chart.yaml +++ b/deployment/helm/Chart.yaml @@ -16,9 +16,9 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.10.13 +version: 0.10.14 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. -appVersion: 0.10.13 +appVersion: 0.10.14 diff --git a/discovery-handler-modules/debug-echo-discovery-handler/Cargo.toml b/discovery-handler-modules/debug-echo-discovery-handler/Cargo.toml index b19d52fe9..8f113f35f 100644 --- a/discovery-handler-modules/debug-echo-discovery-handler/Cargo.toml +++ b/discovery-handler-modules/debug-echo-discovery-handler/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "debug-echo-discovery-handler" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-handler-modules/onvif-discovery-handler/Cargo.toml b/discovery-handler-modules/onvif-discovery-handler/Cargo.toml index c3b6a27dd..359ba8076 100644 --- a/discovery-handler-modules/onvif-discovery-handler/Cargo.toml +++ b/discovery-handler-modules/onvif-discovery-handler/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "onvif-discovery-handler" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-handler-modules/opcua-discovery-handler/Cargo.toml b/discovery-handler-modules/opcua-discovery-handler/Cargo.toml index 7fa7a0a36..34165bcf8 100644 --- a/discovery-handler-modules/opcua-discovery-handler/Cargo.toml +++ b/discovery-handler-modules/opcua-discovery-handler/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "opcua-discovery-handler" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-handler-modules/udev-discovery-handler/Cargo.toml b/discovery-handler-modules/udev-discovery-handler/Cargo.toml index afdb68724..ea0dae0e3 100644 --- a/discovery-handler-modules/udev-discovery-handler/Cargo.toml +++ b/discovery-handler-modules/udev-discovery-handler/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "udev-discovery-handler" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-handlers/debug-echo/Cargo.toml b/discovery-handlers/debug-echo/Cargo.toml index bd6fb8778..62511d567 100644 --- a/discovery-handlers/debug-echo/Cargo.toml +++ b/discovery-handlers/debug-echo/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-debug-echo" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-handlers/onvif/Cargo.toml b/discovery-handlers/onvif/Cargo.toml index e4bfce627..0b023a7ac 100644 --- a/discovery-handlers/onvif/Cargo.toml +++ b/discovery-handlers/onvif/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-onvif" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-handlers/onvif/src/discovery_handler.rs b/discovery-handlers/onvif/src/discovery_handler.rs index aeb228902..b464a9ba4 100644 --- a/discovery-handlers/onvif/src/discovery_handler.rs +++ b/discovery-handlers/onvif/src/discovery_handler.rs @@ -164,10 +164,12 @@ async fn apply_filters( } }; // Evaluate camera ip address against ip filter if provided + // use case-insensitive comparison in case of IPv6 is used let ip_address_as_vec = vec![ip_address.clone()]; if util::execute_filter( discovery_handler_config.ip_addresses.as_ref(), &ip_address_as_vec, + |scope, pattern| scope.to_lowercase() == pattern.to_lowercase(), ) { return None; } @@ -177,6 +179,7 @@ async fn apply_filters( if util::execute_filter( discovery_handler_config.mac_addresses.as_ref(), &mac_address_as_vec, + |scope, pattern| scope.to_lowercase() == pattern.to_lowercase(), ) { return None; } @@ -583,4 +586,64 @@ mod tests { .await .is_none()); } + + #[tokio::test] + async fn test_apply_filters_include_mac_exist_different_letter_cases() { + let mock_uri = "device_uri"; + let mock_ip = "mock.ip"; + let mock_mac = "MocK:Mac"; + + let mut mock = MockOnvifQuery::new(); + configure_scenario( + &mut mock, + Some(IpAndMac { + mock_uri, + mock_ip, + mock_mac, + }), + ); + + let onvif_config = OnvifDiscoveryDetails { + ip_addresses: None, + mac_addresses: Some(FilterList { + action: FilterType::Include, + items: vec![mock_mac.to_uppercase()], + }), + scopes: None, + discovery_timeout_seconds: 1, + }; + let instance = apply_filters(&onvif_config, mock_uri, &mock).await.unwrap(); + + assert_eq!(expected_device(mock_uri, mock_ip, mock_mac), instance); + } + + #[tokio::test] + async fn test_apply_filters_exclude_mac_exist_different_letter_cases() { + let mock_uri = "device_uri"; + let mock_ip = "mock.ip"; + let mock_mac = "MocK:Mac"; + + let mut mock = MockOnvifQuery::new(); + configure_scenario( + &mut mock, + Some(IpAndMac { + mock_uri, + mock_ip, + mock_mac, + }), + ); + + let onvif_config = OnvifDiscoveryDetails { + ip_addresses: None, + mac_addresses: Some(FilterList { + action: FilterType::Exclude, + items: vec![mock_mac.to_uppercase()], + }), + scopes: None, + discovery_timeout_seconds: 1, + }; + assert!(apply_filters(&onvif_config, mock_uri, &mock) + .await + .is_none()); + } } diff --git a/discovery-handlers/onvif/src/discovery_impl.rs b/discovery-handlers/onvif/src/discovery_impl.rs index af2405f31..00d1686a5 100644 --- a/discovery-handlers/onvif/src/discovery_impl.rs +++ b/discovery-handlers/onvif/src/discovery_impl.rs @@ -166,15 +166,19 @@ pub mod util { mod serialize_tests { use super::*; + /// Get SOAP probe message with a specific message id + fn get_expected_probe_message(message_id: &str) -> String { + format!( + "{}urn:schemas-xmlsoap-org:ws:2005:04:discoveryhttp://schemas.xmlsoap.org/ws/2005/04/discovery/Probenetwsdl:NetworkVideoTransmitter", + &message_id) + } + #[test] fn test_create_onvif_discovery_message() { let _ = env_logger::builder().is_test(true).try_init(); let uuid_str = format!("uuid:{}", uuid::Uuid::new_v4()); - let expected_msg = format!( - "{}urn:schemas-xmlsoap-org:ws:2005:04:discoveryhttp://schemas.xmlsoap.org/ws/2005/04/discovery/Probenetwsdl:NetworkVideoTransmitter", - &uuid_str - ); + let expected_msg = get_expected_probe_message(&uuid_str); assert_eq!(expected_msg, create_onvif_discovery_message(&uuid_str)); } } @@ -186,18 +190,25 @@ pub mod util { let response_envelope = yaserde::de::from_str::(discovery_response); // The response envelope follows this format: - // - // https://10.0.0.1:5357/svc - // https://10.0.0.2:5357/svc - // https://10.0.0.3:5357/svc - // + // + // onvif://www.onvif.org/name/foo onvif://www.onvif.org/hardware/bar + // + // https://10.0.0.1:5357/svc + // https://10.0.0.2:5357/svc + // https://10.0.0.3:5357/svc + // + // response_envelope .unwrap() .body .probe_matches .probe_match .iter() - .filter(|probe_match| !execute_filter(scopes, &probe_match.scopes)) + .filter(|probe_match| { + !execute_filter(scopes, &probe_match.scopes, |scope, pattern| { + scope.split_whitespace().any(|s| s == pattern) + }) + }) .flat_map(|probe_match| probe_match.xaddrs.split_whitespace()) .map(|addr| addr.to_string()) .collect::>() @@ -227,10 +238,14 @@ pub mod util { .collect() } - pub(crate) fn execute_filter( + pub(crate) fn execute_filter

( filter_list: Option<&FilterList>, filter_against: &[String], - ) -> bool { + predicate: P, + ) -> bool + where + P: Fn(&str, &str) -> bool, + { if filter_list.is_none() { return false; } @@ -242,7 +257,7 @@ pub mod util { .filter(|pattern| { filter_against .iter() - .filter(|filter_against_item| filter_against_item == pattern) + .filter(|filter_against_item| predicate(filter_against_item, pattern)) .count() > 0 }) @@ -259,20 +274,191 @@ pub mod util { mod deserialize_tests { use super::*; + /// Get SOAP probe match message with a list of XAddrs + fn get_expected_probe_match_message(xaddrs: &[String]) -> String { + format!( + r#" + + + urn:uuid:2bc6f06c-5566-7788-99ac-0012414fb745 + uuid:7b1d26aa-b02e-4ad2-8aab-4c928298ee0c + http://schemas.xmlsoap.org/ws/2004/08/addressing/role/anonymous + http://schemas.xmlsoap.org/ws/2005/04/discovery/ProbeMatches + + + + + + urn:uuid:10919da4-5566-7788-99aa-0012414fb745 + + dn:NetworkVideoTransmitter + onvif://www.onvif.org/type/video_encoder onvif://www.onvif.org/type/audio_encoder onvif://www.onvif.org/hardware/IPC-model onvif://www.onvif.org/location/country/china onvif://www.onvif.org/name/NVT onvif://www.onvif.org/Profile/Streaming + {} + 10 + + + + "#, + &xaddrs.join(" ") + ) + } + #[test] - fn test_get_scope_filtered_uris_from_discovery_response() { + fn test_get_scope_filtered_uris_from_discovery_response_no_filter() { let _ = env_logger::builder().is_test(true).try_init(); let uris = vec!["uri_one".to_string(), "uri_two".to_string()]; - let response = format!( - "\nurn:uuid:2bc6f06c-5566-7788-99ac-0012414fb745uuid:7b1d26aa-b02e-4ad2-8aab-4c928298ee0chttp://schemas.xmlsoap.org/ws/2004/08/addressing/role/anonymoushttp://schemas.xmlsoap.org/ws/2005/04/discovery/ProbeMatchesurn:uuid:10919da4-5566-7788-99aa-0012414fb745dn:NetworkVideoTransmitteronvif://www.onvif.org/type/video_encoder onvif://www.onvif.org/type/audio_encoder onvif://www.onvif.org/hardware/IPC-model onvif://www.onvif.org/location/country/china onvif://www.onvif.org/name/NVT onvif://www.onvif.org/Profile/Streaming {}10", - &uris.join(" ") - ); + let response = get_expected_probe_match_message(&uris); assert_eq!( uris, get_scope_filtered_uris_from_discovery_response(&response, None) ); } + + #[test] + fn test_get_scope_filtered_uris_from_discovery_response_include_scope_exist() { + let _ = env_logger::builder().is_test(true).try_init(); + + let filter_list = FilterList { + action: FilterType::Include, + items: vec!["onvif://www.onvif.org/name/NVT".to_string()], + }; + let uris = vec!["uri_one".to_string(), "uri_two".to_string()]; + let response = get_expected_probe_match_message(&uris); + assert_eq!( + uris, + get_scope_filtered_uris_from_discovery_response( + &response, + Some(filter_list).as_ref() + ) + ); + } + + #[test] + fn test_get_scope_filtered_uris_from_discovery_response_exclude_scope_exist() { + let _ = env_logger::builder().is_test(true).try_init(); + + let filter_list = FilterList { + action: FilterType::Exclude, + items: vec!["onvif://www.onvif.org/name/NVT".to_string()], + }; + let uris = vec!["uri_one".to_string(), "uri_two".to_string()]; + let response = get_expected_probe_match_message(&uris); + assert!(get_scope_filtered_uris_from_discovery_response( + &response, + Some(filter_list).as_ref() + ) + .is_empty()); + } + + #[test] + fn test_get_scope_filtered_uris_from_discovery_response_include_scope_nonexist() { + let _ = env_logger::builder().is_test(true).try_init(); + + let filter_list = FilterList { + action: FilterType::Include, + items: vec!["onvif://www.onvif.org/name/NVT123".to_string()], + }; + let uris = vec!["uri_one".to_string(), "uri_two".to_string()]; + let response = get_expected_probe_match_message(&uris); + assert!(get_scope_filtered_uris_from_discovery_response( + &response, + Some(filter_list).as_ref() + ) + .is_empty()); + } + + #[test] + fn test_get_scope_filtered_uris_from_discovery_response_exclude_scope_nonexist() { + let _ = env_logger::builder().is_test(true).try_init(); + + let filter_list = FilterList { + action: FilterType::Exclude, + items: vec!["onvif://www.onvif.org/name/NVT123".to_string()], + }; + let uris = vec!["uri_one".to_string(), "uri_two".to_string()]; + let response = get_expected_probe_match_message(&uris); + assert_eq!( + uris, + get_scope_filtered_uris_from_discovery_response( + &response, + Some(filter_list).as_ref() + ) + ); + } + + #[test] + fn test_get_scope_filtered_uris_from_discovery_response_include_scope_similar() { + let _ = env_logger::builder().is_test(true).try_init(); + + let filter_list = FilterList { + action: FilterType::Include, + items: vec!["onvif://www.onvif.org/name".to_string()], + }; + let uris = vec!["uri_one".to_string(), "uri_two".to_string()]; + let response = get_expected_probe_match_message(&uris); + assert!(get_scope_filtered_uris_from_discovery_response( + &response, + Some(filter_list).as_ref() + ) + .is_empty()); + } + + #[test] + fn test_get_scope_filtered_uris_from_discovery_response_exclude_scope_similar() { + let _ = env_logger::builder().is_test(true).try_init(); + + let filter_list = FilterList { + action: FilterType::Exclude, + items: vec!["onvif://www.onvif.org/name".to_string()], + }; + let uris = vec!["uri_one".to_string(), "uri_two".to_string()]; + let response = get_expected_probe_match_message(&uris); + assert_eq!( + uris, + get_scope_filtered_uris_from_discovery_response( + &response, + Some(filter_list).as_ref() + ) + ); + } } pub async fn get_discovery_response_socket() -> Result { diff --git a/discovery-handlers/opcua/Cargo.toml b/discovery-handlers/opcua/Cargo.toml index 3d0e215b0..8e88325d8 100644 --- a/discovery-handlers/opcua/Cargo.toml +++ b/discovery-handlers/opcua/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-opcua" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-handlers/udev/Cargo.toml b/discovery-handlers/udev/Cargo.toml index de85db4fd..257956d2f 100644 --- a/discovery-handlers/udev/Cargo.toml +++ b/discovery-handlers/udev/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-udev" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-utils/Cargo.toml b/discovery-utils/Cargo.toml index b7fd76d72..03a4c9cd4 100644 --- a/discovery-utils/Cargo.toml +++ b/discovery-utils/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-discovery-utils" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/samples/brokers/udev-video-broker/Cargo.toml b/samples/brokers/udev-video-broker/Cargo.toml index f06700995..1843236b6 100644 --- a/samples/brokers/udev-video-broker/Cargo.toml +++ b/samples/brokers/udev-video-broker/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "udev-video-broker" -version = "0.10.13" +version = "0.10.14" authors = ["Kate Goldenring ", ""] edition = "2018" rust-version = "1.68.1" diff --git a/shared/Cargo.toml b/shared/Cargo.toml index 9f91b328e..434c95cf6 100644 --- a/shared/Cargo.toml +++ b/shared/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-shared" -version = "0.10.13" +version = "0.10.14" authors = [""] edition = "2018" rust-version = "1.68.1" diff --git a/version.txt b/version.txt index f25c43cdc..c70613aa0 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.10.13 +0.10.14 diff --git a/webhooks/validating/configuration/Cargo.toml b/webhooks/validating/configuration/Cargo.toml index 173da4bc3..86f5701ef 100644 --- a/webhooks/validating/configuration/Cargo.toml +++ b/webhooks/validating/configuration/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "webhook-configuration" -version = "0.10.13" +version = "0.10.14" authors = ["DazWilkin "] edition = "2018" rust-version = "1.68.1"