From e6c6d991dc057d239e77ec1193591d20728f603e Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 3 May 2024 14:44:54 +0200 Subject: [PATCH 1/3] fix ncat sctp tests nmap-ncat seem to have weird bugs in that the received data is only printed on stdout when there is a no data on stdin, not stdin because the returns EOF when it gets read and if there is any data then ncat fails as well as it cannot write it to the remote[1]... So just try to emulate like how it works in a terminal by creating an anonymous pipe that contains no data so ncat is happy and prints our test string as expected. [1] https://github.com/nmap/nmap/issues/2829 Signed-off-by: Paul Holzinger --- test/helpers.bash | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/helpers.bash b/test/helpers.bash index 3ae59412e..93b480d00 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -621,17 +621,18 @@ function run_nc_test() { local host_port=$5 local nc_common_args="" - local stdin=/dev/null + exec {stdin}<>/dev/null case $proto in tcp) ;; # nothing to do (default) udp) nc_common_args=--udp ;; sctp) nc_common_args=--sctp - # for some reason we have to attach STDIN to the server only for the sctp proto - # otherwise it will just exit for unknown reasons. However we must not add STDIN - # to udp and tcp otherwise those tests will fail. - stdin=/dev/zero + # For some reason we have to attach a empty STDIN (not /dev/null and not something with data in it) + # to the server only for the sctp proto otherwise it will just exit for weird reasons. + # As such create a empty anonymous pipe to work around that. + # https://github.com/nmap/nmap/issues/2829 + exec {stdin}<> <(:) ;; *) die "unknown port proto '$proto'" ;; esac @@ -644,7 +645,7 @@ function run_nc_test() { fi nsenter -n -t "${CONTAINER_NS_PIDS[$container_ns]}" timeout --foreground -v --kill=10 5 \ - nc $nc_common_args -l -p $container_port &>"$NETAVARK_TMPDIR/nc-out" <$stdin & + nc $nc_common_args -l -p $container_port &>"$NETAVARK_TMPDIR/nc-out" <&$stdin & # make sure to wait until port is bound otherwise test can flake # https://github.com/containers/netavark/issues/433 @@ -661,6 +662,9 @@ function run_nc_test() { got=$(cat "$NETAVARK_TMPDIR/nc-out") assert "$got" == "$data" "ncat received data" + + # close the fd + exec {stdin}>&- } ################# From ed7cfd837d096ec006c26d7c78ea70f1cf5ea635 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 3 May 2024 14:51:50 +0200 Subject: [PATCH 2/3] Update CI image to fedora 40 Contains a new ncat with udp bugs fixes, we still need the sctp workaround though. from https://github.com/containers/automation_images/pull/349 Signed-off-by: Paul Holzinger --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 2ec76bea4..2680eb741 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -16,7 +16,7 @@ env: CARGO_TARGET_DIR: "$CIRRUS_WORKING_DIR/targets" # Save a little typing (path relative to $CIRRUS_WORKING_DIR) SCRIPT_BASE: "./contrib/cirrus" - IMAGE_SUFFIX: "c20240102t155643z-f39f38d13" + IMAGE_SUFFIX: "c20240506t132946z-f40f39d13" FEDORA_NETAVARK_IMAGE: "fedora-netavark-${IMAGE_SUFFIX}" AARDVARK_DNS_BRANCH: "main" AARDVARK_DNS_URL: "https://api.cirrus-ci.com/v1/artifact/github/containers/aardvark-dns/success/binary.zip?branch=${AARDVARK_DNS_BRANCH}" From 87fe59fd08736adc93fc10f4ede36ca069159457 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 6 May 2024 14:45:02 +0200 Subject: [PATCH 3/3] fix new clippy warnings Remove some unused ipva code fromt he dhcp proxy, I don't see us needing this anythime soon and we can always add it back later. Signed-off-by: Paul Holzinger --- src/dhcp_proxy/ip.rs | 41 ++--------------------------- src/dhcp_proxy/lib.rs | 17 +----------- src/dhcp_proxy/types.rs | 6 ++--- src/dns/aardvark.rs | 3 ++- src/network/macvlan_dhcp.rs | 52 +++++++++++++++++-------------------- src/network/vlan.rs | 2 +- 6 files changed, 33 insertions(+), 88 deletions(-) diff --git a/src/dhcp_proxy/ip.rs b/src/dhcp_proxy/ip.rs index 4f47edb56..396c2f064 100644 --- a/src/dhcp_proxy/ip.rs +++ b/src/dhcp_proxy/ip.rs @@ -12,36 +12,9 @@ use crate::network::netlink; use crate::network::netlink::Socket; use ipnet::IpNet; use log::debug; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::net::{IpAddr, Ipv4Addr}; use std::str::FromStr; -trait IpConv { - fn to_v4(&self) -> Result<&Ipv4Addr, ProxyError>; - fn to_v6(&self) -> Result<&Ipv6Addr, ProxyError>; -} - -// Simple implementation for converting from IPAddr to -// specific IP type -impl IpConv for IpAddr { - fn to_v4(&self) -> Result<&Ipv4Addr, ProxyError> { - match self { - IpAddr::V4(ip) => Ok(ip), - IpAddr::V6(_) => Err(ProxyError::new( - "invalid value for ipv4 conversion".to_string(), - )), - } - } - - fn to_v6(&self) -> Result<&Ipv6Addr, ProxyError> { - match self { - IpAddr::V4(_) => Err(ProxyError::new( - "invalid value for ipv6 conversion".to_string(), - )), - IpAddr::V6(ip) => Ok(ip), - } - } -} - /* Information that came back in the DHCP lease like name_servers, domain and host names, etc. will be implemented in podman; not here. @@ -63,7 +36,6 @@ trait Address { Self: Sized; fn add_ip(&self, nls: &mut Socket) -> Result<(), ProxyError>; fn add_gws(&self, nls: &mut Socket) -> Result<(), ProxyError>; - fn remove(self) -> Result<(), ProxyError>; } fn handle_gws(g: Vec, netmask: &str) -> Result, ProxyError> { @@ -122,7 +94,7 @@ impl Address for MacVLAN { let gateways = match handle_gws(l.gateways.clone(), &l.subnet_mask) { Ok(g) => g, Err(e) => { - return Err(ProxyError::new(format!("bad gateways: {}", e.to_string()))); + return Err(ProxyError::new(format!("bad gateways: {}", e))); } }; let prefix_length = match get_prefix_length_v4(&l.subnet_mask) { @@ -158,15 +130,6 @@ impl Address for MacVLAN { Err(e) => Err(ProxyError::new(e.to_string())), } } - - /* - For now, nv will remove the interface; this causes all IP stuff - to fold. - */ - fn remove(self) -> Result<(), ProxyError> { - debug!("removing interface {}", self.interface); - todo!() - } } // setup takes the DHCP lease and some additional information and diff --git a/src/dhcp_proxy/lib.rs b/src/dhcp_proxy/lib.rs index 6179a927b..848b76b05 100644 --- a/src/dhcp_proxy/lib.rs +++ b/src/dhcp_proxy/lib.rs @@ -9,7 +9,7 @@ use g_rpc::netavark_proxy_client::NetavarkProxyClient; use log::debug; use std::fs::File; use std::net::AddrParseError; -use std::net::{Ipv4Addr, Ipv6Addr}; +use std::net::Ipv4Addr; use std::str::FromStr; use tokio::net::UnixStream; use tonic::transport::{Channel, Endpoint, Uri}; @@ -271,7 +271,6 @@ impl NetworkConfig { trait VectorConv { fn to_v4_addrs(&self) -> Result>, AddrParseError>; - fn to_v6_addrs(&self) -> Result>, AddrParseError>; } impl VectorConv for Vec { @@ -288,18 +287,4 @@ impl VectorConv for Vec { } Ok(Some(out_addrs)) } - - fn to_v6_addrs(&self) -> Result>, AddrParseError> { - if self.is_empty() { - return Ok(None); - } - let mut out_addrs = Vec::new(); - for ip in self { - match Ipv6Addr::from_str(ip) { - Ok(i) => out_addrs.push(i), - Err(e) => return Err(e), - }; - } - Ok(Some(out_addrs)) - } } diff --git a/src/dhcp_proxy/types.rs b/src/dhcp_proxy/types.rs index 86dc4a19b..83e637afd 100644 --- a/src/dhcp_proxy/types.rs +++ b/src/dhcp_proxy/types.rs @@ -40,9 +40,9 @@ impl CustomErr for ProxyError { } } -impl ToString for ProxyError { - fn to_string(&self) -> String { - self.0.to_string() +impl std::fmt::Display for ProxyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) } } diff --git a/src/dns/aardvark.rs b/src/dns/aardvark.rs index 276618eed..9bad282f9 100644 --- a/src/dns/aardvark.rs +++ b/src/dns/aardvark.rs @@ -202,6 +202,7 @@ impl Aardvark { .read(true) .write(true) .create(true) + .truncate(true) .open(&lockfile_path) { Ok(file) => file, @@ -355,7 +356,7 @@ impl Aardvark { pub fn modify_network_dns_servers( &self, network_name: &str, - network_dns_servers: &Vec, + network_dns_servers: &[String], ) -> NetavarkResult<()> { let mut dns_servers_modified = false; let path = Path::new(&self.config).join(network_name); diff --git a/src/network/macvlan_dhcp.rs b/src/network/macvlan_dhcp.rs index 12bab73ab..ed07530af 100644 --- a/src/network/macvlan_dhcp.rs +++ b/src/network/macvlan_dhcp.rs @@ -42,20 +42,18 @@ pub fn get_dhcp_lease( container_iface: container_network_interface.to_string(), container_mac_addr: container_macvlan_mac.to_string(), }; - let lease = match { - tokio::task::LocalSet::new().block_on( - match &tokio::runtime::Builder::new_current_thread() - .enable_io() - .build() - { - Ok(r) => r, - Err(e) => { - return Err(NetavarkError::msg(format!("unable to build thread: {e}"))); - } - }, - nvp_config.get_lease(DEFAULT_UDS_PATH), - ) - } { + let lease = match tokio::task::LocalSet::new().block_on( + match &tokio::runtime::Builder::new_current_thread() + .enable_io() + .build() + { + Ok(r) => r, + Err(e) => { + return Err(NetavarkError::msg(format!("unable to build thread: {e}"))); + } + }, + nvp_config.get_lease(DEFAULT_UDS_PATH), + ) { Ok(l) => l, Err(e) => { return Err(NetavarkError::msg(format!("unable to obtain lease: {e}"))); @@ -114,20 +112,18 @@ pub fn release_dhcp_lease( container_iface: container_network_interface.to_string(), container_mac_addr: container_macvlan_mac.to_string(), }; - match { - tokio::task::LocalSet::new().block_on( - match &tokio::runtime::Builder::new_current_thread() - .enable_io() - .build() - { - Ok(r) => r, - Err(e) => { - return Err(NetavarkError::msg(format!("unable to build thread: {e}"))); - } - }, - nvp_config.drop_lease(DEFAULT_UDS_PATH), - ) - } { + match tokio::task::LocalSet::new().block_on( + match &tokio::runtime::Builder::new_current_thread() + .enable_io() + .build() + { + Ok(r) => r, + Err(e) => { + return Err(NetavarkError::msg(format!("unable to build thread: {e}"))); + } + }, + nvp_config.drop_lease(DEFAULT_UDS_PATH), + ) { Ok(_) => {} Err(e) => { return Err(NetavarkError::Message(e.to_string())); diff --git a/src/network/vlan.rs b/src/network/vlan.rs index 83e84c214..6434d9427 100644 --- a/src/network/vlan.rs +++ b/src/network/vlan.rs @@ -307,7 +307,7 @@ fn setup( let random = Alphanumeric.sample_string(&mut rand::thread_rng(), 10); let tmp_name = "mv-".to_string() + &random; let mut opts = opts.clone(); - opts.name = tmp_name.clone(); + opts.name.clone_from(&tmp_name); result = host.create_link(opts); if let Err(ref e) = result { // if last element return directly