From dced497c389f48cd0f1d81f841b9d3fd41f0a49d Mon Sep 17 00:00:00 2001 From: Joshua Chapman Date: Thu, 12 Dec 2024 11:48:29 +0100 Subject: [PATCH] feat(container): use uuid instead of strings Parse the uuid when converting the event. Signed-off-by: Joshua Chapman --- .../src/docker/container.rs | 4 +- .../src/requests/container.rs | 63 +++++---- .../src/requests/deployment.rs | 19 +-- .../src/requests/image.rs | 25 +++- .../src/requests/mod.rs | 120 ++++++++++++++++-- .../src/requests/network.rs | 12 +- .../src/requests/volume.rs | 17 ++- .../src/service/mod.rs | 34 ++--- 8 files changed, 209 insertions(+), 85 deletions(-) diff --git a/edgehog-device-runtime-containers/src/docker/container.rs b/edgehog-device-runtime-containers/src/docker/container.rs index 6dba20f6..64034ed0 100644 --- a/edgehog-device-runtime-containers/src/docker/container.rs +++ b/edgehog-device-runtime-containers/src/docker/container.rs @@ -487,10 +487,10 @@ where #[derive(Debug, Clone, Default)] pub struct PortBindingMap(pub HashMap>>); -impl TryFrom> for PortBindingMap { +impl TryFrom<&[String]> for PortBindingMap { type Error = BindingError; - fn try_from(value: Vec) -> Result { + fn try_from(value: &[String]) -> Result { value .iter() .try_fold( diff --git a/edgehog-device-runtime-containers/src/requests/container.rs b/edgehog-device-runtime-containers/src/requests/container.rs index 2fe6597c..c26d82d2 100644 --- a/edgehog-device-runtime-containers/src/requests/container.rs +++ b/edgehog-device-runtime-containers/src/requests/container.rs @@ -26,11 +26,11 @@ use tracing::{instrument, trace}; use crate::{ container::{Binding, Container, PortBindingMap}, requests::BindingError, - service::{Id, ResourceType, ServiceError}, + service::{Id, ResourceType}, store::container::RestartPolicy, }; -use super::ReqError; +use super::{ReqError, ReqUuid, VecReqUuid}; /// Request to pull a Docker Container. #[derive(Debug, Clone, FromEvent, PartialEq, Eq, PartialOrd, Ord)] @@ -40,10 +40,10 @@ use super::ReqError; rename_all = "camelCase" )] pub struct CreateContainer { - pub(crate) id: String, - pub(crate) image_id: String, - pub(crate) network_ids: Vec, - pub(crate) volume_ids: Vec, + pub(crate) id: ReqUuid, + pub(crate) image_id: ReqUuid, + pub(crate) network_ids: VecReqUuid, + pub(crate) volume_ids: VecReqUuid, // TODO: remove this image, use the image id pub(crate) image: String, pub(crate) hostname: String, @@ -56,17 +56,17 @@ pub struct CreateContainer { } impl CreateContainer { - pub(crate) fn dependencies(&self) -> Result, ServiceError> { - std::iter::once(Id::try_from_str(ResourceType::Image, &self.image_id)) + pub(crate) fn dependencies(&self) -> Vec { + std::iter::once(Id::new(ResourceType::Image, *self.image_id)) .chain( self.network_ids .iter() - .map(|id| Id::try_from_str(ResourceType::Network, id)), + .map(|id| Id::new(ResourceType::Network, **id)), ) .chain( self.volume_ids .iter() - .map(|id| Id::try_from_str(ResourceType::Volume, id)), + .map(|id| Id::new(ResourceType::Volume, **id)), ) .collect() } @@ -83,18 +83,18 @@ impl TryFrom for Container { }; let restart_policy = RestartPolicy::from_str(&value.restart_policy)?.into(); - let port_bindings = PortBindingMap::try_from(value.port_bindings)?; + let port_bindings = PortBindingMap::try_from(value.port_bindings.as_slice())?; Ok(Container { id: None, - name: value.id, + name: value.id.to_string(), image: value.image, hostname, restart_policy, env: value.env, network_mode: value.network_mode, // Use the network ids - networks: value.network_ids, + networks: value.network_ids.iter().map(|id| id.to_string()).collect(), binds: value.binds, port_bindings, privileged: value.privileged, @@ -207,15 +207,17 @@ pub(crate) mod tests { use astarte_device_sdk::{AstarteType, DeviceEvent, Value}; use bollard::secret::RestartPolicyNameEnum; + use itertools::Itertools; use pretty_assertions::assert_eq; + use uuid::Uuid; use super::*; - pub fn create_container_request_event( + pub fn create_container_request_event( id: impl Display, image_id: &str, image: &str, - network_ids: &[&str], + network_ids: &[S], ) -> DeviceEvent { let fields = [ ("id", AstarteType::String(id.to_string())), @@ -250,20 +252,19 @@ pub(crate) mod tests { #[test] fn create_container_request() { - let event = create_container_request_event( - "id", - "image_id", - "image", - &["9808bbd5-2e81-4f99-83e7-7cc60623a196"], - ); + let id = ReqUuid(Uuid::new_v4()); + let image_id = ReqUuid(Uuid::new_v4()); + let network_ids = VecReqUuid(vec![ReqUuid(Uuid::new_v4())]); + let event = + create_container_request_event(id, &image_id.to_string(), "image", &network_ids); let request = CreateContainer::from_event(event).unwrap(); let expect = CreateContainer { - id: "id".to_string(), - image_id: "image_id".to_string(), - network_ids: vec!["9808bbd5-2e81-4f99-83e7-7cc60623a196".to_string()], - volume_ids: vec![], + id, + image_id, + network_ids, + volume_ids: VecReqUuid(vec![]), image: "image".to_string(), hostname: "hostname".to_string(), restart_policy: "no".to_string(), @@ -278,12 +279,20 @@ pub(crate) mod tests { let container = Container::try_from(request).unwrap(); + let network_ids = expect + .network_ids + .iter() + .map(|s| s.to_string()) + .collect_vec(); + let network_ids = network_ids.iter().map(|s| s.as_str()).collect_vec(); + let name = id.to_string(); + let exp = Container { id: None, - name: "id", + name: name.as_str(), image: "image", network_mode: "bridge", - networks: vec!["9808bbd5-2e81-4f99-83e7-7cc60623a196"], + networks: network_ids, hostname: Some("hostname"), restart_policy: RestartPolicyNameEnum::NO, env: vec!["env"], diff --git a/edgehog-device-runtime-containers/src/requests/deployment.rs b/edgehog-device-runtime-containers/src/requests/deployment.rs index 7cb86dc7..7d9206fc 100644 --- a/edgehog-device-runtime-containers/src/requests/deployment.rs +++ b/edgehog-device-runtime-containers/src/requests/deployment.rs @@ -22,6 +22,8 @@ use astarte_device_sdk::{event::FromEventError, types::TypeError, AstarteType, F use tracing::error; use uuid::Uuid; +use super::{ReqUuid, VecReqUuid}; + /// Request to pull a Docker Deployment. #[derive(Debug, Clone, FromEvent, PartialEq, Eq, PartialOrd, Ord)] #[from_event( @@ -30,8 +32,8 @@ use uuid::Uuid; rename_all = "camelCase" )] pub struct CreateDeployment { - pub(crate) id: String, - pub(crate) containers: Vec, + pub(crate) id: ReqUuid, + pub(crate) containers: VecReqUuid, } /// Command for a previously received deployment @@ -150,14 +152,14 @@ struct DeploymentUpdateEvent { #[cfg(test)] pub(crate) mod tests { - use std::collections::HashMap; + use std::{collections::HashMap, fmt::Display}; use astarte_device_sdk::{AstarteType, DeviceEvent, Value}; use pretty_assertions::assert_eq; use super::*; - pub fn create_deployment_request_event(id: &str, containers: &[&str]) -> DeviceEvent { + pub fn create_deployment_request_event(id: &str, containers: &[S]) -> DeviceEvent { let fields = [ ("id", AstarteType::String(id.to_string())), ( @@ -178,14 +180,13 @@ pub(crate) mod tests { #[test] fn create_deployment_request() { - let event = create_deployment_request_event("id", &["container_id"]); + let id = ReqUuid(Uuid::new_v4()); + let containers = VecReqUuid(vec![ReqUuid(Uuid::new_v4())]); + let event = create_deployment_request_event(&id.to_string(), &containers); let request = CreateDeployment::from_event(event).unwrap(); - let expect = CreateDeployment { - id: "id".to_string(), - containers: vec!["container_id".to_string()], - }; + let expect = CreateDeployment { id, containers }; assert_eq!(request, expect); } diff --git a/edgehog-device-runtime-containers/src/requests/image.rs b/edgehog-device-runtime-containers/src/requests/image.rs index 79cd3d61..9b1b33c7 100644 --- a/edgehog-device-runtime-containers/src/requests/image.rs +++ b/edgehog-device-runtime-containers/src/requests/image.rs @@ -22,6 +22,8 @@ use astarte_device_sdk::FromEvent; use crate::image::Image; +use super::ReqUuid; + /// Request to pull a Docker Image. #[derive(Debug, Clone, FromEvent, PartialEq, Eq, PartialOrd, Ord)] #[from_event( @@ -30,7 +32,7 @@ use crate::image::Image; rename_all = "camelCase" )] pub struct CreateImage { - pub(crate) id: String, + pub(crate) id: ReqUuid, pub(crate) reference: String, pub(crate) registry_auth: String, } @@ -56,9 +58,22 @@ pub(crate) mod tests { use std::fmt::Display; use astarte_device_sdk::{DeviceEvent, Value}; + use uuid::Uuid; use super::*; + pub fn mock_image_req( + id: Uuid, + reference: impl Into, + registry_auth: impl Into, + ) -> CreateImage { + CreateImage { + id: ReqUuid(id), + reference: reference.into(), + registry_auth: registry_auth.into(), + } + } + pub fn create_image_request_event( id: impl Display, reference: &str, @@ -82,12 +97,13 @@ pub(crate) mod tests { #[test] fn create_image_request() { - let event = create_image_request_event("id", "reference", "registry_auth"); + let id = Uuid::new_v4(); + let event = create_image_request_event(id.to_string(), "reference", "registry_auth"); let request = CreateImage::from_event(event).unwrap(); let expect = CreateImage { - id: "id".to_string(), + id: ReqUuid(id), reference: "reference".to_string(), registry_auth: "registry_auth".to_string(), }; @@ -97,7 +113,8 @@ pub(crate) mod tests { #[test] fn should_convert_to_image() { - let event = create_image_request_event("id", "reference", "registry_auth"); + let id = Uuid::new_v4(); + let event = create_image_request_event(id.to_string(), "reference", "registry_auth"); let request = CreateImage::from_event(event).unwrap(); diff --git a/edgehog-device-runtime-containers/src/requests/mod.rs b/edgehog-device-runtime-containers/src/requests/mod.rs index c299f096..c9b3723f 100644 --- a/edgehog-device-runtime-containers/src/requests/mod.rs +++ b/edgehog-device-runtime-containers/src/requests/mod.rs @@ -18,11 +18,15 @@ //! Container requests sent from Astarte. -use std::{collections::HashMap, num::ParseIntError}; +use std::{borrow::Borrow, collections::HashMap, fmt::Display, num::ParseIntError, ops::Deref}; -use astarte_device_sdk::{event::FromEventError, DeviceEvent, FromEvent}; +use astarte_device_sdk::{ + event::FromEventError, types::TypeError, AstarteType, DeviceEvent, FromEvent, +}; use container::CreateContainer; use deployment::{CreateDeployment, DeploymentCommand, DeploymentUpdate}; +use tracing::error; +use uuid::Uuid; use crate::store::container::RestartPolicyError; @@ -128,19 +132,112 @@ where .collect() } +/// Wrapper to convert an [`AstarteType`] to [`Uuid`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(crate) struct ReqUuid(pub(crate) Uuid); + +impl Display for ReqUuid { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Borrow for ReqUuid { + fn borrow(&self) -> &Uuid { + &self.0 + } +} + +impl Deref for ReqUuid { + type Target = Uuid; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl TryFrom<&str> for ReqUuid { + type Error = TypeError; + + fn try_from(value: &str) -> Result { + Uuid::parse_str(value).map(ReqUuid).map_err(|err| { + error!( + error = format!("{:#}", eyre::Report::new(err)), + value, "couldn't parse uuid value" + ); + + TypeError::Conversion + }) + } +} + +impl TryFrom for ReqUuid { + type Error = TypeError; + + fn try_from(value: AstarteType) -> Result { + let value = String::try_from(value)?; + + Self::try_from(value.as_str()) + } +} + +impl From for Uuid { + fn from(value: ReqUuid) -> Self { + value.0 + } +} + +impl From<&ReqUuid> for Uuid { + fn from(value: &ReqUuid) -> Self { + value.0 + } +} + +/// Wrapper to convert an [`AstarteType`] to [`Vec`]. +/// +/// This is required because we cannot implement [`TryFrom`] for [`Vec`], because +/// of the orphan rule. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] +pub(crate) struct VecReqUuid(pub(crate) Vec); + +impl Deref for VecReqUuid { + type Target = Vec; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl TryFrom for VecReqUuid { + type Error = TypeError; + + fn try_from(value: AstarteType) -> Result { + let value = Vec::::try_from(value)?; + + value + .iter() + .map(|v| ReqUuid::try_from(v.as_str())) + .collect::, TypeError>>() + .map(VecReqUuid) + } +} + #[cfg(test)] mod tests { use super::*; use astarte_device_sdk::Value; + use image::tests::mock_image_req; use network::{tests::create_network_request_event, CreateNetwork}; - use crate::requests::{image::CreateImage, ContainerRequest}; + use crate::requests::ContainerRequest; #[test] fn from_event_image() { + let id = Uuid::new_v4(); + let fields = [ - ("id", "id"), + ("id", id.to_string().as_str()), ("reference", "reference"), ("registryAuth", "registry_auth"), ] @@ -155,11 +252,11 @@ mod tests { let request = ContainerRequest::from_event(event).unwrap(); - let expect = ContainerRequest::Image(CreateImage { - id: "id".to_string(), - reference: "reference".to_string(), - registry_auth: "registry_auth".to_string(), - }); + let expect = ContainerRequest::Image(mock_image_req( + id, + "reference".to_string(), + "registry_auth".to_string(), + )); assert_eq!(request, expect); } @@ -183,12 +280,13 @@ mod tests { #[test] fn from_event_network() { - let event = create_network_request_event("id", "driver", &[]); + let id = Uuid::new_v4(); + let event = create_network_request_event(id.to_string(), "driver", &[]); let request = ContainerRequest::from_event(event).unwrap(); let expect = CreateNetwork { - id: "id".to_string(), + id: ReqUuid(id), driver: "driver".to_string(), internal: false, enable_ipv6: false, diff --git a/edgehog-device-runtime-containers/src/requests/network.rs b/edgehog-device-runtime-containers/src/requests/network.rs index 6927acca..918e6cb4 100644 --- a/edgehog-device-runtime-containers/src/requests/network.rs +++ b/edgehog-device-runtime-containers/src/requests/network.rs @@ -22,7 +22,7 @@ use astarte_device_sdk::FromEvent; use crate::network::Network; -use super::{parse_kv_map, ReqError}; +use super::{parse_kv_map, ReqError, ReqUuid}; /// Request to pull a Docker Network. #[derive(Debug, Clone, FromEvent, PartialEq, Eq, PartialOrd, Ord)] @@ -32,7 +32,7 @@ use super::{parse_kv_map, ReqError}; rename_all = "camelCase" )] pub struct CreateNetwork { - pub(crate) id: String, + pub(crate) id: ReqUuid, pub(crate) driver: String, pub(crate) internal: bool, pub(crate) enable_ipv6: bool, @@ -47,7 +47,7 @@ impl TryFrom for Network { Ok(Network { id: None, - name: value.id, + name: value.id.to_string(), driver: value.driver, internal: value.internal, enable_ipv6: value.enable_ipv6, @@ -63,6 +63,7 @@ pub(crate) mod tests { use astarte_device_sdk::{AstarteType, DeviceEvent, Value}; use itertools::Itertools; + use uuid::Uuid; use super::*; @@ -94,12 +95,13 @@ pub(crate) mod tests { #[test] fn create_network_request() { - let event = create_network_request_event("id", "driver", &["foo=bar", "some="]); + let id = Uuid::new_v4(); + let event = create_network_request_event(id.to_string(), "driver", &["foo=bar", "some="]); let request = CreateNetwork::from_event(event).unwrap(); let expect = CreateNetwork { - id: "id".to_string(), + id: ReqUuid(id), driver: "driver".to_string(), internal: false, enable_ipv6: false, diff --git a/edgehog-device-runtime-containers/src/requests/volume.rs b/edgehog-device-runtime-containers/src/requests/volume.rs index b12a1477..b3f955ff 100644 --- a/edgehog-device-runtime-containers/src/requests/volume.rs +++ b/edgehog-device-runtime-containers/src/requests/volume.rs @@ -22,7 +22,7 @@ use astarte_device_sdk::FromEvent; use crate::volume::Volume; -use super::{parse_kv_map, ReqError}; +use super::{parse_kv_map, ReqError, ReqUuid}; /// Request to pull a Docker Volume. #[derive(Debug, Clone, FromEvent, PartialEq, Eq, PartialOrd, Ord)] @@ -32,7 +32,7 @@ use super::{parse_kv_map, ReqError}; rename_all = "camelCase" )] pub struct CreateVolume { - pub(crate) id: String, + pub(crate) id: ReqUuid, pub(crate) driver: String, pub(crate) options: Vec, } @@ -50,7 +50,7 @@ impl TryFrom for Volume { let driver_opts = parse_kv_map(&value.options)?; Ok(Volume { - name: value.id, + name: value.id.to_string(), driver, driver_opts, }) @@ -63,6 +63,7 @@ pub(crate) mod tests { use astarte_device_sdk::{AstarteType, DeviceEvent, Value}; use itertools::Itertools; + use uuid::Uuid; use super::*; @@ -93,12 +94,13 @@ pub(crate) mod tests { #[test] fn create_volume_request() { - let event = create_volume_request_event("id", "driver", &["foo=bar", "some="]); + let id = Uuid::new_v4(); + let event = create_volume_request_event(id.to_string(), "driver", &["foo=bar", "some="]); let request = CreateVolume::from_event(event).unwrap(); let expect = CreateVolume { - id: "id".to_string(), + id: ReqUuid(id), driver: "driver".to_string(), options: ["foo=bar", "some="].map(str::to_string).to_vec(), }; @@ -108,12 +110,13 @@ pub(crate) mod tests { #[test] fn volume_default_driver() { - let event = create_volume_request_event("id", "", &["foo=bar", "some="]); + let id = Uuid::new_v4().to_string(); + let event = create_volume_request_event(&id, "", &["foo=bar", "some="]); let request = CreateVolume::from_event(event).unwrap(); let expect = Volume { - name: "id", + name: id.as_str(), driver: "local", driver_opts: HashMap::from([("foo".to_string(), "bar"), ("some".to_string(), "")]), }; diff --git a/edgehog-device-runtime-containers/src/service/mod.rs b/edgehog-device-runtime-containers/src/service/mod.rs index d42fa3f4..7412918f 100644 --- a/edgehog-device-runtime-containers/src/service/mod.rs +++ b/edgehog-device-runtime-containers/src/service/mod.rs @@ -21,7 +21,6 @@ use std::{ collections::HashSet, fmt::{Debug, Display}, - str::FromStr, }; use astarte_device_sdk::event::FromEventError; @@ -232,7 +231,7 @@ impl Service { where D: Client + Sync + 'static, { - let id = Id::try_from_str(ResourceType::Image, &req.id)?; + let id = Id::new(ResourceType::Image, *req.id); debug!("creating image with id {id}"); @@ -249,7 +248,7 @@ impl Service { where D: Client + Sync + 'static, { - let id = Id::try_from_str(ResourceType::Volume, &req.id)?; + let id = Id::new(ResourceType::Volume, *req.id); debug!("creating volume with id {id}"); @@ -266,7 +265,7 @@ impl Service { where D: Client + Sync + 'static, { - let id = Id::try_from_str(ResourceType::Network, &req.id)?; + let id = Id::new(ResourceType::Network, *req.id); debug!("creating network with id {id}"); @@ -283,11 +282,11 @@ impl Service { where D: Client + Sync + 'static, { - let id = Id::try_from_str(ResourceType::Container, &req.id)?; + let id = Id::new(ResourceType::Container, *req.id); debug!("creating container with id {id}"); - let deps = req.dependencies()?; + let deps = req.dependencies(); let container = Container::try_from(req)?; @@ -302,15 +301,15 @@ impl Service { where D: Client + Sync + 'static, { - let id = Id::try_from_str(ResourceType::Deployment, &req.id)?; + let id = Id::new(ResourceType::Deployment, *req.id); debug!("creating deployment with id {id}"); let deps: Vec = req .containers .iter() - .map(|id| Id::try_from_str(ResourceType::Container, id)) - .try_collect()?; + .map(|id| Id::new(ResourceType::Container, **id)) + .collect(); self.create(id, deps, NodeType::Deployment).await?; @@ -658,15 +657,6 @@ impl Id { Self { rt, id } } - pub(crate) fn try_from_str(rt: ResourceType, id: &str) -> Result { - let id = Uuid::from_str(id).map_err(|err| ServiceError::Uuid { - id: id.to_string(), - source: err, - })?; - - Ok(Self { rt, id }) - } - pub(crate) fn uuid(&self) -> &Uuid { &self.id } @@ -1160,8 +1150,12 @@ mod tests { let image_req = ContainerRequest::from_event(create_image_req).unwrap(); - let create_container_req = - create_container_request_event(container_id, &image_id.to_string(), reference, &[]); + let create_container_req = create_container_request_event( + container_id, + &image_id.to_string(), + reference, + &Vec::::new(), + ); let container_req = ContainerRequest::from_event(create_container_req).unwrap();