github/kata-containers
1
0
Fork 2
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-10-31 17:37:20 +00:00
Code Issues Projects Releases Wiki Activity

agent: Address linter and tests

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

Fixes: #1294

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
This commit is contained in:
Maksym Pavlenko
2021-01-19 20:23:11 -08:00
parent 96762ab7ab
commit fa93831f66
3 changed files with 55 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
src/agent/src/netlink.rs
View File

@@ -1,5 +1,9 @@
use anyhow::Context;
use anyhow::{anyhow, Result};
// Copyright (c) 2021 Kata Maintainers
//
// 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")

4
src/agent/src/rpc.rs
View File

@@ -1702,8 +1702,8 @@ mod tests {
assert!(result.is_ok(), "load module should success");
}
#[test]
fn test_append_guest_hooks() {
#[tokio::test]
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 {

37
src/agent/src/sandbox.rs
View File

@@ -433,8 +433,8 @@ mod tests {
baremount.mount()
}
#[test]
fn set_sandbox_storage() {
#[tokio::test]
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]
fn remove_sandbox_storage() {
#[tokio::test]
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]
fn unset_sandbox_storage() {
#[tokio::test]
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]
fn get_container_entry_exist() {
#[tokio::test]
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]
fn get_container_no_entry() {
#[tokio::test]
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]
pub fn test_sandbox_is_running() {
#[tokio::test]
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]
fn test_sandbox_set_hostname() {
#[tokio::test]
async fn test_sandbox_set_hostname() {
let logger = slog::Logger::root(slog::Discard, o!());
let mut s = Sandbox::new(&logger).unwrap();
let hostname = "abc123";