agent: Address linter and tests

- Fix clippy complains
- Use #[tokio::test] for async tests
- Improve IPv6 check

Fixes: #1294

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>

src/agent/src/netlink.rs

@@ -1,5 +1,9 @@
-use anyhow::Context;
+// Copyright (c) 2021 Kata Maintainers
-use anyhow::{anyhow, Result};
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+use anyhow::{anyhow, Context, Result};
 use futures::{future, StreamExt, TryStreamExt};
 use ipnetwork::{IpNetwork, Ipv4Network, Ipv6Network};
 use protobuf::RepeatedField;
@@ -189,7 +193,7 @@ impl Handle {
         };
 
         next.map(|msg| msg.into())
-            .ok_or(anyhow!("Link not found ({})", filter))
+            .ok_or_else(|| anyhow!("Link not found ({})", filter))
     }
 
     async fn list_links(&self) -> Result<Vec<Link>> {
@@ -313,12 +317,12 @@ impl Handle {
         // Split the list so we add routes with no gateway first.
         // Note: `partition_in_place` is a better fit here, since it reorders things inplace (instead of
         // allocating two separate collections), however it's not yet in stable Rust.
-        let (a, b): (Vec<Route>, Vec<Route>) = list.into_iter().partition(|p| p.gateway.len() == 0);
+        let (a, b): (Vec<Route>, Vec<Route>) = list.into_iter().partition(|p| p.gateway.is_empty());
         let list = a.iter().chain(&b);
 
         for route in list {
             let link = self.find_link(LinkFilter::Name(&route.device)).await?;
-            let is_v6 = route.gateway.contains("::") || route.dest.contains("::");
+            let is_v6 = is_ipv6(route.get_gateway()) || is_ipv6(route.get_dest());
 
             const MAIN_TABLE: u8 = packet::constants::RT_TABLE_MAIN;
             const UNICAST: u8 = packet::constants::RTN_UNICAST;
@@ -332,7 +336,7 @@ impl Handle {
             // This if branch is a bit clumsy because it does almost the same.
             // TODO: Simplify this once https://github.com/little-dude/netlink/pull/140 is merged and released
             if is_v6 {
-                let dest_addr = if route.dest.len() > 0 {
+                let dest_addr = if !route.dest.is_empty() {
                     Ipv6Network::from_str(&route.dest)?
                 } else {
                     Ipv6Network::new(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0), 0)?
@@ -350,7 +354,7 @@ impl Handle {
                     .destination_prefix(dest_addr.ip(), dest_addr.prefix())
                     .output_interface(link.index());
 
-                if route.source.len() > 0 {
+                if !route.source.is_empty() {
                     let network = Ipv6Network::from_str(&route.source)?;
                     if network.prefix() > 0 {
                         request = request.source_prefix(network.ip(), network.prefix());
@@ -362,7 +366,7 @@ impl Handle {
                     }
                 }
 
-                if route.gateway.len() > 0 {
+                if !route.gateway.is_empty() {
                     let ip = Ipv6Addr::from_str(&route.gateway)?;
                     request = request.gateway(ip);
                 }
@@ -376,7 +380,7 @@ impl Handle {
                     )
                 })?;
             } else {
-                let dest_addr = if route.dest.len() > 0 {
+                let dest_addr = if !route.dest.is_empty() {
                     Ipv4Network::from_str(&route.dest)?
                 } else {
                     Ipv4Network::new(Ipv4Addr::new(0, 0, 0, 0), 0)?
@@ -394,7 +398,7 @@ impl Handle {
                     .destination_prefix(dest_addr.ip(), dest_addr.prefix())
                     .output_interface(link.index());
 
-                if route.source.len() > 0 {
+                if !route.source.is_empty() {
                     let network = Ipv4Network::from_str(&route.source)?;
                     if network.prefix() > 0 {
                         request = request.source_prefix(network.ip(), network.prefix());
@@ -406,7 +410,7 @@ impl Handle {
                     }
                 }
 
-                if route.gateway.len() > 0 {
+                if !route.gateway.is_empty() {
                     let ip = Ipv4Addr::from_str(&route.gateway)?;
                     request = request.gateway(ip);
                 }
@@ -605,8 +609,12 @@ fn format_address(data: &[u8]) -> Result<String> {
     }
 }
 
+fn is_ipv6(str: &str) -> bool {
+    Ipv6Addr::from_str(str).is_ok()
+}
+
 fn parse_mac_address(addr: &str) -> Result<[u8; 6]> {
-    let mut split = addr.splitn(6, ":");
+    let mut split = addr.splitn(6, ':');
 
     // Parse single Mac address block
     let mut parse_next = || -> Result<u8> {
@@ -648,7 +656,7 @@ impl Link {
                     None
                 }
             })
-            .unwrap_or(String::new())
+            .unwrap_or_default()
     }
 
     /// Extract Mac address.
@@ -663,7 +671,7 @@ impl Link {
                     None
                 }
             })
-            .unwrap_or(String::new())
+            .unwrap_or_default()
     }
 
     /// Returns whether the link is UP
@@ -750,7 +758,7 @@ impl Address {
                     None
                 }
             })
-            .unwrap_or(String::new())
+            .unwrap_or_default()
     }
 
     fn local(&self) -> String {
@@ -765,7 +773,7 @@ impl Address {
                     None
                 }
             })
-            .unwrap_or(String::new())
+            .unwrap_or_default()
     }
 }
 
@@ -939,6 +947,16 @@ mod tests {
         assert_eq!(bytes, [0xAB, 0x0C, 0xDE, 0x12, 0x34, 0x56]);
     }
 
+    #[test]
+    fn check_ipv6() {
+        assert!(is_ipv6("::1"));
+        assert!(is_ipv6("2001:0:3238:DFE1:63::FEFB"));
+
+        assert!(!is_ipv6(""));
+        assert!(!is_ipv6("127.0.0.1"));
+        assert!(!is_ipv6("10.10.10.10"));
+    }
+
     fn clean_env_for_test_add_one_arp_neighbor(dummy_name: &str, ip: &str) {
         // ip link delete dummy
         Command::new("ip")

src/agent/src/rpc.rs

@@ -1702,8 +1702,8 @@ mod tests {
         assert!(result.is_ok(), "load module should success");
     }
 
-    #[test]
+    #[tokio::test]
-    fn test_append_guest_hooks() {
+    async fn test_append_guest_hooks() {
         let logger = slog::Logger::root(slog::Discard, o!());
         let mut s = Sandbox::new(&logger).unwrap();
         s.hooks = Some(Hooks {

src/agent/src/sandbox.rs

@@ -433,8 +433,8 @@ mod tests {
         baremount.mount()
     }
 
-    #[test]
+    #[tokio::test]
-    fn set_sandbox_storage() {
+    async fn set_sandbox_storage() {
         let logger = slog::Logger::root(slog::Discard, o!());
         let mut s = Sandbox::new(&logger).unwrap();
 
@@ -467,8 +467,8 @@ mod tests {
         );
     }
 
-    #[test]
+    #[tokio::test]
-    fn remove_sandbox_storage() {
+    async fn remove_sandbox_storage() {
         skip_if_not_root!();
 
         let logger = slog::Logger::root(slog::Discard, o!());
@@ -523,9 +523,9 @@ mod tests {
         assert!(s.remove_sandbox_storage(destdir_path).is_ok());
     }
 
-    #[test]
+    #[tokio::test]
     #[allow(unused_assignments)]
-    fn unset_and_remove_sandbox_storage() {
+    async fn unset_and_remove_sandbox_storage() {
         skip_if_not_root!();
 
         let logger = slog::Logger::root(slog::Discard, o!());
@@ -575,8 +575,8 @@ mod tests {
         assert!(s.unset_and_remove_sandbox_storage(&other_dir_str).is_err());
     }
 
-    #[test]
+    #[tokio::test]
-    fn unset_sandbox_storage() {
+    async fn unset_sandbox_storage() {
         let logger = slog::Logger::root(slog::Discard, o!());
         let mut s = Sandbox::new(&logger).unwrap();
 
@@ -658,8 +658,8 @@ mod tests {
         .unwrap()
     }
 
-    #[test]
+    #[tokio::test]
-    fn get_container_entry_exist() {
+    async fn get_container_entry_exist() {
         skip_if_not_root!();
         let logger = slog::Logger::root(slog::Discard, o!());
         let mut s = Sandbox::new(&logger).unwrap();
@@ -671,8 +671,8 @@ mod tests {
         assert!(cnt.is_some());
     }
 
-    #[test]
+    #[tokio::test]
-    fn get_container_no_entry() {
+    async fn get_container_no_entry() {
         let logger = slog::Logger::root(slog::Discard, o!());
         let mut s = Sandbox::new(&logger).unwrap();
 
@@ -690,8 +690,9 @@ mod tests {
         s.add_container(linux_container);
         assert!(s.get_container("some_id").is_some());
     }
-    #[test]
+
-    fn update_shared_pidns() {
+    #[tokio::test]
+    async fn update_shared_pidns() {
         skip_if_not_root!();
         let logger = slog::Logger::root(slog::Discard, o!());
         let mut s = Sandbox::new(&logger).unwrap();
@@ -731,8 +732,8 @@ mod tests {
         assert!(s.hooks.as_ref().unwrap().poststop.is_empty());
     }
 
-    #[test]
+    #[tokio::test]
-    pub fn test_sandbox_is_running() {
+    async fn test_sandbox_is_running() {
         let logger = slog::Logger::root(slog::Discard, o!());
         let mut s = Sandbox::new(&logger).unwrap();
         s.running = true;
@@ -741,8 +742,8 @@ mod tests {
         assert!(!s.is_running());
     }
 
-    #[test]
+    #[tokio::test]
-    fn test_sandbox_set_hostname() {
+    async fn test_sandbox_set_hostname() {
         let logger = slog::Logger::root(slog::Discard, o!());
         let mut s = Sandbox::new(&logger).unwrap();
         let hostname = "abc123";