From 4e2aec2673ae774c91fdb7d253415057aa17e8cd Mon Sep 17 00:00:00 2001 From: Johnson Shih Date: Mon, 6 Mar 2023 01:27:11 -0800 Subject: [PATCH 1/7] Support discoveryProperties in Configuration Signed-off-by: Johnson Shih --- agent/src/util/discovery_operator.rs | 1109 ++++++++++++++++- .../helm/crds/akri-configuration-crd.yaml | 6 + .../debug-echo/src/discovery_handler.rs | 1 + discovery-utils/proto/discovery.proto | 7 + discovery-utils/src/discovery/mod.rs | 2 + discovery-utils/src/discovery/v0.rs | 9 + shared/src/akri/configuration.rs | 4 + 7 files changed, 1121 insertions(+), 17 deletions(-) diff --git a/agent/src/util/discovery_operator.rs b/agent/src/util/discovery_operator.rs index 63c552e17..85357bd75 100644 --- a/agent/src/util/discovery_operator.rs +++ b/agent/src/util/discovery_operator.rs @@ -12,7 +12,8 @@ use super::{ streaming_extension::StreamingExt, }; use akri_discovery_utils::discovery::v0::{ - discovery_handler_client::DiscoveryHandlerClient, Device, DiscoverRequest, DiscoverResponse, + discovery_handler_client::DiscoveryHandlerClient, ByteData, Device, DiscoverRequest, + DiscoverResponse, }; use akri_shared::{ akri::configuration::Configuration, @@ -23,11 +24,16 @@ use blake2::{ digest::{Update, VariableOutput}, VarBlake2b, }; +use k8s_openapi::api::core::v1::{ + ConfigMap, ConfigMapKeySelector, EnvVar, EnvVarSource, Secret, SecretKeySelector, +}; +use kube::api::{Api, ListParams}; use log::{error, trace}; #[cfg(test)] use mock_instant::Instant; #[cfg(test)] use mockall::{automock, predicate::*}; +use std::io::{Error, ErrorKind}; #[cfg(not(test))] use std::time::Instant; use std::{collections::HashMap, convert::TryFrom, sync::Arc}; @@ -104,9 +110,30 @@ impl DiscoveryOperator { } /// Calls discover on the Discovery Handler at the given endpoint and returns the connection stream. - pub async fn get_stream(&self, endpoint: &DiscoveryHandlerEndpoint) -> Option { + pub async fn get_stream<'a>( + &'a self, + kube_interface: Arc, + endpoint: &'a DiscoveryHandlerEndpoint, + ) -> Option { + let discovery_properties = match self + .get_discovery_properties( + kube_interface.clone(), + &self.config.spec.discovery_handler.discovery_properties, + ) + .await + { + Ok(data) => data, + Err(e) => { + error!( + "get_stream - fail to retrieve discovery properties for Configuration {:?}, error {:?}", + self.config.metadata.name, e + ); + return None; + } + }; let discover_request = tonic::Request::new(DiscoverRequest { discovery_details: self.config.spec.discovery_handler.discovery_details.clone(), + discovery_properties, }); trace!("get_stream - endpoint is {:?}", endpoint); match endpoint { @@ -450,6 +477,234 @@ impl DiscoveryOperator { } Ok(()) } + + async fn get_discovery_properties( + &self, + kube_interface: Arc, + properties: &Option>, + ) -> anyhow::Result> { + match properties { + None => Ok(HashMap::new()), + Some(properties) => { + let mut tmp_properties = HashMap::new(); + for p in properties { + match self.get_discovery_property(kube_interface.clone(), p).await { + Ok(tmp_p) => { + if let Some((k, v)) = tmp_p { + tmp_properties.insert(k, v.clone()); + } + } + Err(e) => return Err(e), + } + } + Ok(tmp_properties) + } + } + } + + async fn get_discovery_property( + &self, + kube_interface: Arc, + property: &EnvVar, + ) -> anyhow::Result> { + let value; + if let Some(v) = &property.value { + value = ByteData { + vec: Some(v.as_bytes().to_vec()), + }; + } else if let Some(value_from) = &property.value_from { + let kube_client = ActualKubeClient::new(kube_interface.clone()); + value = match self + .get_discovery_property_value_from(&kube_client, value_from) + .await + { + Ok(byte_data) => { + if byte_data.is_none() { + // optional value, not found + return Ok(None); + } + byte_data.unwrap() + } + Err(e) => return Err(e), + }; + } else { + // property without value + value = ByteData { vec: None } + } + + Ok(Some((property.name.clone(), value))) + } + + async fn get_discovery_property_value_from( + &self, + kube_client: &dyn KubeClient, + property: &EnvVarSource, + ) -> anyhow::Result> { + if let Some(config_map_key_ref) = &property.config_map_key_ref { + return get_discovery_property_value_from_config_map( + kube_client, + self.config.metadata.namespace.as_ref().unwrap(), + config_map_key_ref, + ) + .await; + } + if let Some(secret_key_selector) = &property.secret_key_ref { + get_discovery_property_value_from_secret( + kube_client, + self.config.metadata.namespace.as_ref().unwrap(), + secret_key_selector, + ) + .await + } else { + let error = Error::new(ErrorKind::InvalidInput, "no supported value_from found"); + Err(error.into()) + } + } +} + +/// This provides a mockable way to query configMap and secret +#[cfg_attr(test, automock)] +#[tonic::async_trait] +pub trait KubeClient: Send + Sync { + async fn get_secret(&self, name: &str, namespace: &str) -> anyhow::Result>; + + async fn get_config_map( + &self, + name: &str, + namespace: &str, + ) -> anyhow::Result>; +} + +struct ActualKubeClient { + pub kube_interface: Arc, +} + +impl ActualKubeClient { + pub fn new(kube_interface: Arc) -> Self { + ActualKubeClient { kube_interface } + } +} + +#[tonic::async_trait] +impl KubeClient for ActualKubeClient { + async fn get_secret(&self, name: &str, namespace: &str) -> anyhow::Result> { + let resource_client: Api = + Api::namespaced(self.kube_interface.get_kube_client(), namespace); + let lp = ListParams::default(); + for s in resource_client.list(&lp).await? { + if name == s.metadata.name.as_ref().unwrap() { + return Ok(Some(s)); + } + } + Ok(None) + } + + async fn get_config_map( + &self, + name: &str, + namespace: &str, + ) -> anyhow::Result> { + let resource_client: Api = + Api::namespaced(self.kube_interface.get_kube_client(), namespace); + let lp = ListParams::default(); + for s in resource_client.list(&lp).await? { + if name == s.metadata.name.as_ref().unwrap() { + return Ok(Some(s)); + } + } + Ok(None) + } +} + +async fn get_discovery_property_value_from_secret( + kube_client: &dyn KubeClient, + namespace: &str, + secret_key_selector: &SecretKeySelector, +) -> anyhow::Result> { + let optional = secret_key_selector.optional.unwrap_or_default(); + if secret_key_selector.name.is_none() { + if optional { + return Ok(None); + } + return Err(Error::new(ErrorKind::InvalidInput, "secret name is none").into()); + } + let secret_key = &secret_key_selector.key; + let secret_name = secret_key_selector.name.as_ref().unwrap(); + + let secret = kube_client.get_secret(secret_name, namespace).await?; + if secret.is_none() { + if optional { + return Ok(None); + } else { + return Err(Error::new(ErrorKind::InvalidInput, "secret not found").into()); + } + } + let secret = secret.unwrap(); + // All key-value pairs in the stringData field are internally merged into the data field + // we don't need to check string_data. + if let Some(data) = secret.data { + if let Some(v) = data.get(secret_key) { + return Ok(Some(ByteData { + vec: Some(v.0.clone()), + })); + } + } + + // secret key/value not found + if optional { + Ok(None) + } else { + Err(Error::new(ErrorKind::InvalidInput, "secret data not found").into()) + } +} + +async fn get_discovery_property_value_from_config_map( + kube_client: &dyn KubeClient, + namespace: &str, + config_map_key_selector: &ConfigMapKeySelector, +) -> anyhow::Result> { + let optional = config_map_key_selector.optional.unwrap_or_default(); + if config_map_key_selector.name.is_none() { + if optional { + return Ok(None); + } + return Err(Error::new(ErrorKind::InvalidInput, "config_map name is none").into()); + } + let config_map_name = config_map_key_selector.name.as_ref().unwrap(); + let config_map_key = &config_map_key_selector.key; + + let config_map = kube_client + .get_config_map(config_map_name, namespace) + .await?; + if config_map.is_none() { + if optional { + return Ok(None); + } else { + return Err(Error::new(ErrorKind::InvalidInput, "config_map not found").into()); + } + } + let config_map = config_map.unwrap(); + if let Some(data) = config_map.data { + if let Some(v) = data.get(config_map_key) { + return Ok(Some(ByteData { + vec: Some(v.as_bytes().to_vec()), + })); + } + } + if let Some(binary_data) = config_map.binary_data { + if let Some(v) = binary_data.get(config_map_key) { + return Ok(Some(ByteData { + vec: Some(v.0.clone()), + })); + } + } + + // config_map_key/value not found + if optional { + Ok(None) + } else { + Err(Error::new(ErrorKind::InvalidInput, "config_map data not found").into()) + } } pub mod start_discovery { @@ -630,7 +885,10 @@ pub mod start_discovery { dh_details: &'a DiscoveryDetails, ) -> anyhow::Result<()> { loop { - if let Some(stream_type) = discovery_operator.get_stream(endpoint).await { + if let Some(stream_type) = discovery_operator + .get_stream(kube_interface.clone(), endpoint) + .await + { match stream_type { StreamType::External(mut stream) => { match discovery_operator @@ -749,8 +1007,10 @@ pub mod tests { use akri_shared::{ akri::configuration::Configuration, k8s::MockKubeInterface, os::env_var::MockEnvVarQuery, }; + use k8s_openapi::ByteString; use mock_instant::{Instant, MockClock}; use mockall::Sequence; + use std::collections::BTreeMap; use std::time::Duration; use tokio::sync::{broadcast, mpsc}; @@ -1028,7 +1288,7 @@ pub mod tests { let (running_sender, running_receiver) = tokio::sync::broadcast::channel::<()>(1); mock_discovery_operator .expect_get_stream() - .returning(move |_| { + .returning(move |_, _| { running_sender.clone().send(()).unwrap(); None }); @@ -1089,7 +1349,7 @@ pub mod tests { mock_discovery_operator .expect_get_stream() .times(1) - .returning(|_| None) + .returning(|_, _| None) .in_sequence(&mut get_stream_seq); // Second time successfully get stream let (_, rx) = mpsc::channel(2); @@ -1097,7 +1357,7 @@ pub mod tests { mock_discovery_operator .expect_get_stream() .times(1) - .return_once(move |_| stream_type) + .return_once(move |_, _| stream_type) .in_sequence(&mut get_stream_seq); // Discovery should be initiated mock_discovery_operator @@ -1467,15 +1727,10 @@ pub mod tests { .unwrap(); } - fn setup_non_mocked_dh( - dh_name: &str, - endpoint: &DiscoveryHandlerEndpoint, - ) -> DiscoveryOperator { - let path_to_config = "../test/yaml/config-a.yaml"; + fn create_discovery_operator(path_to_config: &str) -> DiscoveryOperator { let config_yaml = std::fs::read_to_string(path_to_config).expect("Unable to read file"); let config: Configuration = serde_yaml::from_str(&config_yaml).unwrap(); let discovery_handler_map = Arc::new(std::sync::Mutex::new(HashMap::new())); - add_discovery_handler_to_map(dh_name, endpoint, false, discovery_handler_map.clone()); DiscoveryOperator::new( discovery_handler_map, config, @@ -1483,6 +1738,20 @@ pub mod tests { ) } + fn setup_non_mocked_dh( + dh_name: &str, + endpoint: &DiscoveryHandlerEndpoint, + ) -> DiscoveryOperator { + let discovery_operator = create_discovery_operator("../test/yaml/config-a.yaml"); + add_discovery_handler_to_map( + dh_name, + endpoint, + false, + discovery_operator.discovery_handler_map.clone(), + ); + discovery_operator + } + #[tokio::test] async fn test_get_stream_embedded() { let _ = env_logger::builder().is_test(true).try_init(); @@ -1499,9 +1768,11 @@ pub mod tests { config, Arc::new(tokio::sync::RwLock::new(HashMap::new())), ); + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + // test embedded debugEcho socket if let Some(StreamType::Embedded(_)) = discovery_operator - .get_stream(&DiscoveryHandlerEndpoint::Embedded) + .get_stream(mock_kube_interface, &DiscoveryHandlerEndpoint::Embedded) .await { // expected @@ -1538,8 +1809,13 @@ pub mod tests { mock_discovery_handler::get_mock_discovery_handler_dir_and_endpoint("mock.sock"); let dh_endpoint = DiscoveryHandlerEndpoint::Uds(endpoint.to_string()); let discovery_operator = setup_non_mocked_dh("mock", &dh_endpoint); + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + // Should not be able to get stream if DH is not running - assert!(discovery_operator.get_stream(&dh_endpoint).await.is_none()); + assert!(discovery_operator + .get_stream(mock_kube_interface, &dh_endpoint) + .await + .is_none()); } #[tokio::test] @@ -1556,8 +1832,13 @@ pub mod tests { return_error, ) .await; + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + // Assert that get_stream returns none if the DH returns error - assert!(discovery_operator.get_stream(&dh_endpoint).await.is_none()); + assert!(discovery_operator + .get_stream(mock_kube_interface, &dh_endpoint) + .await + .is_none()); } #[tokio::test] @@ -1574,8 +1855,11 @@ pub mod tests { return_error, ) .await; - if let Some(StreamType::External(mut receiver)) = - discovery_operator.get_stream(&dh_endpoint).await + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + + if let Some(StreamType::External(mut receiver)) = discovery_operator + .get_stream(mock_kube_interface, &dh_endpoint) + .await { // MockDiscoveryHandler returns an empty array of devices assert_eq!( @@ -1586,4 +1870,795 @@ pub mod tests { panic!("expected external stream"); } } + + #[tokio::test] + async fn test_get_discovery_properties_no_properties() { + let _ = env_logger::builder().is_test(true).try_init(); + let discovery_operator = create_discovery_operator("../test/yaml/config-a.yaml"); + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + + // properties should be empty if not specified + assert!(discovery_operator + .get_discovery_properties(mock_kube_interface, &None) + .await + .unwrap() + .is_empty()); + } + + #[tokio::test] + async fn test_get_discovery_properties_empty_property_list() { + let _ = env_logger::builder().is_test(true).try_init(); + let discovery_operator = create_discovery_operator("../test/yaml/config-a.yaml"); + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + let properties = Vec::::new(); + + // properties should be empty if property list is empty + assert!(discovery_operator + .get_discovery_properties(mock_kube_interface, &Some(properties)) + .await + .unwrap() + .is_empty()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_no_value() { + let _ = env_logger::builder().is_test(true).try_init(); + let discovery_operator = create_discovery_operator("../test/yaml/config-a.yaml"); + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + let property_name_1 = "property_name_1".to_string(); + let property_name_2 = "".to_string(); // allow empty property name + let properties = vec![ + EnvVar { + name: property_name_1.clone(), + value: None, + value_from: None, + }, + EnvVar { + name: property_name_2.clone(), + value: None, + value_from: None, + }, + ]; + let expected_result = HashMap::from([ + (property_name_1, ByteData { vec: None }), + (property_name_2, ByteData { vec: None }), + ]); + + // properties should only contain (name, None) if no value specified + let result = discovery_operator + .get_discovery_properties(mock_kube_interface.clone(), &Some(properties)) + .await + .unwrap(); + assert_eq!(result, expected_result); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_with_value() { + let _ = env_logger::builder().is_test(true).try_init(); + let discovery_operator = create_discovery_operator("../test/yaml/config-a.yaml"); + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + let property_name_1 = "property_name_1".to_string(); + let property_name_2 = "".to_string(); // allow empty property name + let property_value_1 = "property_value_1".to_string(); + let property_value_2 = "property_value_2".to_string(); + let properties = vec![ + EnvVar { + name: property_name_1.clone(), + value: Some(property_value_1.clone()), + value_from: None, + }, + EnvVar { + name: property_name_2.clone(), + value: Some(property_value_2.clone()), + value_from: None, + }, + ]; + let expected_result = HashMap::from([ + ( + property_name_1, + ByteData { + vec: Some(property_value_1.into()), + }, + ), + ( + property_name_2, + ByteData { + vec: Some(property_value_2.into()), + }, + ), + ]); + + // properties should contains (name, value) if specified + let result = discovery_operator + .get_discovery_properties(mock_kube_interface, &Some(properties)) + .await + .unwrap(); + assert_eq!(result, expected_result); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_unknown_ref() { + let _ = env_logger::builder().is_test(true).try_init(); + let discovery_operator = create_discovery_operator("../test/yaml/config-a.yaml"); + let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); + let property_name = "property_name_1".to_string(); + let property_value_from = EnvVarSource::default(); + let properties = vec![EnvVar { + name: property_name.clone(), + value: None, + value_from: Some(property_value_from), + }]; + + // get_discovery_properties should return error if no supported key ref specified + assert!(discovery_operator + .get_discovery_properties(mock_kube_interface, &Some(properties)) + .await + .is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_secret_no_secret_name() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + + let selector = SecretKeySelector::default(); + + let mock_kube_client = MockKubeClient::new(); + + // get_discovery_property_value_from_secret should return error if mandatory secret name is empty + let result = + get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_secret_name_optional() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + + let selector = SecretKeySelector { + key: String::default(), + name: None, + optional: Some(true), + }; + + let mock_kube_client = MockKubeClient::new(); + + // get_discovery_property_value_from_secret should return empty if optional secret name is empty + let result = + get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) + .await; + assert!(result.unwrap().is_none()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_secret_no_secret_found() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let secret_name = "secret_1"; + let key_in_secret = "key_in_secret"; + + let selector = SecretKeySelector { + key: key_in_secret.to_string(), + name: Some(secret_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_secret() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == secret_name + }) + .returning(move |_, _| Ok(None)); + + // get_discovery_property_value_from_secret should return error if no secret not found + let result = + get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_secret_no_secret_found_optional() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let secret_name = "secret_1"; + let key_in_secret = "key_in_secret"; + + let selector = SecretKeySelector { + key: key_in_secret.to_string(), + name: Some(secret_name.to_string()), + optional: Some(true), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_secret() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == secret_name + }) + .returning(move |_, _| Ok(None)); + + // get_discovery_property_value_from_secret for an optional key should return None if secret not found + let result = + get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) + .await; + assert!(result.unwrap().is_none()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_secret_no_key() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let secret_name = "secret_1"; + let key_in_secret = "key_in_secret"; + + let selector = SecretKeySelector { + key: key_in_secret.to_string(), + name: Some(secret_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_secret() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == secret_name + }) + .returning(move |_, _| Ok(Some(Secret::default()))); + + // get_discovery_property_value_from_secret should return error if key in secret not found + assert!(get_discovery_property_value_from_secret( + &mock_kube_client, + namespace_name, + &selector, + ) + .await + .is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_secret_no_key_optional() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let secret_name = "secret_1"; + let key_in_secret = "key_in_config_map"; + + let selector = SecretKeySelector { + key: key_in_secret.to_string(), + name: Some(secret_name.to_string()), + optional: Some(true), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_secret() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == secret_name + }) + .returning(move |_, _| Ok(Some(Secret::default()))); + + // get_discovery_property_value_from_secret for an optional key should return None if key in secret not found + let result = + get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) + .await; + assert!(result.unwrap().is_none()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_secret_no_value() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let secret_name = "secret_1"; + let key_in_secret = "key_in_secret"; + + let selector = SecretKeySelector { + key: key_in_secret.to_string(), + name: Some(secret_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_secret() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == secret_name + }) + .returning(move |_, _| { + let secret = Secret { + data: Some(BTreeMap::new()), + ..Default::default() + }; + Ok(Some(secret)) + }); + + // get_discovery_property_value_from_secret should return error if no value in secret + let result = + get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_secret_no_value_optional() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let secret_name = "secret_1"; + let key_in_secret = "key_in_config_map"; + + let selector = SecretKeySelector { + key: key_in_secret.to_string(), + name: Some(secret_name.to_string()), + optional: Some(true), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_secret() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == secret_name + }) + .returning(move |_, _| { + let secret = Secret { + data: Some(BTreeMap::new()), + ..Default::default() + }; + Ok(Some(secret)) + }); + + // get_discovery_property_value_from_secret for an optional key should return None if key in secret not found + let result = + get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) + .await; + assert!(result.unwrap().is_none()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_secret_data_value() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let secret_name = "secret_1"; + let key_in_secret = "key_in_secret"; + let value_in_secret = "value_in_secret"; + + let selector = SecretKeySelector { + key: key_in_secret.to_string(), + name: Some(secret_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_secret() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == secret_name + }) + .returning(move |_, _| { + let data = BTreeMap::from([( + key_in_secret.to_string(), + ByteString(value_in_secret.into()), + )]); + let secret = Secret { + data: Some(data), + ..Default::default() + }; + Ok(Some(secret)) + }); + + let expected_result = ByteData { + vec: Some(value_in_secret.into()), + }; + + // get_discovery_property_value_from_secret should return correct value if data value in secret + let result = + get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) + .await; + assert_eq!(result.unwrap().unwrap(), expected_result); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_config_map_name() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + + let selector = ConfigMapKeySelector::default(); + + let mock_kube_client = MockKubeClient::new(); + + // get_discovery_property_value_from_config_map should return error if mandatory configMap name is empty + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_config_map_name_optional() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + + let selector = ConfigMapKeySelector { + key: String::default(), + name: None, + optional: Some(true), + }; + + let mock_kube_client = MockKubeClient::new(); + + // get_discovery_property_value_from_config_map should return empty if optional configMap name is empty + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert!(result.unwrap().is_none()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_config_map_found() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| Ok(None)); + + // get_discovery_property_value_from_config_map should return error if no configMap not found + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_config_map_found_optional() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(true), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| Ok(None)); + + // get_discovery_property_value_from_config_map for an optional key should return None if configMap not found + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert!(result.unwrap().is_none()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_key() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| Ok(Some(ConfigMap::default()))); + + // get_discovery_property_value_from_config_map should return error if key in configMap not found + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_key_optional() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(true), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| Ok(Some(ConfigMap::default()))); + + // get_discovery_property_value_from_config_map for an optional key should return None if key in configMap not found + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert!(result.unwrap().is_none()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_value() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| { + let config_map = ConfigMap { + data: Some(BTreeMap::new()), + binary_data: Some(BTreeMap::new()), + ..Default::default() + }; + Ok(Some(config_map)) + }); + + // get_discovery_property_value_from_config_map should return error if no value in configMap + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_no_value_optional() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(true), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| { + let config_map = ConfigMap { + data: Some(BTreeMap::new()), + binary_data: Some(BTreeMap::new()), + ..Default::default() + }; + Ok(Some(config_map)) + }); + + // get_discovery_property_value_from_config_map for an optional key should return None if key in configMap not found + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert!(result.unwrap().is_none()); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_data_value() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + let value_in_config_map = "value_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| { + let data = BTreeMap::from([( + key_in_config_map.to_string(), + value_in_config_map.to_string(), + )]); + let config_map = ConfigMap { + data: Some(data), + binary_data: Some(BTreeMap::new()), + ..Default::default() + }; + Ok(Some(config_map)) + }); + + let expected_result = ByteData { + vec: Some(value_in_config_map.into()), + }; + + // get_discovery_property_value_from_config_map should return correct value if data value in configMap + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert_eq!(result.unwrap().unwrap(), expected_result); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_binary_data_value() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + let value_in_config_map = "value_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| { + let binary_data = BTreeMap::from([( + key_in_config_map.to_string(), + ByteString(value_in_config_map.into()), + )]); + let config_map = ConfigMap { + data: Some(BTreeMap::new()), + binary_data: Some(binary_data), + ..Default::default() + }; + Ok(Some(config_map)) + }); + + let expected_result = ByteData { + vec: Some(value_in_config_map.into()), + }; + + // get_discovery_property_value_from_config_map should return correct value if binary data value in configMap + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert_eq!(result.unwrap().unwrap(), expected_result); + } + + #[tokio::test] + async fn test_get_discovery_properties_value_from_config_map_data_and_binary_data_value() { + let _ = env_logger::builder().is_test(true).try_init(); + let namespace_name = "namespace_name"; + let config_map_name = "config_map_1"; + let key_in_config_map = "key_in_config_map"; + let value_in_config_map = "value_in_config_map"; + let binary_value_in_config_map = "binary_value_in_config_map"; + + let selector = ConfigMapKeySelector { + key: key_in_config_map.to_string(), + name: Some(config_map_name.to_string()), + optional: Some(false), + }; + + let mut mock_kube_client = MockKubeClient::new(); + mock_kube_client + .expect_get_config_map() + .times(1) + .withf(move |name: &str, namespace: &str| { + namespace == namespace_name && name == config_map_name + }) + .returning(move |_, _| { + let data = BTreeMap::from([( + key_in_config_map.to_string(), + value_in_config_map.to_string(), + )]); + let binary_data = BTreeMap::from([( + key_in_config_map.to_string(), + ByteString(binary_value_in_config_map.into()), + )]); + let config_map = ConfigMap { + data: Some(data), + binary_data: Some(binary_data), + ..Default::default() + }; + Ok(Some(config_map)) + }); + + let expected_result = ByteData { + vec: Some(value_in_config_map.into()), + }; + + // get_discovery_property_value_from_config_map should return value from data if both data and binary data value exist + let result = get_discovery_property_value_from_config_map( + &mock_kube_client, + namespace_name, + &selector, + ) + .await; + assert_eq!(result.unwrap().unwrap(), expected_result); + } } diff --git a/deployment/helm/crds/akri-configuration-crd.yaml b/deployment/helm/crds/akri-configuration-crd.yaml index 7ba8a38fb..4189109a2 100644 --- a/deployment/helm/crds/akri-configuration-crd.yaml +++ b/deployment/helm/crds/akri-configuration-crd.yaml @@ -22,6 +22,12 @@ spec: type: string discoveryDetails: type: string + discoveryProperties: + nullable: true + type: array + items: # {{EnvVar}} + x-kubernetes-preserve-unknown-fields: true + type: object capacity: type: integer brokerSpec: # {{BrokerSpec}} diff --git a/discovery-handlers/debug-echo/src/discovery_handler.rs b/discovery-handlers/debug-echo/src/discovery_handler.rs index 849ae8caa..4e016054b 100644 --- a/discovery-handlers/debug-echo/src/discovery_handler.rs +++ b/discovery-handlers/debug-echo/src/discovery_handler.rs @@ -200,6 +200,7 @@ mod tests { }; let discover_request = tonic::Request::new(DiscoverRequest { discovery_details: deserialized.discovery_details.clone(), + discovery_properties: HashMap::new(), }); let mut stream = discovery_handler .discover(discover_request) diff --git a/discovery-utils/proto/discovery.proto b/discovery-utils/proto/discovery.proto index cfc14d968..ee279a394 100644 --- a/discovery-utils/proto/discovery.proto +++ b/discovery-utils/proto/discovery.proto @@ -33,10 +33,17 @@ service DiscoveryHandler { rpc Discover (DiscoverRequest) returns (stream DiscoverResponse); } +message ByteData { + optional bytes vec = 1; +} + message DiscoverRequest { // String containing all the details (such as filtering options) // the `DiscoveryHandler` needs to find a set of devices. string discovery_details = 1; + // list of Key-value pairs containing additional information + // for the 'DiscoveryHandler' to discover devices + map discovery_properties = 2; } message DiscoverResponse { diff --git a/discovery-utils/src/discovery/mod.rs b/discovery-utils/src/discovery/mod.rs index 29a00134f..874633b13 100644 --- a/discovery-utils/src/discovery/mod.rs +++ b/discovery-utils/src/discovery/mod.rs @@ -260,6 +260,7 @@ pub mod server { v0::{discovery_handler_client::DiscoveryHandlerClient, DiscoverRequest}, }; use super::*; + use std::collections::HashMap; use std::convert::TryFrom; use tempfile::Builder; use tokio::net::UnixStream; @@ -290,6 +291,7 @@ pub mod server { let mut stream = discovery_handler_client .discover(Request::new(DiscoverRequest { discovery_details: String::new(), + discovery_properties: HashMap::new(), })) .await .unwrap() diff --git a/discovery-utils/src/discovery/v0.rs b/discovery-utils/src/discovery/v0.rs index f21bb2105..1e3a7d849 100644 --- a/discovery-utils/src/discovery/v0.rs +++ b/discovery-utils/src/discovery/v0.rs @@ -30,11 +30,20 @@ pub mod register_discovery_handler_request { #[derive(Clone, PartialEq, ::prost::Message)] pub struct Empty {} #[derive(Clone, PartialEq, ::prost::Message)] +pub struct ByteData { + #[prost(bytes = "vec", optional, tag = "1")] + pub vec: ::core::option::Option<::prost::alloc::vec::Vec>, +} +#[derive(Clone, PartialEq, ::prost::Message)] pub struct DiscoverRequest { /// String containing all the details (such as filtering options) /// the `DiscoveryHandler` needs to find a set of devices. #[prost(string, tag = "1")] pub discovery_details: ::prost::alloc::string::String, + /// list of Key-value pairs containing additional information + /// for the 'DiscoveryHandler' to discover devices + #[prost(map = "string, message", tag = "2")] + pub discovery_properties: ::std::collections::HashMap<::prost::alloc::string::String, ByteData>, } #[derive(Clone, PartialEq, ::prost::Message)] pub struct DiscoverResponse { diff --git a/shared/src/akri/configuration.rs b/shared/src/akri/configuration.rs index 4bd3db7ca..74937b445 100644 --- a/shared/src/akri/configuration.rs +++ b/shared/src/akri/configuration.rs @@ -5,6 +5,7 @@ // #![allow(non_camel_case_types)] use k8s_openapi::api::batch::v1::JobSpec; +use k8s_openapi::api::core::v1::EnvVar; use k8s_openapi::api::core::v1::PodSpec; use k8s_openapi::api::core::v1::ServiceSpec; use kube::CustomResource; @@ -27,6 +28,9 @@ pub struct DiscoveryHandlerInfo { /// A string that a Discovery Handler knows how to parse to obtain necessary discovery details #[serde(default)] pub discovery_details: String, + + #[serde(default, skip_serializing_if = "Option::is_none")] + pub discovery_properties: Option>, } /// This defines a workload that should be scheduled to nodes From 368b030baa75526b91ad5daf77cd5a96ba791206 Mon Sep 17 00:00:00 2001 From: Johnson Shih Date: Wed, 14 Jun 2023 11:51:52 -0700 Subject: [PATCH 2/7] Update version 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 +- 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 +- 17 files changed, 31 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6937e08ee..fbdaed0be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -333,7 +333,7 @@ checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" [[package]] name = "agent" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-debug-echo", "akri-discovery-utils", @@ -402,7 +402,7 @@ dependencies = [ [[package]] name = "akri-debug-echo" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-discovery-utils", "akri-shared", @@ -422,7 +422,7 @@ dependencies = [ [[package]] name = "akri-discovery-utils" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-shared", "anyhow", @@ -444,7 +444,7 @@ dependencies = [ [[package]] name = "akri-onvif" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-discovery-utils", "akri-shared", @@ -472,7 +472,7 @@ dependencies = [ [[package]] name = "akri-opcua" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-discovery-utils", "akri-shared", @@ -496,7 +496,7 @@ dependencies = [ [[package]] name = "akri-shared" -version = "0.10.11" +version = "0.11.0" dependencies = [ "anyhow", "async-trait", @@ -525,7 +525,7 @@ dependencies = [ [[package]] name = "akri-udev" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-discovery-utils", "anyhow", @@ -1043,7 +1043,7 @@ checksum = "fbdcdcb6d86f71c5e97409ad45898af11cbc995b4ee8112d59095a28d376c935" [[package]] name = "controller" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-shared", "anyhow", @@ -1243,7 +1243,7 @@ dependencies = [ [[package]] name = "debug-echo-discovery-handler" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-debug-echo", "akri-discovery-utils", @@ -2540,7 +2540,7 @@ checksum = "b7e5500299e16ebb147ae15a00a942af264cf3688f47923b8fc2cd5858f23ad3" [[package]] name = "onvif-discovery-handler" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-discovery-utils", "akri-onvif", @@ -2590,7 +2590,7 @@ dependencies = [ [[package]] name = "opcua-discovery-handler" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-discovery-utils", "akri-opcua", @@ -4206,7 +4206,7 @@ dependencies = [ [[package]] name = "udev-discovery-handler" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-discovery-utils", "akri-udev", @@ -4217,7 +4217,7 @@ dependencies = [ [[package]] name = "udev-video-broker" -version = "0.10.11" +version = "0.11.0" dependencies = [ "akri-shared", "env_logger", @@ -4494,7 +4494,7 @@ dependencies = [ [[package]] name = "webhook-configuration" -version = "0.10.11" +version = "0.11.0" dependencies = [ "actix", "actix-rt 2.7.0", diff --git a/agent/Cargo.toml b/agent/Cargo.toml index babd8c194..c08d4b866 100644 --- a/agent/Cargo.toml +++ b/agent/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "agent" -version = "0.10.11" +version = "0.11.0" authors = ["Kate Goldenring ", ""] edition = "2018" rust-version = "1.68.1" diff --git a/controller/Cargo.toml b/controller/Cargo.toml index 91bf6f504..b28c42634 100644 --- a/controller/Cargo.toml +++ b/controller/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "controller" -version = "0.10.11" +version = "0.11.0" authors = ["", ""] edition = "2018" rust-version = "1.68.1" diff --git a/deployment/helm/Chart.yaml b/deployment/helm/Chart.yaml index 9d8710439..9e9142cac 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.11 +version: 0.11.0 # 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.11 +appVersion: 0.11.0 diff --git a/discovery-handler-modules/debug-echo-discovery-handler/Cargo.toml b/discovery-handler-modules/debug-echo-discovery-handler/Cargo.toml index 6c373518f..fdf128a4b 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.11" +version = "0.11.0" 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 771239fe2..be4359c1c 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.11" +version = "0.11.0" 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 b3badeb96..b000cf76b 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.11" +version = "0.11.0" 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 51e61198c..1b1f4dd1c 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.11" +version = "0.11.0" 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 add11e517..7ce4bcf3b 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.11" +version = "0.11.0" 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 54a537c78..815363ee6 100644 --- a/discovery-handlers/onvif/Cargo.toml +++ b/discovery-handlers/onvif/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-onvif" -version = "0.10.11" +version = "0.11.0" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-handlers/opcua/Cargo.toml b/discovery-handlers/opcua/Cargo.toml index 32a145c7d..ddc8f0f99 100644 --- a/discovery-handlers/opcua/Cargo.toml +++ b/discovery-handlers/opcua/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-opcua" -version = "0.10.11" +version = "0.11.0" 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 f345a9082..acc97f9f3 100644 --- a/discovery-handlers/udev/Cargo.toml +++ b/discovery-handlers/udev/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-udev" -version = "0.10.11" +version = "0.11.0" authors = ["Kate Goldenring "] edition = "2018" rust-version = "1.68.1" diff --git a/discovery-utils/Cargo.toml b/discovery-utils/Cargo.toml index 0e8083a1e..9bfb15dbb 100644 --- a/discovery-utils/Cargo.toml +++ b/discovery-utils/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-discovery-utils" -version = "0.10.11" +version = "0.11.0" 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 2194077d4..301385c89 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.11" +version = "0.11.0" authors = ["Kate Goldenring ", ""] edition = "2018" rust-version = "1.68.1" diff --git a/shared/Cargo.toml b/shared/Cargo.toml index 41e6a491f..9df684598 100644 --- a/shared/Cargo.toml +++ b/shared/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "akri-shared" -version = "0.10.11" +version = "0.11.0" authors = [""] edition = "2018" rust-version = "1.68.1" diff --git a/version.txt b/version.txt index 223df1984..d9df1bbc0 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.10.11 +0.11.0 diff --git a/webhooks/validating/configuration/Cargo.toml b/webhooks/validating/configuration/Cargo.toml index 86eb70a79..976a6a928 100644 --- a/webhooks/validating/configuration/Cargo.toml +++ b/webhooks/validating/configuration/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "webhook-configuration" -version = "0.10.11" +version = "0.11.0" authors = ["DazWilkin "] edition = "2018" rust-version = "1.68.1" From 3dc682c9894d8aa3c2f36a69aaf700fbe6aa0576 Mon Sep 17 00:00:00 2001 From: Johnson Shih Date: Thu, 15 Jun 2023 15:37:47 -0700 Subject: [PATCH 3/7] use get_opt to get kube resources by name Signed-off-by: Johnson Shih --- agent/src/util/discovery_operator.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/agent/src/util/discovery_operator.rs b/agent/src/util/discovery_operator.rs index 85357bd75..3873e4356 100644 --- a/agent/src/util/discovery_operator.rs +++ b/agent/src/util/discovery_operator.rs @@ -27,7 +27,7 @@ use blake2::{ use k8s_openapi::api::core::v1::{ ConfigMap, ConfigMapKeySelector, EnvVar, EnvVarSource, Secret, SecretKeySelector, }; -use kube::api::{Api, ListParams}; +use kube::api::Api; use log::{error, trace}; #[cfg(test)] use mock_instant::Instant; @@ -590,13 +590,8 @@ impl KubeClient for ActualKubeClient { async fn get_secret(&self, name: &str, namespace: &str) -> anyhow::Result> { let resource_client: Api = Api::namespaced(self.kube_interface.get_kube_client(), namespace); - let lp = ListParams::default(); - for s in resource_client.list(&lp).await? { - if name == s.metadata.name.as_ref().unwrap() { - return Ok(Some(s)); - } - } - Ok(None) + let resource = resource_client.get_opt(name).await?; + Ok(resource) } async fn get_config_map( @@ -606,13 +601,8 @@ impl KubeClient for ActualKubeClient { ) -> anyhow::Result> { let resource_client: Api = Api::namespaced(self.kube_interface.get_kube_client(), namespace); - let lp = ListParams::default(); - for s in resource_client.list(&lp).await? { - if name == s.metadata.name.as_ref().unwrap() { - return Ok(Some(s)); - } - } - Ok(None) + let resource = resource_client.get_opt(name).await?; + Ok(resource) } } @@ -2013,7 +2003,7 @@ pub mod tests { } #[tokio::test] - async fn test_get_discovery_properties_value_from_config_map_no_secret_name_optional() { + async fn test_get_discovery_properties_value_from_secret_no_secret_name_optional() { let _ = env_logger::builder().is_test(true).try_init(); let namespace_name = "namespace_name"; From 5954c9e950ca6c6b02b7a12d9dd01edb63184504 Mon Sep 17 00:00:00 2001 From: Johnson Shih Date: Wed, 12 Jul 2023 10:03:05 -0700 Subject: [PATCH 4/7] validate discovery properties in webhook Signed-off-by: Johnson Shih --- agent/src/util/discovery_operator.rs | 81 ++--- .../helm/crds/akri-configuration-crd.yaml | 2 +- shared/src/akri/configuration.rs | 29 +- webhooks/validating/configuration/src/main.rs | 321 ++++++++++++++++++ 4 files changed, 378 insertions(+), 55 deletions(-) diff --git a/agent/src/util/discovery_operator.rs b/agent/src/util/discovery_operator.rs index 3873e4356..40c373008 100644 --- a/agent/src/util/discovery_operator.rs +++ b/agent/src/util/discovery_operator.rs @@ -16,7 +16,7 @@ use akri_discovery_utils::discovery::v0::{ DiscoverResponse, }; use akri_shared::{ - akri::configuration::Configuration, + akri::configuration::{Configuration, DiscoveryProperty, DiscoveryPropertySource}, k8s, os::env_var::{ActualEnvVarQuery, EnvVarQuery}, }; @@ -24,9 +24,7 @@ use blake2::{ digest::{Update, VariableOutput}, VarBlake2b, }; -use k8s_openapi::api::core::v1::{ - ConfigMap, ConfigMapKeySelector, EnvVar, EnvVarSource, Secret, SecretKeySelector, -}; +use k8s_openapi::api::core::v1::{ConfigMap, ConfigMapKeySelector, Secret, SecretKeySelector}; use kube::api::Api; use log::{error, trace}; #[cfg(test)] @@ -481,7 +479,7 @@ impl DiscoveryOperator { async fn get_discovery_properties( &self, kube_interface: Arc, - properties: &Option>, + properties: &Option>, ) -> anyhow::Result> { match properties { None => Ok(HashMap::new()), @@ -505,7 +503,7 @@ impl DiscoveryOperator { async fn get_discovery_property( &self, kube_interface: Arc, - property: &EnvVar, + property: &DiscoveryProperty, ) -> anyhow::Result> { let value; if let Some(v) = &property.value { @@ -538,26 +536,25 @@ impl DiscoveryOperator { async fn get_discovery_property_value_from( &self, kube_client: &dyn KubeClient, - property: &EnvVarSource, + property: &DiscoveryPropertySource, ) -> anyhow::Result> { - if let Some(config_map_key_ref) = &property.config_map_key_ref { - return get_discovery_property_value_from_config_map( - kube_client, - self.config.metadata.namespace.as_ref().unwrap(), - config_map_key_ref, - ) - .await; - } - if let Some(secret_key_selector) = &property.secret_key_ref { - get_discovery_property_value_from_secret( - kube_client, - self.config.metadata.namespace.as_ref().unwrap(), - secret_key_selector, - ) - .await - } else { - let error = Error::new(ErrorKind::InvalidInput, "no supported value_from found"); - Err(error.into()) + match property { + DiscoveryPropertySource::ConfigMapKeyRef(config_map_key_selector) => { + get_discovery_property_value_from_config_map( + kube_client, + self.config.metadata.namespace.as_ref().unwrap(), + config_map_key_selector, + ) + .await + } + DiscoveryPropertySource::SecretKeyRef(secret_key_selector) => { + get_discovery_property_value_from_secret( + kube_client, + self.config.metadata.namespace.as_ref().unwrap(), + secret_key_selector, + ) + .await + } } } } @@ -1880,7 +1877,7 @@ pub mod tests { let _ = env_logger::builder().is_test(true).try_init(); let discovery_operator = create_discovery_operator("../test/yaml/config-a.yaml"); let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); - let properties = Vec::::new(); + let properties = Vec::::new(); // properties should be empty if property list is empty assert!(discovery_operator @@ -1898,12 +1895,12 @@ pub mod tests { let property_name_1 = "property_name_1".to_string(); let property_name_2 = "".to_string(); // allow empty property name let properties = vec![ - EnvVar { + DiscoveryProperty { name: property_name_1.clone(), value: None, value_from: None, }, - EnvVar { + DiscoveryProperty { name: property_name_2.clone(), value: None, value_from: None, @@ -1932,12 +1929,12 @@ pub mod tests { let property_value_1 = "property_value_1".to_string(); let property_value_2 = "property_value_2".to_string(); let properties = vec![ - EnvVar { + DiscoveryProperty { name: property_name_1.clone(), value: Some(property_value_1.clone()), value_from: None, }, - EnvVar { + DiscoveryProperty { name: property_name_2.clone(), value: Some(property_value_2.clone()), value_from: None, @@ -1966,26 +1963,6 @@ pub mod tests { assert_eq!(result, expected_result); } - #[tokio::test] - async fn test_get_discovery_properties_value_from_unknown_ref() { - let _ = env_logger::builder().is_test(true).try_init(); - let discovery_operator = create_discovery_operator("../test/yaml/config-a.yaml"); - let mock_kube_interface: Arc = Arc::new(MockKubeInterface::new()); - let property_name = "property_name_1".to_string(); - let property_value_from = EnvVarSource::default(); - let properties = vec![EnvVar { - name: property_name.clone(), - value: None, - value_from: Some(property_value_from), - }]; - - // get_discovery_properties should return error if no supported key ref specified - assert!(discovery_operator - .get_discovery_properties(mock_kube_interface, &Some(properties)) - .await - .is_err()); - } - #[tokio::test] async fn test_get_discovery_properties_value_from_secret_no_secret_name() { let _ = env_logger::builder().is_test(true).try_init(); @@ -2044,7 +2021,7 @@ pub mod tests { }) .returning(move |_, _| Ok(None)); - // get_discovery_property_value_from_secret should return error if no secret not found + // get_discovery_property_value_from_secret should return error if secret not found let result = get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) .await; @@ -2319,7 +2296,7 @@ pub mod tests { }) .returning(move |_, _| Ok(None)); - // get_discovery_property_value_from_config_map should return error if no configMap not found + // get_discovery_property_value_from_config_map should return error if configMap not found let result = get_discovery_property_value_from_config_map( &mock_kube_client, namespace_name, diff --git a/deployment/helm/crds/akri-configuration-crd.yaml b/deployment/helm/crds/akri-configuration-crd.yaml index 4189109a2..d25d9c463 100644 --- a/deployment/helm/crds/akri-configuration-crd.yaml +++ b/deployment/helm/crds/akri-configuration-crd.yaml @@ -25,7 +25,7 @@ spec: discoveryProperties: nullable: true type: array - items: # {{EnvVar}} + items: # {{DiscoveryProperty}} x-kubernetes-preserve-unknown-fields: true type: object capacity: diff --git a/shared/src/akri/configuration.rs b/shared/src/akri/configuration.rs index 74937b445..9ad3425e3 100644 --- a/shared/src/akri/configuration.rs +++ b/shared/src/akri/configuration.rs @@ -5,8 +5,9 @@ // #![allow(non_camel_case_types)] use k8s_openapi::api::batch::v1::JobSpec; -use k8s_openapi::api::core::v1::EnvVar; +use k8s_openapi::api::core::v1::ConfigMapKeySelector; use k8s_openapi::api::core::v1::PodSpec; +use k8s_openapi::api::core::v1::SecretKeySelector; use k8s_openapi::api::core::v1::ServiceSpec; use kube::CustomResource; use kube::{ @@ -19,6 +20,30 @@ use std::collections::HashMap; pub type ConfigurationList = ObjectList; +/// This defines kinds of discovery property source +#[derive(Serialize, Deserialize, PartialEq, Clone, Debug, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub enum DiscoveryPropertySource { + /// Source is a key of a ConfigMap. + ConfigMapKeyRef(ConfigMapKeySelector), + /// Source is a key of a Secret. + SecretKeyRef(SecretKeySelector), +} + +/// DiscoveryProperty represents a property for discovery devices +#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct DiscoveryProperty { + /// Name of the discovery property + pub name: String, + + /// value of the discovery property + pub value: Option, + + /// Source for the discovery property value. Ignored if value is not empty. + pub value_from: Option, +} + /// This specifies which `DiscoveryHandler` should be used for discovery /// and any details that need to be sent to the `DiscoveryHandler`. #[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, PartialEq)] @@ -30,7 +55,7 @@ pub struct DiscoveryHandlerInfo { pub discovery_details: String, #[serde(default, skip_serializing_if = "Option::is_none")] - pub discovery_properties: Option>, + pub discovery_properties: Option>, } /// This defines a workload that should be scheduled to nodes diff --git a/webhooks/validating/configuration/src/main.rs b/webhooks/validating/configuration/src/main.rs index c0e8f0405..3514662c1 100644 --- a/webhooks/validating/configuration/src/main.rs +++ b/webhooks/validating/configuration/src/main.rs @@ -249,6 +249,7 @@ mod tests { use super::*; use actix_web::test; const BROKER_SPEC_INSERTION_KEYWORD: &str = "INSERT_BROKER_SPEC_HERE"; + const DISCOVERY_PROPERTIES_INSERTION_KEYWORD: &str = "INSERT_DISCOVERY_PROPERTIES_HERE"; const ADMISSION_REVIEW: &str = r#" { "kind": "AdmissionReview", @@ -402,6 +403,71 @@ mod tests { } }"#; + const ADMISSION_REVIEW_FOR_DISCOVERY_PROPERTIES: &str = r#" + { + "kind": "AdmissionReview", + "apiVersion": "admission.k8s.io/v1", + "request": { + "uid": "00000000-0000-0000-0000-000000000000", + "kind": { + "group": "akri.sh", + "version": "v0", + "kind": "Configuration" + }, + "resource": { + "group": "akri.sh", + "version": "v0", + "resource": "configurations" + }, + "requestKind": { + "group": "akri.sh", + "version": "v0", + "kind": "Configuration" + }, + "requestResource": { + "group": "akri.sh", + "version": "v0", + "resource": "configurations" + }, + "name": "name", + "namespace": "default", + "operation": "CREATE", + "userInfo": { + "username": "admin", + "uid": "admin", + "groups": [] + }, + "object": { + "apiVersion": "akri.sh/v0", + "kind": "Configuration", + "metadata": { + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "" + }, + "creationTimestamp": "2021-01-01T00:00:00Z", + "generation": 1, + "managedFields": [], + "name": "name", + "namespace": "default", + "uid": "00000000-0000-0000-0000-000000000000" + }, + "spec": { + "discoveryHandler": { + INSERT_DISCOVERY_PROPERTIES_HERE + "name": "debugEcho", + "discoveryDetails": "descriptions:\n- \"foo0\"\n- \"foo1\"\n" + } + } + }, + "oldObject": null, + "dryRun": false, + "options": { + "kind": "CreateOptions", + "apiVersion": "meta.k8s.io/v1" + } + } + }"#; + const VALID_BROKER_POD_SPEC: &str = r#" "brokerPodSpec": { "containers": [ @@ -552,6 +618,11 @@ mod tests { ) } + fn get_admission_review_with_discovery_properties(discovery_properties: &str) -> String { + ADMISSION_REVIEW_FOR_DISCOVERY_PROPERTIES + .replace(DISCOVERY_PROPERTIES_INSERTION_KEYWORD, discovery_properties) + } + // JSON Syntax Tests #[test] fn test_both_null() { @@ -789,6 +860,256 @@ mod tests { assert!(resp.allowed); } + #[test] + fn test_validate_configuration_discovery_properties_empty() { + let discovery_properties = ""; + + // no discovery properties specified should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + fn test_validate_configuration_discovery_properties_empty_list() { + let discovery_properties = r#" + "discoveryProperties": [], + "#; + + // empty discovery properties array should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + fn test_validate_configuration_discovery_properties_plain_text() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn", + "value":"vvvv" + } + ], + "#; + + // plain text discovery properties specified should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + fn test_validate_configuration_discovery_properties_plain_text_empty_value() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn", + "value": "" + } + ],"#; + + // plain text discovery properties, empty value should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + fn test_validate_configuration_discovery_properties_plain_text_no_value() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn" + } + ],"#; + + // plain text discovery properties, value not specified should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + fn test_validate_configuration_discovery_properties_plain_text_empty_name() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "" + } + ],"#; + + // plain text discovery properties, empty name should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + #[should_panic(expected = "Could not parse as Akri Configuration")] + fn test_validate_configuration_discovery_properties_plain_text_no_name() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "value":"vvvv" + } + ], + "#; + + // plain text discovery properties without name should fail, missing field 'name' + run_validate_configuration_discovery_properties(discovery_properties); + } + + #[test] + #[should_panic(expected = "Could not parse as Akri Configuration")] + fn test_validate_configuration_discovery_properties_empty_value_from() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn", + "valueFrom": { + } + } + ],"#; + + // valueFrom discovery properties, not specified should fail, missing content of 'valueFrom' + run_validate_configuration_discovery_properties(discovery_properties); + } + + #[test] + #[should_panic(expected = "Could not parse as Akri Configuration")] + fn test_validate_configuration_discovery_properties_unknown_value_from() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn", + "valueFrom": { + "fieldRef": { + "fieldPath": "ffff" + } + } + } + ],"#; + + // valueFrom discovery properties, unknown ref should fail + run_validate_configuration_discovery_properties(discovery_properties); + } + + #[test] + fn test_validate_configuration_discovery_properties_secret() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn", + "valueFrom": { + "secretKeyRef": { + "name": "nnnn1", + "key": "kkk", + "optional": false + } + } + } + ],"#; + + // valueFrom discovery properties, secretKeyRef should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + fn test_validate_configuration_discovery_properties_config_map() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn", + "valueFrom": { + "configMapKeyRef": { + "name": "nnnn1", + "key": "kkk", + "optional": false + } + } + } + ],"#; + + // valueFrom discovery properties, configMapKeyRef should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + fn test_validate_configuration_discovery_properties_value_and_value_from() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn", + "value": "vvvv", + "valueFrom": { + "configMapKeyRef": { + "name": "nnnn1", + "key": "kkk", + "optional": false + } + } + } + ],"#; + + // both value and valueFrom are specified should success + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(resp.allowed); + } + + #[test] + fn test_validate_configuration_discovery_properties_config_map_invalid_ref_name() { + // invalid "name1" in configMapKeyRef + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "nnnn", + "valueFrom": { + "configMapKeyRef": { + "name1": "nnnn1", + "key": "kkk", + "optional": false + } + } + } + ],"#; + + let resp = run_validate_configuration_discovery_properties(discovery_properties); + assert!(!resp.allowed); + } + + #[test] + #[should_panic(expected = "Could not parse as Akri Configuration")] + fn test_validate_configuration_discovery_properties_config_map_multiple_value_from() { + let discovery_properties = r#" + "discoveryProperties": [ + { + "name": "name1", + "valueFrom": { + "configMapKeyRef": { + "name": "nnnn1", + "key": "kkk", + "optional": false + }, + "secretKeyRef": { + "name": "nnnn1", + "key": "kkk", + "optional": false + } + } + } + ],"#; + + run_validate_configuration_discovery_properties(discovery_properties); + } + + fn run_validate_configuration_discovery_properties( + discovery_properties: &str, + ) -> AdmissionResponse { + let valid: AdmissionReview = serde_json::from_str( + &get_admission_review_with_discovery_properties(discovery_properties), + ) + .expect("v1.AdmissionReview JSON"); + let rqst = valid.request.expect("v1.AdmissionRequest JSON"); + validate_configuration(&rqst) + } + #[actix_rt::test] async fn test_validate_valid_podspec() { let mut app = test::init_service(App::new().service(validate)).await; From 829aac7924fc56e0ffc3fcaac2a722c2b11357db Mon Sep 17 00:00:00 2001 From: Johnson Shih Date: Tue, 18 Jul 2023 16:44:52 -0700 Subject: [PATCH 5/7] address review comments Signed-off-by: Johnson Shih --- agent/src/util/discovery_operator.rs | 38 +++++++++++++++++++++++----- shared/src/akri/configuration.rs | 2 ++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/agent/src/util/discovery_operator.rs b/agent/src/util/discovery_operator.rs index 40c373008..f9434bb7c 100644 --- a/agent/src/util/discovery_operator.rs +++ b/agent/src/util/discovery_operator.rs @@ -613,7 +613,11 @@ async fn get_discovery_property_value_from_secret( if optional { return Ok(None); } - return Err(Error::new(ErrorKind::InvalidInput, "secret name is none").into()); + return Err(Error::new( + ErrorKind::InvalidInput, + "discoveryProperties' referenced Secret name is none", + ) + .into()); } let secret_key = &secret_key_selector.key; let secret_name = secret_key_selector.name.as_ref().unwrap(); @@ -623,7 +627,11 @@ async fn get_discovery_property_value_from_secret( if optional { return Ok(None); } else { - return Err(Error::new(ErrorKind::InvalidInput, "secret not found").into()); + return Err(Error::new( + ErrorKind::InvalidInput, + "discoveryProperties' referenced Secret not found", + ) + .into()); } } let secret = secret.unwrap(); @@ -641,7 +649,11 @@ async fn get_discovery_property_value_from_secret( if optional { Ok(None) } else { - Err(Error::new(ErrorKind::InvalidInput, "secret data not found").into()) + Err(Error::new( + ErrorKind::InvalidInput, + "discoveryProperties' referenced Secret data not found", + ) + .into()) } } @@ -655,7 +667,11 @@ async fn get_discovery_property_value_from_config_map( if optional { return Ok(None); } - return Err(Error::new(ErrorKind::InvalidInput, "config_map name is none").into()); + return Err(Error::new( + ErrorKind::InvalidInput, + "discoveryProperties' referenced ConfigMap name is none", + ) + .into()); } let config_map_name = config_map_key_selector.name.as_ref().unwrap(); let config_map_key = &config_map_key_selector.key; @@ -667,7 +683,11 @@ async fn get_discovery_property_value_from_config_map( if optional { return Ok(None); } else { - return Err(Error::new(ErrorKind::InvalidInput, "config_map not found").into()); + return Err(Error::new( + ErrorKind::InvalidInput, + "discoveryProperties' referenced ConfigMap not found", + ) + .into()); } } let config_map = config_map.unwrap(); @@ -686,11 +706,15 @@ async fn get_discovery_property_value_from_config_map( } } - // config_map_key/value not found + // config_map key/value not found if optional { Ok(None) } else { - Err(Error::new(ErrorKind::InvalidInput, "config_map data not found").into()) + Err(Error::new( + ErrorKind::InvalidInput, + "discoveryProperties' referenced ConfigMap data not found", + ) + .into()) } } diff --git a/shared/src/akri/configuration.rs b/shared/src/akri/configuration.rs index 9ad3425e3..50e4015d7 100644 --- a/shared/src/akri/configuration.rs +++ b/shared/src/akri/configuration.rs @@ -38,9 +38,11 @@ pub struct DiscoveryProperty { pub name: String, /// value of the discovery property + #[serde(default, skip_serializing_if = "Option::is_none")] pub value: Option, /// Source for the discovery property value. Ignored if value is not empty. + #[serde(default, skip_serializing_if = "Option::is_none")] pub value_from: Option, } From 6e7868c477a3d6244ac042afcb393e04e8f2fe5a Mon Sep 17 00:00:00 2001 From: Johnson Shih Date: Thu, 20 Jul 2023 10:38:29 -0700 Subject: [PATCH 6/7] add schema for discoveryProperties to Configuration CRD Signed-off-by: Johnson Shih --- agent/src/util/discovery_operator.rs | 325 +++++------------- .../helm/crds/akri-configuration-crd.yaml | 48 ++- shared/src/akri/configuration.rs | 28 +- webhooks/validating/configuration/src/main.rs | 6 +- 4 files changed, 168 insertions(+), 239 deletions(-) diff --git a/agent/src/util/discovery_operator.rs b/agent/src/util/discovery_operator.rs index f9434bb7c..d0e1d60b1 100644 --- a/agent/src/util/discovery_operator.rs +++ b/agent/src/util/discovery_operator.rs @@ -16,7 +16,9 @@ use akri_discovery_utils::discovery::v0::{ DiscoverResponse, }; use akri_shared::{ - akri::configuration::{Configuration, DiscoveryProperty, DiscoveryPropertySource}, + akri::configuration::{ + Configuration, DiscoveryProperty, DiscoveryPropertyKeySelector, DiscoveryPropertySource, + }, k8s, os::env_var::{ActualEnvVarQuery, EnvVarQuery}, }; @@ -24,7 +26,7 @@ use blake2::{ digest::{Update, VariableOutput}, VarBlake2b, }; -use k8s_openapi::api::core::v1::{ConfigMap, ConfigMapKeySelector, Secret, SecretKeySelector}; +use k8s_openapi::api::core::v1::{ConfigMap, Secret}; use kube::api::Api; use log::{error, trace}; #[cfg(test)] @@ -540,20 +542,11 @@ impl DiscoveryOperator { ) -> anyhow::Result> { match property { DiscoveryPropertySource::ConfigMapKeyRef(config_map_key_selector) => { - get_discovery_property_value_from_config_map( - kube_client, - self.config.metadata.namespace.as_ref().unwrap(), - config_map_key_selector, - ) - .await + get_discovery_property_value_from_config_map(kube_client, config_map_key_selector) + .await } DiscoveryPropertySource::SecretKeyRef(secret_key_selector) => { - get_discovery_property_value_from_secret( - kube_client, - self.config.metadata.namespace.as_ref().unwrap(), - secret_key_selector, - ) - .await + get_discovery_property_value_from_secret(kube_client, secret_key_selector).await } } } @@ -605,24 +598,16 @@ impl KubeClient for ActualKubeClient { async fn get_discovery_property_value_from_secret( kube_client: &dyn KubeClient, - namespace: &str, - secret_key_selector: &SecretKeySelector, + secret_key_selector: &DiscoveryPropertyKeySelector, ) -> anyhow::Result> { let optional = secret_key_selector.optional.unwrap_or_default(); - if secret_key_selector.name.is_none() { - if optional { - return Ok(None); - } - return Err(Error::new( - ErrorKind::InvalidInput, - "discoveryProperties' referenced Secret name is none", - ) - .into()); - } + let secret_name = &secret_key_selector.name; + let secret_namespace = &secret_key_selector.namespace; let secret_key = &secret_key_selector.key; - let secret_name = secret_key_selector.name.as_ref().unwrap(); - let secret = kube_client.get_secret(secret_name, namespace).await?; + let secret = kube_client + .get_secret(secret_name, secret_namespace) + .await?; if secret.is_none() { if optional { return Ok(None); @@ -659,25 +644,15 @@ async fn get_discovery_property_value_from_secret( async fn get_discovery_property_value_from_config_map( kube_client: &dyn KubeClient, - namespace: &str, - config_map_key_selector: &ConfigMapKeySelector, + config_map_key_selector: &DiscoveryPropertyKeySelector, ) -> anyhow::Result> { let optional = config_map_key_selector.optional.unwrap_or_default(); - if config_map_key_selector.name.is_none() { - if optional { - return Ok(None); - } - return Err(Error::new( - ErrorKind::InvalidInput, - "discoveryProperties' referenced ConfigMap name is none", - ) - .into()); - } - let config_map_name = config_map_key_selector.name.as_ref().unwrap(); + let config_map_name = &config_map_key_selector.name; + let config_map_namespace = &config_map_key_selector.namespace; let config_map_key = &config_map_key_selector.key; let config_map = kube_client - .get_config_map(config_map_name, namespace) + .get_config_map(config_map_name, config_map_namespace) .await?; if config_map.is_none() { if optional { @@ -1987,42 +1962,6 @@ pub mod tests { assert_eq!(result, expected_result); } - #[tokio::test] - async fn test_get_discovery_properties_value_from_secret_no_secret_name() { - let _ = env_logger::builder().is_test(true).try_init(); - let namespace_name = "namespace_name"; - - let selector = SecretKeySelector::default(); - - let mock_kube_client = MockKubeClient::new(); - - // get_discovery_property_value_from_secret should return error if mandatory secret name is empty - let result = - get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) - .await; - assert!(result.is_err()); - } - - #[tokio::test] - async fn test_get_discovery_properties_value_from_secret_no_secret_name_optional() { - let _ = env_logger::builder().is_test(true).try_init(); - let namespace_name = "namespace_name"; - - let selector = SecretKeySelector { - key: String::default(), - name: None, - optional: Some(true), - }; - - let mock_kube_client = MockKubeClient::new(); - - // get_discovery_property_value_from_secret should return empty if optional secret name is empty - let result = - get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) - .await; - assert!(result.unwrap().is_none()); - } - #[tokio::test] async fn test_get_discovery_properties_value_from_secret_no_secret_found() { let _ = env_logger::builder().is_test(true).try_init(); @@ -2030,9 +1969,10 @@ pub mod tests { let secret_name = "secret_1"; let key_in_secret = "key_in_secret"; - let selector = SecretKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_secret.to_string(), - name: Some(secret_name.to_string()), + name: secret_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2046,9 +1986,7 @@ pub mod tests { .returning(move |_, _| Ok(None)); // get_discovery_property_value_from_secret should return error if secret not found - let result = - get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) - .await; + let result = get_discovery_property_value_from_secret(&mock_kube_client, &selector).await; assert!(result.is_err()); } @@ -2059,9 +1997,10 @@ pub mod tests { let secret_name = "secret_1"; let key_in_secret = "key_in_secret"; - let selector = SecretKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_secret.to_string(), - name: Some(secret_name.to_string()), + name: secret_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(true), }; @@ -2075,9 +2014,7 @@ pub mod tests { .returning(move |_, _| Ok(None)); // get_discovery_property_value_from_secret for an optional key should return None if secret not found - let result = - get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) - .await; + let result = get_discovery_property_value_from_secret(&mock_kube_client, &selector).await; assert!(result.unwrap().is_none()); } @@ -2088,9 +2025,10 @@ pub mod tests { let secret_name = "secret_1"; let key_in_secret = "key_in_secret"; - let selector = SecretKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_secret.to_string(), - name: Some(secret_name.to_string()), + name: secret_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2104,13 +2042,11 @@ pub mod tests { .returning(move |_, _| Ok(Some(Secret::default()))); // get_discovery_property_value_from_secret should return error if key in secret not found - assert!(get_discovery_property_value_from_secret( - &mock_kube_client, - namespace_name, - &selector, - ) - .await - .is_err()); + assert!( + get_discovery_property_value_from_secret(&mock_kube_client, &selector,) + .await + .is_err() + ); } #[tokio::test] @@ -2120,9 +2056,10 @@ pub mod tests { let secret_name = "secret_1"; let key_in_secret = "key_in_config_map"; - let selector = SecretKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_secret.to_string(), - name: Some(secret_name.to_string()), + name: secret_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(true), }; @@ -2136,9 +2073,7 @@ pub mod tests { .returning(move |_, _| Ok(Some(Secret::default()))); // get_discovery_property_value_from_secret for an optional key should return None if key in secret not found - let result = - get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) - .await; + let result = get_discovery_property_value_from_secret(&mock_kube_client, &selector).await; assert!(result.unwrap().is_none()); } @@ -2149,9 +2084,10 @@ pub mod tests { let secret_name = "secret_1"; let key_in_secret = "key_in_secret"; - let selector = SecretKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_secret.to_string(), - name: Some(secret_name.to_string()), + name: secret_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2171,9 +2107,7 @@ pub mod tests { }); // get_discovery_property_value_from_secret should return error if no value in secret - let result = - get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) - .await; + let result = get_discovery_property_value_from_secret(&mock_kube_client, &selector).await; assert!(result.is_err()); } @@ -2184,9 +2118,10 @@ pub mod tests { let secret_name = "secret_1"; let key_in_secret = "key_in_config_map"; - let selector = SecretKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_secret.to_string(), - name: Some(secret_name.to_string()), + name: secret_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(true), }; @@ -2206,9 +2141,7 @@ pub mod tests { }); // get_discovery_property_value_from_secret for an optional key should return None if key in secret not found - let result = - get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) - .await; + let result = get_discovery_property_value_from_secret(&mock_kube_client, &selector).await; assert!(result.unwrap().is_none()); } @@ -2220,9 +2153,10 @@ pub mod tests { let key_in_secret = "key_in_secret"; let value_in_secret = "value_in_secret"; - let selector = SecretKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_secret.to_string(), - name: Some(secret_name.to_string()), + name: secret_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2250,54 +2184,10 @@ pub mod tests { }; // get_discovery_property_value_from_secret should return correct value if data value in secret - let result = - get_discovery_property_value_from_secret(&mock_kube_client, namespace_name, &selector) - .await; + let result = get_discovery_property_value_from_secret(&mock_kube_client, &selector).await; assert_eq!(result.unwrap().unwrap(), expected_result); } - #[tokio::test] - async fn test_get_discovery_properties_value_from_config_map_no_config_map_name() { - let _ = env_logger::builder().is_test(true).try_init(); - let namespace_name = "namespace_name"; - - let selector = ConfigMapKeySelector::default(); - - let mock_kube_client = MockKubeClient::new(); - - // get_discovery_property_value_from_config_map should return error if mandatory configMap name is empty - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; - assert!(result.is_err()); - } - - #[tokio::test] - async fn test_get_discovery_properties_value_from_config_map_no_config_map_name_optional() { - let _ = env_logger::builder().is_test(true).try_init(); - let namespace_name = "namespace_name"; - - let selector = ConfigMapKeySelector { - key: String::default(), - name: None, - optional: Some(true), - }; - - let mock_kube_client = MockKubeClient::new(); - - // get_discovery_property_value_from_config_map should return empty if optional configMap name is empty - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; - assert!(result.unwrap().is_none()); - } - #[tokio::test] async fn test_get_discovery_properties_value_from_config_map_no_config_map_found() { let _ = env_logger::builder().is_test(true).try_init(); @@ -2305,9 +2195,10 @@ pub mod tests { let config_map_name = "config_map_1"; let key_in_config_map = "key_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2321,12 +2212,8 @@ pub mod tests { .returning(move |_, _| Ok(None)); // get_discovery_property_value_from_config_map should return error if configMap not found - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert!(result.is_err()); } @@ -2337,9 +2224,10 @@ pub mod tests { let config_map_name = "config_map_1"; let key_in_config_map = "key_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(true), }; @@ -2353,12 +2241,8 @@ pub mod tests { .returning(move |_, _| Ok(None)); // get_discovery_property_value_from_config_map for an optional key should return None if configMap not found - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert!(result.unwrap().is_none()); } @@ -2369,9 +2253,10 @@ pub mod tests { let config_map_name = "config_map_1"; let key_in_config_map = "key_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2385,12 +2270,8 @@ pub mod tests { .returning(move |_, _| Ok(Some(ConfigMap::default()))); // get_discovery_property_value_from_config_map should return error if key in configMap not found - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert!(result.is_err()); } @@ -2401,9 +2282,10 @@ pub mod tests { let config_map_name = "config_map_1"; let key_in_config_map = "key_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(true), }; @@ -2417,12 +2299,8 @@ pub mod tests { .returning(move |_, _| Ok(Some(ConfigMap::default()))); // get_discovery_property_value_from_config_map for an optional key should return None if key in configMap not found - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert!(result.unwrap().is_none()); } @@ -2433,9 +2311,10 @@ pub mod tests { let config_map_name = "config_map_1"; let key_in_config_map = "key_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2456,12 +2335,8 @@ pub mod tests { }); // get_discovery_property_value_from_config_map should return error if no value in configMap - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert!(result.is_err()); } @@ -2472,9 +2347,10 @@ pub mod tests { let config_map_name = "config_map_1"; let key_in_config_map = "key_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(true), }; @@ -2495,12 +2371,8 @@ pub mod tests { }); // get_discovery_property_value_from_config_map for an optional key should return None if key in configMap not found - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert!(result.unwrap().is_none()); } @@ -2512,9 +2384,10 @@ pub mod tests { let key_in_config_map = "key_in_config_map"; let value_in_config_map = "value_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2543,12 +2416,8 @@ pub mod tests { }; // get_discovery_property_value_from_config_map should return correct value if data value in configMap - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert_eq!(result.unwrap().unwrap(), expected_result); } @@ -2560,9 +2429,10 @@ pub mod tests { let key_in_config_map = "key_in_config_map"; let value_in_config_map = "value_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2591,12 +2461,8 @@ pub mod tests { }; // get_discovery_property_value_from_config_map should return correct value if binary data value in configMap - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert_eq!(result.unwrap().unwrap(), expected_result); } @@ -2609,9 +2475,10 @@ pub mod tests { let value_in_config_map = "value_in_config_map"; let binary_value_in_config_map = "binary_value_in_config_map"; - let selector = ConfigMapKeySelector { + let selector = DiscoveryPropertyKeySelector { key: key_in_config_map.to_string(), - name: Some(config_map_name.to_string()), + name: config_map_name.to_string(), + namespace: namespace_name.to_string(), optional: Some(false), }; @@ -2644,12 +2511,8 @@ pub mod tests { }; // get_discovery_property_value_from_config_map should return value from data if both data and binary data value exist - let result = get_discovery_property_value_from_config_map( - &mock_kube_client, - namespace_name, - &selector, - ) - .await; + let result = + get_discovery_property_value_from_config_map(&mock_kube_client, &selector).await; assert_eq!(result.unwrap().unwrap(), expected_result); } } diff --git a/deployment/helm/crds/akri-configuration-crd.yaml b/deployment/helm/crds/akri-configuration-crd.yaml index d25d9c463..29c0ccefe 100644 --- a/deployment/helm/crds/akri-configuration-crd.yaml +++ b/deployment/helm/crds/akri-configuration-crd.yaml @@ -26,8 +26,54 @@ spec: nullable: true type: array items: # {{DiscoveryProperty}} - x-kubernetes-preserve-unknown-fields: true type: object + required: + - name + properties: + name: + type: string + value: + type: string + nullable: true + valueFrom: + type: object + properties: + secretKeyRef: + type: object + required: + - name + properties: + key: + type: string + name: + type: string + namespace: + type: string + optional: + type: boolean + configMapKeyRef: + type: object + required: + - name + properties: + key: + type: string + name: + type: string + namespace: + type: string + optional: + type: boolean + oneOf: + - properties: + required: ["secretKeyRef"] + - properties: + required: ["configMapKeyRef"] + oneOf: + - properties: + required: ["value"] + - properties: + required: ["valueFrom"] capacity: type: integer brokerSpec: # {{BrokerSpec}} diff --git a/shared/src/akri/configuration.rs b/shared/src/akri/configuration.rs index 50e4015d7..b2ebc7907 100644 --- a/shared/src/akri/configuration.rs +++ b/shared/src/akri/configuration.rs @@ -5,9 +5,7 @@ // #![allow(non_camel_case_types)] use k8s_openapi::api::batch::v1::JobSpec; -use k8s_openapi::api::core::v1::ConfigMapKeySelector; use k8s_openapi::api::core::v1::PodSpec; -use k8s_openapi::api::core::v1::SecretKeySelector; use k8s_openapi::api::core::v1::ServiceSpec; use kube::CustomResource; use kube::{ @@ -20,14 +18,36 @@ use std::collections::HashMap; pub type ConfigurationList = ObjectList; +/// Selects a key from a ConfigMap or Secret +#[derive(Serialize, Deserialize, PartialEq, Clone, Debug, JsonSchema)] +pub struct DiscoveryPropertyKeySelector { + /// The key to select. + pub key: String, + + /// Name of the referent. + pub name: String, + + /// Namespace of the referent + #[serde(default = "default_namespace")] + pub namespace: String, + + /// Specify whether the referent or its key must be defined + #[serde(default, skip_serializing_if = "Option::is_none")] + pub optional: Option, +} + +fn default_namespace() -> String { + String::from("default") +} + /// This defines kinds of discovery property source #[derive(Serialize, Deserialize, PartialEq, Clone, Debug, JsonSchema)] #[serde(rename_all = "camelCase")] pub enum DiscoveryPropertySource { /// Source is a key of a ConfigMap. - ConfigMapKeyRef(ConfigMapKeySelector), + ConfigMapKeyRef(DiscoveryPropertyKeySelector), /// Source is a key of a Secret. - SecretKeyRef(SecretKeySelector), + SecretKeyRef(DiscoveryPropertyKeySelector), } /// DiscoveryProperty represents a property for discovery devices diff --git a/webhooks/validating/configuration/src/main.rs b/webhooks/validating/configuration/src/main.rs index 900743389..afaa3e9f4 100644 --- a/webhooks/validating/configuration/src/main.rs +++ b/webhooks/validating/configuration/src/main.rs @@ -1054,7 +1054,8 @@ mod tests { } #[test] - fn test_validate_configuration_discovery_properties_config_map_invalid_ref_name() { + #[should_panic(expected = "Could not parse as Akri Configuration")] + fn test_validate_configuration_discovery_properties_config_map_invalid_ref_name_xxx() { // invalid "name1" in configMapKeyRef let discovery_properties = r#" "discoveryProperties": [ @@ -1070,8 +1071,7 @@ mod tests { } ],"#; - let resp = run_validate_configuration_discovery_properties(discovery_properties); - assert!(!resp.allowed); + run_validate_configuration_discovery_properties(discovery_properties); } #[test] From 3b2f2f60ccbcaff2f6746dea056e3dca089f450a Mon Sep 17 00:00:00 2001 From: Johnson Shih Date: Thu, 20 Jul 2023 12:42:57 -0700 Subject: [PATCH 7/7] update unit test case Signed-off-by: Johnson Shih --- webhooks/validating/configuration/src/main.rs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/webhooks/validating/configuration/src/main.rs b/webhooks/validating/configuration/src/main.rs index afaa3e9f4..e50c6199a 100644 --- a/webhooks/validating/configuration/src/main.rs +++ b/webhooks/validating/configuration/src/main.rs @@ -1031,28 +1031,6 @@ mod tests { assert!(resp.allowed); } - #[test] - fn test_validate_configuration_discovery_properties_value_and_value_from() { - let discovery_properties = r#" - "discoveryProperties": [ - { - "name": "nnnn", - "value": "vvvv", - "valueFrom": { - "configMapKeyRef": { - "name": "nnnn1", - "key": "kkk", - "optional": false - } - } - } - ],"#; - - // both value and valueFrom are specified should success - let resp = run_validate_configuration_discovery_properties(discovery_properties); - assert!(resp.allowed); - } - #[test] #[should_panic(expected = "Could not parse as Akri Configuration")] fn test_validate_configuration_discovery_properties_config_map_invalid_ref_name_xxx() {