diff --git a/.cirrus.yml b/.cirrus.yml index 5133d9af..771dd0be 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: "c20241010t105554z-f40f39d13" + IMAGE_SUFFIX: "c20241107t210000z-f41f40d13" 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}" diff --git a/Makefile b/Makefile index 6b9b7d9b..4b0a18a8 100644 --- a/Makefile +++ b/Makefile @@ -130,7 +130,7 @@ integration: $(CARGO_TARGET_DIR) examples .PHONY: validate validate: $(CARGO_TARGET_DIR) $(CARGO) fmt --all -- --check - $(CARGO) clippy -p netavark@$(CRATE_VERSION) -- -D warnings + $(CARGO) clippy -p netavark -- -D warnings $(MAKE) docs .PHONY: vendor-tarball diff --git a/src/dhcp_proxy/cache.rs b/src/dhcp_proxy/cache.rs index 0ad98307..6d3c6cfb 100644 --- a/src/dhcp_proxy/cache.rs +++ b/src/dhcp_proxy/cache.rs @@ -288,7 +288,7 @@ mod cache_tests { let deserialized_lease = s .get(macaddr) .expect("Could not get the mac address from the map") - .get(0) + .first() .expect("Could not get lease from set of mac addresses") .clone(); // Assure that the amount of leases added is correct amount @@ -331,7 +331,7 @@ mod cache_tests { let deserialized_lease = s .get(macaddr) .expect("Could not get the mac address from the map") - .get(0) + .first() .expect("Could not get lease from set of mac addresses") .clone(); // Assure that the amount of leases added is correct amount @@ -354,7 +354,7 @@ mod cache_tests { let deserialized_lease = s .get(macaddr) .expect("Could not get the mac address from the map") - .get(0) + .first() .expect("Could not get lease from set of mac addresses") .clone(); @@ -418,7 +418,7 @@ mod cache_tests { let deserialized_lease = s .get(macaddr) .expect("Could not get the mac address from the map") - .get(0) + .first() .expect("Could not get lease from set of mac addresses") .clone(); // Assure that the amount of leases added is correct amount @@ -450,7 +450,7 @@ mod cache_tests { let deserialized_updated_lease = s .get(macaddr) .expect("Could not get lease from deserialized map") - .get(0) + .first() .expect("Could not find lease in set of multi-homing leases"); assert_eq!(deserialized_updated_lease, &new_lease); diff --git a/src/firewall/firewalld.rs b/src/firewall/firewalld.rs index 4c4623df..e4274d2e 100644 --- a/src/firewall/firewalld.rs +++ b/src/firewall/firewalld.rs @@ -168,25 +168,22 @@ impl firewall::FirewallDriver for FirewallD { // prevention - if two ports end up mapped to different containers, // that is not detected, and firewalld will allow it to happen. // Only one of them will win and be active, though. - match setup_portfw.port_mappings { - Some(ports) => { - for port in ports { - if !port.host_ip.is_empty() { + if let Some(ports) = setup_portfw.port_mappings { + for port in ports { + if !port.host_ip.is_empty() { + port_forwarding_rules + .append(Value::new(make_port_tuple(port, &port.host_ip)))?; + } else { + if let Some(v4) = setup_portfw.container_ip_v4 { port_forwarding_rules - .append(Value::new(make_port_tuple(port, &port.host_ip)))?; - } else { - if let Some(v4) = setup_portfw.container_ip_v4 { - port_forwarding_rules - .append(Value::new(make_port_tuple(port, &v4.to_string())))?; - } - if let Some(v6) = setup_portfw.container_ip_v6 { - port_forwarding_rules - .append(Value::new(make_port_tuple(port, &v6.to_string())))?; - } + .append(Value::new(make_port_tuple(port, &v4.to_string())))?; + } + if let Some(v6) = setup_portfw.container_ip_v6 { + port_forwarding_rules + .append(Value::new(make_port_tuple(port, &v6.to_string())))?; } } } - None => {} }; // dns port forwarding requires rich rules as we also want to match destination ip diff --git a/src/firewall/state.rs b/src/firewall/state.rs index 206a6fd8..d573c834 100644 --- a/src/firewall/state.rs +++ b/src/firewall/state.rs @@ -328,14 +328,9 @@ mod tests { let res = remove_fw_config(config_dir, network_id, container_id, true); assert!(res.is_ok(), "remove_fw_config failed"); - assert_eq!( - paths.net_conf_file.exists(), - false, - "net conf should not exists" - ); - assert_eq!( - paths.port_conf_file.exists(), - false, + assert!(!paths.net_conf_file.exists(), "net conf should not exists"); + assert!( + !paths.port_conf_file.exists(), "port conf should not exists" ); diff --git a/src/firewall/varktables/types.rs b/src/firewall/varktables/types.rs index f161df4c..3b74881d 100644 --- a/src/firewall/varktables/types.rs +++ b/src/firewall/varktables/types.rs @@ -538,110 +538,107 @@ pub fn get_port_forwarding_chains<'a>( } } - match pfwd.port_mappings { - Some(ports) => { - for i in ports { - let host_ip = if i.host_ip.is_empty() { - None - } else { - match i.host_ip.parse() { - Ok(ip) => match ip { - IpAddr::V4(v4) => { - if is_ipv6 { - continue; - } - if !v4.is_unspecified() { - Some(IpAddr::V4(v4)) - } else { - None - } + if let Some(ports) = pfwd.port_mappings { + for i in ports { + let host_ip = if i.host_ip.is_empty() { + None + } else { + match i.host_ip.parse() { + Ok(ip) => match ip { + IpAddr::V4(v4) => { + if is_ipv6 { + continue; } - IpAddr::V6(v6) => { - if !is_ipv6 { - continue; - } - if !v6.is_unspecified() { - Some(IpAddr::V6(v6)) - } else { - None - } + if !v4.is_unspecified() { + Some(IpAddr::V4(v4)) + } else { + None } - }, - Err(_) => { - return Err(NetavarkError::msg(format!( - "invalid host ip \"{}\" provided for port {}", - i.host_ip, i.host_port, - ))); } + IpAddr::V6(v6) => { + if !is_ipv6 { + continue; + } + if !v6.is_unspecified() { + Some(IpAddr::V6(v6)) + } else { + None + } + } + }, + Err(_) => { + return Err(NetavarkError::msg(format!( + "invalid host ip \"{}\" provided for port {}", + i.host_ip, i.host_port, + ))); } - }; - - // hostport dnat - let is_range = i.range > 1; - let mut host_port = i.host_port.to_string(); - if is_range { - host_port = format!("{}:{}", i.host_port, (i.host_port + (i.range - 1))) } - netavark_hostport_dn_chain.build_rule(VarkRule::new( - format!( - // I'm leaving this commented code for now in the case - // we need to revert. - // "-j {} -p {} -m multiport --destination-ports {} {}", - "-j {} -p {} --dport {} {}", - network_dn_chain_name, i.protocol, &host_port, comment_dn_network_cid - ), - None, - )); - - let mut dn_setmark_rule_localhost = format!( - "-j {} -s {} -p {} --dport {}", - NETAVARK_HOSTPORT_SETMARK, network_address, i.protocol, &host_port - ); - - let mut dn_setmark_rule_subnet = format!( - "-j {} -s {} -p {} --dport {}", - NETAVARK_HOSTPORT_SETMARK, localhost_ip, i.protocol, &host_port - ); + }; - // if a destination ip address is provided, we need to alter - // the rule a bit - if let Some(host_ip) = host_ip { - dn_setmark_rule_localhost = format!("{dn_setmark_rule_localhost} -d {host_ip}"); - dn_setmark_rule_subnet = format!("{dn_setmark_rule_subnet} -d {host_ip}"); - } + // hostport dnat + let is_range = i.range > 1; + let mut host_port = i.host_port.to_string(); + if is_range { + host_port = format!("{}:{}", i.host_port, (i.host_port + (i.range - 1))) + } + netavark_hostport_dn_chain.build_rule(VarkRule::new( + format!( + // I'm leaving this commented code for now in the case + // we need to revert. + // "-j {} -p {} -m multiport --destination-ports {} {}", + "-j {} -p {} --dport {} {}", + network_dn_chain_name, i.protocol, &host_port, comment_dn_network_cid + ), + None, + )); + + let mut dn_setmark_rule_localhost = format!( + "-j {} -s {} -p {} --dport {}", + NETAVARK_HOSTPORT_SETMARK, network_address, i.protocol, &host_port + ); + + let mut dn_setmark_rule_subnet = format!( + "-j {} -s {} -p {} --dport {}", + NETAVARK_HOSTPORT_SETMARK, localhost_ip, i.protocol, &host_port + ); + + // if a destination ip address is provided, we need to alter + // the rule a bit + if let Some(host_ip) = host_ip { + dn_setmark_rule_localhost = format!("{dn_setmark_rule_localhost} -d {host_ip}"); + dn_setmark_rule_subnet = format!("{dn_setmark_rule_subnet} -d {host_ip}"); + } - // dn container (the actual port usages) - netavark_hashed_dn_chain.build_rule(VarkRule::new(dn_setmark_rule_localhost, None)); + // dn container (the actual port usages) + netavark_hashed_dn_chain.build_rule(VarkRule::new(dn_setmark_rule_localhost, None)); - netavark_hashed_dn_chain.build_rule(VarkRule::new(dn_setmark_rule_subnet, None)); + netavark_hashed_dn_chain.build_rule(VarkRule::new(dn_setmark_rule_subnet, None)); - let mut container_ip_value = container_ip.to_string(); - if is_ipv6 { - container_ip_value = format!("[{container_ip_value}]") - } - let mut container_port = i.container_port.to_string(); - if is_range { - container_port = format!( - "{}-{}/{}", - i.container_port, - (i.container_port + (i.range - 1)), - i.host_port - ); - } - let mut dnat_rule = format!( - "-j {} -p {} --to-destination {}:{} --destination-port {}", - DNAT, i.protocol, container_ip_value, container_port, &host_port + let mut container_ip_value = container_ip.to_string(); + if is_ipv6 { + container_ip_value = format!("[{container_ip_value}]") + } + let mut container_port = i.container_port.to_string(); + if is_range { + container_port = format!( + "{}-{}/{}", + i.container_port, + (i.container_port + (i.range - 1)), + i.host_port ); - - // if a destination ip address is provided, we need to alter - // the rule a bit - if let Some(host_ip) = host_ip { - dnat_rule = format!("{dnat_rule} -d {host_ip}") - } - netavark_hashed_dn_chain.build_rule(VarkRule::new(dnat_rule, None)); } + let mut dnat_rule = format!( + "-j {} -p {} --to-destination {}:{} --destination-port {}", + DNAT, i.protocol, container_ip_value, container_port, &host_port + ); + + // if a destination ip address is provided, we need to alter + // the rule a bit + if let Some(host_ip) = host_ip { + dnat_rule = format!("{dnat_rule} -d {host_ip}") + } + netavark_hashed_dn_chain.build_rule(VarkRule::new(dnat_rule, None)); } - None => {} }; // The order is important here. Be certain before changing it diff --git a/src/network/bridge.rs b/src/network/bridge.rs index ce89427a..8c766fd4 100644 --- a/src/network/bridge.rs +++ b/src/network/bridge.rs @@ -188,11 +188,8 @@ impl driver::NetworkDriver for Bridge<'_> { } } let mut names = vec![self.info.container_name.to_string()]; - match &self.info.per_network_opts.aliases { - Some(n) => { - names.extend(n.clone()); - } - None => {} + if let Some(n) = &self.info.per_network_opts.aliases { + names.extend(n.clone()); } let gw = data diff --git a/test-dhcp/helpers.bash b/test-dhcp/helpers.bash index b43e088f..e1b59ff1 100644 --- a/test-dhcp/helpers.bash +++ b/test-dhcp/helpers.bash @@ -281,25 +281,24 @@ function basic_setup() { } # -# add_bridge br0 +# add_bridge # function add_bridge() { local bridge_name="$1" br_cidr=$(gateway_from_subnet "$SUBNET_CIDR") - run_in_container_netns brctl addbr $bridge_name - run_in_container_netns ifconfig $bridge_name $br_cidr up - run_in_container_netns firewall-cmd --add-interface=$bridge_name --zone=trusted + run_in_container_netns ip link add $bridge_name type bridge + run_in_container_netns ip addr add $br_cidr dev $bridge_name + run_in_container_netns ip link set $bridge_name up } # -# remove_bridge br0 +# remove_bridge # function remove_bridge() { local bridge_name="$1" - run_in_container_netns firewall-cmd --remove-interface="$bridge_name" --zone=trusted run_in_container_netns ip link set "$bridge_name" down # shellcheck disable=SC2086 - run_in_container_netns brctl delbr $bridge_name + run_in_container_netns ip link del $bridge_name } # @@ -310,10 +309,7 @@ function remove_veth() { local bridge_name="$2" local veth_br_name="${veth_name}br" - run_in_container_netns ip link set "$veth_br_name" down - run_in_container_netns ip link set "$veth_name" down - run_in_container_netns brctl delif "$bridge_name" "$veth_br_name" - run_in_container_netns ip link del "$veth_br_name" type veth peer name "$veth_name" + run_in_container_netns ip link del "$veth_br_name" } # @@ -324,7 +320,7 @@ function add_veth() { local bridge_name="$2" local veth_br_name="${veth_name}br" run_in_container_netns ip link add "$veth_br_name" type veth peer name "$veth_name" - run_in_container_netns brctl addif "$bridge_name" "$veth_br_name" + run_in_container_netns ip link set "$veth_br_name" master "$bridge_name" run_in_container_netns ip link set "$veth_br_name" up run_in_container_netns ip link set "$veth_name" up }