Merge pull request #978 from Luap99/sctp-test

fix ncat sctp test and update CI image

Author: openshift-merge-bot[bot]

.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}"

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<T> {
         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<String>, netmask: &str) -> Result<Vec<IpNet>, ProxyError> {
@@ -122,7 +94,7 @@ impl Address<Ipv4Addr> 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<Ipv4Addr> 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

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<Option<Vec<Ipv4Addr>>, AddrParseError>;
-    fn to_v6_addrs(&self) -> Result<Option<Vec<Ipv6Addr>>, AddrParseError>;
 }
 
 impl VectorConv for Vec<String> {
@@ -288,18 +287,4 @@ impl VectorConv for Vec<String> {
         }
         Ok(Some(out_addrs))
     }
-
-    fn to_v6_addrs(&self) -> Result<Option<Vec<Ipv6Addr>>, 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))
-    }
 }

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)
     }
 }
 

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<String>,
+        network_dns_servers: &[String],
     ) -> NetavarkResult<()> {
         let mut dns_servers_modified = false;
         let path = Path::new(&self.config).join(network_name);

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()));

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

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}>&-
 }
 
 #################