Fix clippy warnings, other cleanups

Signed-off-by: Santtu Lakkala <[email protected]>

client/src/client.rs

@@ -4,26 +4,21 @@ use async_channel::Receiver;
 use givc_common::pb;
 pub use givc_common::query::{Event, QueryResult};
 use givc_common::types::*;
+use std::future::Future;
+use std::pin::Pin;
 use tokio_stream::StreamExt;
 use tonic::transport::Channel;
 use tracing::debug;
 
 type Client = pb::admin_service_client::AdminServiceClient<Channel>;
 
-#[derive(Debug)]
 pub struct WatchResult {
     pub initial: Vec<QueryResult>,
     // Design defence: we use `async-channel` here, as it could be used with both
     // tokio's and glib's eventloop, and recommended by gtk4-rs developers:
     pub channel: Receiver<Event>,
 
-    task: tokio::task::JoinHandle<()>,
-}
-
-impl Drop for WatchResult {
-    fn drop(&mut self) {
-        self.task.abort()
-    }
+    pub task: Pin<Box<dyn Future<Output = ()>>>,
 }
 
 #[derive(Debug)]
@@ -39,20 +34,20 @@ impl AdminClient {
 
     // New style api, not yet implemented, stub atm to make current code happy
     // FIXME: Still doubt if constructor should be sync or async
-    pub fn new(addr: String, port: u16, tls_info: Option<(String, TlsConfig)>) -> Self {
-        let (name, tls) = match tls_info {
+    pub fn new(address: String, port: u16, tls_info: Option<(String, TlsConfig)>) -> Self {
+        let (tls_name, tls) = match tls_info {
             Some((name, tls)) => (name, Some(tls)),
             None => (String::from("bogus(no tls)"), None),
         };
         Self {
             endpoint: EndpointConfig {
                 transport: TransportConfig {
-                    address: addr,
-                    port: port,
+                    address,
+                    port,
                     protocol: String::from("bogus"),
-                    tls_name: name,
+                    tls_name,
                 },
-                tls: tls,
+                tls,
             },
         }
     }
@@ -92,8 +87,7 @@ impl AdminClient {
             vm_name,
             args,
         };
-        let response = self.connect_to().await?.start_application(request).await?;
-        // Ok(response.into_inner().cmd_status)
+        let _response = self.connect_to().await?.start_application(request).await?;
         Ok(())
     }
     pub async fn stop(&self, _app: String) -> anyhow::Result<()> {
@@ -164,6 +158,8 @@ impl AdminClient {
     }
 
     pub async fn watch(&self) -> anyhow::Result<WatchResult> {
+        use pb::admin::watch_item::Status;
+        use pb::admin::WatchItem;
         let (tx, rx) = async_channel::bounded::<Event>(10);
 
         let mut watch = self
@@ -174,15 +170,13 @@ impl AdminClient {
             .into_inner();
 
         let list = match watch.try_next().await? {
-            Some(first) => match first.status {
-                Some(pb::admin::watch_item::Status::Initial(init)) => QueryResult::parse_list(init.list)?,
-                Some(item) => bail!("Protocol error, first item in stream not pb::admin::watch_item::Status::Initial, {:?}", item),
-                None => bail!("Protocol error, initial item missing"),
-            },
+            Some(WatchItem { status: Some(Status::Initial(init)) }) => QueryResult::parse_list(init.list)?,
+            Some(WatchItem { status: Some(item) }) => bail!("Protocol error, first item in stream not pb::admin::watch_item::Status::Initial, {:?}", item),
+            Some(_) => bail!("Protocol error, initial item missing"),
             None => bail!("Protocol error, status field missing"),
         };
 
-        let task = tokio::task::spawn(async move {
+        let task = async move {
             loop {
                 if let Ok(Some(event)) = watch.try_next().await {
                     let event = match Event::try_from(event) {
@@ -201,12 +195,12 @@ impl AdminClient {
                     break;
                 }
             }
-        });
+        };
 
         let result = WatchResult {
             initial: list,
             channel: rx,
-            task,
+            task: Box::pin(task),
         };
         Ok(result)
     }

client/src/endpoint.rs

@@ -25,11 +25,11 @@ pub struct EndpointConfig {
 
 impl TlsConfig {
     pub fn client_config(&self) -> anyhow::Result<ClientTlsConfig> {
-        let pem = std::fs::read_to_string(self.ca_cert_file_path.as_os_str())?;
+        let pem = std::fs::read(&self.ca_cert_file_path)?;
         let ca = Certificate::from_pem(pem);
 
-        let client_cert = std::fs::read(self.cert_file_path.as_os_str())?;
-        let client_key = std::fs::read(self.key_file_path.as_os_str())?;
+        let client_cert = std::fs::read(&self.cert_file_path)?;
+        let client_key = std::fs::read(&self.key_file_path)?;
         let client_identity = Identity::from_pem(client_cert, client_key);
         let tls_name = self
             .tls_name
@@ -42,8 +42,8 @@ impl TlsConfig {
     }
 
     pub fn server_config(&self) -> anyhow::Result<ServerTlsConfig> {
-        let cert = std::fs::read(self.cert_file_path.as_os_str())?;
-        let key = std::fs::read(self.key_file_path.as_os_str())?;
+        let cert = std::fs::read(&self.cert_file_path)?;
+        let key = std::fs::read(&self.key_file_path)?;
         let identity = Identity::from_pem(cert, key);
         let config = ServerTlsConfig::new().identity(identity);
         Ok(config)

common/src/query.rs

@@ -55,13 +55,13 @@ impl TryFrom<pb::QueryListItem> for QueryResult {
     }
 }
 
-impl Into<pb::QueryListItem> for QueryResult {
-    fn into(self) -> pb::QueryListItem {
-        pb::QueryListItem {
-            name: self.name,
-            description: self.description,
-            vm_status: self.status.to_string(),
-            trust_level: self.trust_level.to_string(),
+impl From<QueryResult> for pb::QueryListItem {
+    fn from(val: QueryResult) -> Self {
+        Self {
+            name: val.name,
+            description: val.description,
+            vm_status: val.status.to_string(),
+            trust_level: val.trust_level.to_string(),
         }
     }
 }
@@ -117,12 +117,12 @@ impl TryFrom<pb::WatchItem> for Event {
     }
 }
 
-impl Into<pb::WatchItem> for Event {
-    fn into(self) -> pb::WatchItem {
-        match self {
-            Event::UnitRegistered(value) => Self::watch_item(Status::Added(value.into())),
-            Event::UnitStatusChanged(value) => Self::watch_item(Status::Updated(value.into())),
-            Event::UnitShutdown(value) => Self::watch_item(Status::Removed(value.into())),
+impl From<Event> for pb::WatchItem {
+    fn from(val: Event) -> Self {
+        match val {
+            Event::UnitRegistered(value) => Event::watch_item(Status::Added(value.into())),
+            Event::UnitStatusChanged(value) => Event::watch_item(Status::Updated(value.into())),
+            Event::UnitShutdown(value) => Event::watch_item(Status::Removed(value.into())),
         }
     }
 }

common/src/types.rs

@@ -1,7 +1,7 @@
 // This module contain literal translations of types from internal/pkgs/types/types.go
 // Some of them would be rewritten, replaced, or even removed
 use crate::pb;
-use anyhow::{anyhow, bail};
+use anyhow::bail;
 use std::convert::{Into, TryFrom};
 
 #[derive(Debug, Copy, Clone, PartialEq)]
@@ -104,30 +104,30 @@ impl TryFrom<u32> for UnitType {
 // so we can use just Into<u32> trait
 // FIXME:  Combination of `UnitType{ vm: Host, service: VM}` is ILLEGAL!!!
 //         Should we use TryInto, or fix type system?
-impl Into<u32> for UnitType {
-    fn into(self) -> u32 {
+impl From<UnitType> for u32 {
+    fn from(val: UnitType) -> Self {
         use ServiceType::*;
         use VmType::*;
-        match self.vm {
-            Host => match self.service {
+        match val.vm {
+            Host => match val.service {
                 Mgr => 0,
                 Svc => 1,
                 App => 2,
                 VM => 100500,
             },
-            AdmVM => match self.service {
+            AdmVM => match val.service {
                 VM => 3,
                 Mgr => 4,
                 Svc => 5,
                 App => 6,
             },
-            SysVM => match self.service {
+            SysVM => match val.service {
                 VM => 7,
                 Mgr => 8,
                 Svc => 9,
                 App => 10,
             },
-            AppVM => match self.service {
+            AppVM => match val.service {
                 VM => 11,
                 Mgr => 12,
                 Svc => 13,
@@ -172,14 +172,14 @@ impl TryFrom<pb::UnitStatus> for UnitStatus {
     }
 }
 
-impl Into<pb::UnitStatus> for UnitStatus {
-    fn into(self) -> pb::UnitStatus {
-        pb::UnitStatus {
-            name: self.name,
-            description: self.description,
-            load_state: self.load_state,
-            active_state: self.active_state,
-            path: self.path,
+impl From<UnitStatus> for pb::UnitStatus {
+    fn from(val: UnitStatus) -> Self {
+        Self {
+            name: val.name,
+            description: val.description,
+            load_state: val.load_state,
+            active_state: val.active_state,
+            path: val.path,
         }
     }
 }
@@ -206,13 +206,13 @@ impl TryFrom<pb::TransportConfig> for EndpointEntry {
     }
 }
 
-impl Into<pb::TransportConfig> for EndpointEntry {
-    fn into(self) -> pb::TransportConfig {
+impl From<EndpointEntry> for pb::TransportConfig {
+    fn from(val: EndpointEntry) -> Self {
         pb::TransportConfig {
-            protocol: self.protocol,
-            address: self.address,
-            port: self.port.to_string(),
-            name: self.tls_name,
+            protocol: val.protocol,
+            address: val.address,
+            port: val.port.to_string(),
+            name: val.tls_name,
         }
     }
 }

flake.nix

@@ -66,13 +66,14 @@
                   ./internal
                 ];
               };
-            in
-            rec {
-              givc-agent = pkgs.callPackage ./nixos/packages/givc-agent.nix { inherit src; };
               givc-admin-rs = pkgs.callPackage ./nixos/packages/givc-admin-rs.nix {
                 inherit crane;
                 src = ./.;
               };
+            in
+            {
+              inherit givc-admin-rs;
+              givc-agent = pkgs.callPackage ./nixos/packages/givc-agent.nix { inherit src; };
               givc-cli = givc-admin-rs.cli;
             };
         };

internal/cmd/givc-agent/main.go

@@ -27,7 +27,7 @@ import (
 func main() {
 
 	var err error
-	log.Infof("Executing %s", filepath.Base(os.Args[0]))
+	log.Infof("Running %s", filepath.Base(os.Args[0]))
 
 	name := os.Getenv("NAME")
 	if name == "" {

src/admin/entry.rs

@@ -2,7 +2,7 @@
 // Some of them would be rewritten, replaced, or even removed
 use crate::pb;
 use anyhow::anyhow;
-use std::convert::{Into, TryFrom};
+use std::convert::TryFrom;
 
 use givc_common::query::*;
 use givc_common::types::*;
@@ -83,25 +83,25 @@ impl TryFrom<pb::RegistryRequest> for RegistryEntry {
         // Protocol very inconsistent here
         Ok(Self {
             name: req.name,
-            status: status,
-            watch: watch,
+            status,
+            watch,
             r#type: ty,
             placement: Placement::Endpoint(endpoint),
         })
     }
 }
 
-impl Into<QueryResult> for RegistryEntry {
-    fn into(self) -> QueryResult {
-        let status = if self.status.is_running() {
+impl From<RegistryEntry> for QueryResult {
+    fn from(val: RegistryEntry) -> Self {
+        let status = if val.status.is_running() {
             VMStatus::Running
         } else {
             VMStatus::PoweredOff
         };
         QueryResult {
-            name: self.name,
-            description: self.status.description,
-            status: status,
+            name: val.name,
+            description: val.status.description,
+            status,
             trust_level: TrustLevel::default(),
         }
     }

src/admin/registry.rs

@@ -18,6 +18,12 @@ pub struct Registry {
     pubsub: broadcast::Sender<Event>,
 }
 
+impl Default for Registry {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl Registry {
     pub fn new() -> Self {
         Self {
@@ -34,6 +40,7 @@ impl Registry {
             info!("Replaced old entry {:#?}", old);
             self.send_event(Event::UnitShutdown(old.into()))
         };
+        info!("Sending event {event:?}");
         self.send_event(event)
     }
 
@@ -45,7 +52,10 @@ impl Registry {
                 self.send_event(Event::UnitShutdown(entry.into()));
                 Ok(())
             }
-            None => bail!("Can't deregister entry {}, it not registered", name),
+            None => Err(anyhow!(
+                "Can't deregister entry {}, it not registered",
+                name
+            )),
         }
     }
 
@@ -64,7 +74,7 @@ impl Registry {
             .filter(|x| x.starts_with(name))
             .cloned()
             .collect();
-        if list.len() == 0 {
+        if list.is_empty() {
             bail!("No entries match string {}", name)
         } else {
             Ok(list)
@@ -115,7 +125,7 @@ impl Registry {
                 e.status = status;
                 self.send_event(Event::UnitStatusChanged(e.clone().into()))
             })
-            .ok_or_else(|| anyhow!("Can't update state for {}, is not registered", name))
+            .ok_or_else(|| anyhow!("Can't update state for {name}, is not registered"))
     }
 
     // FIXME: Should we dump full contents here for `query`/`query_list` high-level API
@@ -156,7 +166,7 @@ mod tests {
         r.register(bar);
 
         assert!(r.contains(&foo_key));
-        assert!(r.contains(&"bar".to_string()));
+        assert!(r.contains("bar"));
 
         let foo1 = r.by_name(&foo_key)?;
         assert_eq!(foo1, foo);