diff --git a/src/commands/setup.rs b/src/commands/setup.rs index 2768a3a3e..33125e6d2 100644 --- a/src/commands/setup.rs +++ b/src/commands/setup.rs @@ -66,11 +66,17 @@ impl Setup { let config_dir = get_config_dir(config_dir, "setup")?; let mut drivers = Vec::with_capacity(network_options.network_info.len()); - // Perform per-network setup - for (net_name, network) in network_options.network_info.iter() { - let per_network_opts = network_options.networks.get(net_name).ok_or_else(|| { - NetavarkError::Message(format!("network options for network {net_name} not found")) - })?; + for named_network_opts in &network_options.networks { + let per_network_opts = &named_network_opts.opts; + let network = network_options + .network_info + .get(&named_network_opts.name) + .ok_or_else(|| { + NetavarkError::Message(format!( + "network info for network {} not found", + &named_network_opts.name + )) + })?; let mut driver = get_network_driver( DriverInfo { diff --git a/src/commands/teardown.rs b/src/commands/teardown.rs index 04e78c202..65fc9da7d 100644 --- a/src/commands/teardown.rs +++ b/src/commands/teardown.rs @@ -95,12 +95,14 @@ impl Teardown { let (mut hostns, mut netns) = core_utils::open_netlink_sockets(&self.network_namespace_path)?; - for (net_name, network) in network_options.network_info.iter() { - let per_network_opts = match network_options.networks.get(net_name) { + for named_network_opts in &network_options.networks { + let per_network_opts = &named_network_opts.opts; + let network = match network_options.network_info.get(&named_network_opts.name) { Some(opts) => opts, None => { error_list.push(NetavarkError::Message(format!( - "network options for network {net_name} not found" + "network info for network {} not found", + &named_network_opts.name ))); continue; } diff --git a/src/network/types.rs b/src/network/types.rs index fa8bfe88f..051df5ea7 100644 --- a/src/network/types.rs +++ b/src/network/types.rs @@ -1,11 +1,14 @@ // Crate contains the types which are accepted by netavark. use ipnet::IpNet; +use serde::de; use std::collections::HashMap; +use std::fmt::Formatter; +use std::marker::PhantomData; use std::net::IpAddr; // Network describes the Network attributes. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct Network { /// Set up dns for this network #[serde(rename = "dns_enabled")] @@ -60,7 +63,7 @@ pub struct Network { } /// NetworkOptions for a given container. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct NetworkOptions { /// The container id, used for ipam allocation. #[serde(rename = "container_id")] @@ -78,7 +81,8 @@ pub struct NetworkOptions { /// The networks listed in "network_info" have to match this, /// both use the network name as key for the map. #[serde(rename = "networks")] - pub networks: HashMap, + #[serde(deserialize_with = "deserialize_network_options_map_or_vec")] + pub networks: Vec, /// The networks which are needed to run this. /// It has to match the networks listed in "networks", @@ -95,8 +99,18 @@ pub struct NetworkOptions { pub dns_servers: Option>, } +/// NamedPerNetworkOptions are the container specific network setup options. +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +pub struct NamedPerNetworkOptions { + /// The name of the network. + pub name: String, + + #[serde(flatten)] + pub opts: PerNetworkOptions, +} + /// PerNetworkOptions are options which should be set on a per network basis -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct PerNetworkOptions { /// Aliases contains a list of names which the dns server should resolve /// to this container. Should only be set when DNSEnabled is true on the Network. @@ -123,7 +137,7 @@ pub struct PerNetworkOptions { } /// PortMapping is one or more ports that will be mapped into the container. -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct PortMapping { /// ContainerPort is the port number that will be exposed from the /// container. @@ -206,7 +220,7 @@ pub struct NetAddress { } /// Subnet for a network. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct Subnet { /// Gateway IP for this Network. #[serde(rename = "gateway")] @@ -222,7 +236,7 @@ pub struct Subnet { } /// Static routes for a network. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct Route { /// Gateway IP for this route. #[serde(rename = "gateway")] @@ -238,7 +252,7 @@ pub struct Route { } /// LeaseRange contains the range where IP are leased. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct LeaseRange { /// EndIP last IP in the subnet which should be used to assign ips. #[serde(rename = "end_ip")] @@ -268,3 +282,43 @@ pub struct NetworkPluginExec { /// The special network options for this specific container pub network_options: PerNetworkOptions, } + +pub fn deserialize_network_options_map_or_vec<'a, 'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: de::Deserializer<'de>, +{ + struct MapOrVec(PhantomData>); + impl<'de> de::Visitor<'de> for MapOrVec { + type Value = Vec; + + fn expecting(&self, formatter: &mut Formatter) -> std::fmt::Result { + formatter.write_str("array of network options or map") + } + + fn visit_map(self, mut map: M) -> Result + where + M: de::MapAccess<'de>, + { + let mut arr = Vec::with_capacity(map.size_hint().unwrap_or_default()); + + while let Some((key, value)) = map.next_entry()? { + arr.push(NamedPerNetworkOptions { + name: key, + opts: value, + }); + } + Ok(arr) + } + + fn visit_seq(self, visitor: S) -> Result + where + S: de::SeqAccess<'de>, + { + serde::Deserialize::deserialize(de::value::SeqAccessDeserializer::new(visitor)) + } + } + + deserializer.deserialize_any(MapOrVec(PhantomData)) +} diff --git a/src/test/config/setupopts2.test.json b/src/test/config/setupopts2.test.json index 1bd606c68..14a07dfb8 100644 --- a/src/test/config/setupopts2.test.json +++ b/src/test/config/setupopts2.test.json @@ -1,14 +1,15 @@ { "container_id": "someID", "container_name": "someName", - "networks": { - "podman": { + "networks": [ + { + "name": "podman", "static_ips": [ "10.88.0.2" ], "interface_name": "ethsomething0" } - }, + ], "network_info": { "podman": { "name": "podman", diff --git a/src/test/config/twoNetworks-array.json b/src/test/config/twoNetworks-array.json new file mode 100644 index 000000000..49775eb76 --- /dev/null +++ b/src/test/config/twoNetworks-array.json @@ -0,0 +1,60 @@ +{ + "container_id": "2d0fe608dc86d7ba65dcec724a335b618328f08daa62afd2ee7fa9d41f74e5a9", + "container_name": "", + "networks": [ + { + "name": "podman1", + "static_ips": [ + "10.0.0.2" + ], + "interface_name": "eth0" + }, + { + "name": "podman2", + "static_ips": [ + "10.1.0.2" + ], + "interface_name": "eth1" + } + ], + "network_info": { + "podman1": { + "name": "podman1", + "id": "4937f73ac0df011d4f2848d5f83f5c20b707e71a8d98789bbe80d8f64a815e79", + "driver": "bridge", + "network_interface": "podman1", + "created": "2021-11-04T19:08:39.124321192+01:00", + "subnets": [ + { + "subnet": "10.0.0.0/24", + "gateway": "10.0.0.1" + } + ], + "ipv6_enabled": false, + "internal": false, + "dns_enabled": false, + "ipam_options": { + "driver": "host-local" + } + }, + "podman2": { + "name": "podman2", + "id": "488a7d9be4fa72a5b80811bd847aac1d99d1a09060739b4e08687949c957cda8", + "driver": "bridge", + "network_interface": "podman2", + "created": "2021-11-04T19:08:39.124800596+01:00", + "subnets": [ + { + "subnet": "10.1.0.0/24", + "gateway": "10.1.0.1" + } + ], + "ipv6_enabled": false, + "internal": false, + "dns_enabled": false, + "ipam_options": { + "driver": "host-local" + } + } + } +} diff --git a/src/test/config/twoNetworks.json b/src/test/config/twoNetworks-map.json similarity index 100% rename from src/test/config/twoNetworks.json rename to src/test/config/twoNetworks-map.json diff --git a/src/test/test.rs b/src/test/test.rs index e37c3ff3c..85df4b2dd 100644 --- a/src/test/test.rs +++ b/src/test/test.rs @@ -7,13 +7,43 @@ mod tests { use netavark::network; #[test] // Test setup options loader - fn test_setup_opts_load() { - match network::types::NetworkOptions::load(Some(OsString::from( + fn test_setup_opts_load_map() { + let a = network::types::NetworkOptions::load(Some(OsString::from( "src/test/config/setupopts.test.json", - ))) { - Ok(_) => {} - Err(e) => panic!("{}", e), - } + ))) + .unwrap(); + assert_eq!(a.networks.len(), 1); + } + + #[test] + fn test_setup_opts_load_vec() { + let a = network::types::NetworkOptions::load(Some(OsString::from( + "src/test/config/setupopts2.test.json", + ))) + .unwrap(); + assert_eq!(a.networks.len(), 1); + } + + #[test] + // Test setup options loader + fn test_load_two_networks() { + let array = network::types::NetworkOptions::load(Some(OsString::from( + "src/test/config/twoNetworks-array.json", + ))) + .unwrap(); + assert_eq!(array.networks.len(), 2); + assert_eq!(array.networks[0].name, "podman1"); + assert_eq!(array.networks[0].opts.interface_name, "eth0"); + assert_eq!(array.networks[1].name, "podman2"); + assert_eq!(array.networks[1].opts.interface_name, "eth1"); + + let map = network::types::NetworkOptions::load(Some(OsString::from( + "src/test/config/twoNetworks-map.json", + ))) + .unwrap(); + + // both the parsed array or map version should result in the same value + assert_eq!(array, map); } // Test if we can deserialize values correctly diff --git a/test/250-bridge-nftables.bats b/test/250-bridge-nftables.bats index 566cdfda7..a0f0eb286 100644 --- a/test/250-bridge-nftables.bats +++ b/test/250-bridge-nftables.bats @@ -1014,6 +1014,17 @@ net/ipv4/conf/podman1/rp_filter = 2" assert_json "$result" 'has("t1")' == "true" "t1 object key exists" assert_json "$result" 'has("t2")' == "true" "t2 object key exists" + # verify the setup order of the contianer interfaces by checking the interface index + run_in_container_netns ip -j addr show eth0 + result="$output" + assert_json "$result" '.[0].ifindex' == "2" "eth0 interface must have index 2" + assert_json "$result" '.[0].addr_info.[0].local' == "10.89.1.2" "first ip adddress on eth0" + + run_in_container_netns ip -j addr show eth1 + result="$output" + assert_json "$result" '.[0].ifindex' == "3" "eth1 interface must have index 3" + assert_json "$result" '.[0].addr_info.[0].local' == "10.89.2.2" "first ip adddress on eth1" + run_in_container_netns ip link del eth0 run_in_container_netns ip link del eth1 @@ -1034,6 +1045,23 @@ net/ipv4/conf/podman1/rp_filter = 2" assert "$output" !~ "jump nv_fae505bb_10_89_1_0_nm24_dnat" "network 2 fw rule should not exist" } +@test "$fw_driver - two networks reversed" { + # reverse the two networks to ensure the order is indeed depended on the array + run_netavark --file <(jq ".networks |= reverse" ${TESTSDIR}/testfiles/two-networks.json) setup $(get_container_netns_path) + + # verify the setup order of the contianer interfaces by checking the interface index + # it should be reversed now compared to the test above + run_in_container_netns ip -j addr show eth1 + result="$output" + assert_json "$result" '.[0].ifindex' == "2" "eth1 interface must have index 2" + assert_json "$result" '.[0].addr_info.[0].local' == "10.89.2.2" "first ip adddress on eth1" + + run_in_container_netns ip -j addr show eth0 + result="$output" + assert_json "$result" '.[0].ifindex' == "3" "eth0 interface must have index 3" + assert_json "$result" '.[0].addr_info.[0].local' == "10.89.1.2" "first ip adddress on eth0" +} + @test "$fw_driver - ipv6 disabled error message" { # disable ipv6 in the netns run_in_host_netns sysctl net.ipv6.conf.all.disable_ipv6=1 @@ -1227,7 +1255,7 @@ function check_simple_bridge_nftables() { assert "${lines[3]}" =~ "ip6 daddr != ff00::/8 snat ip6 to fd20:100:200::1" run_netavark --file ${TESTSDIR}/testfiles/bridge-outbound-addr6.json teardown $(get_container_netns_path) - + # Check that the chain is removed expected_rc=1 run_in_host_netns nft list chain inet netavark nv_2f259bab_fd10-88-a--_nm64 } diff --git a/test/testfiles/two-networks.json b/test/testfiles/two-networks.json index da7ed3910..fbe518cc8 100644 --- a/test/testfiles/two-networks.json +++ b/test/testfiles/two-networks.json @@ -3,27 +3,29 @@ "container_name": "heuristic_archimedes", "port_mappings": [ { - "host_ip": "", - "container_port": 8080, - "host_port": 8080, - "range": 1, - "protocol": "tcp" + "host_ip": "", + "container_port": 8080, + "host_port": 8080, + "range": 1, + "protocol": "tcp" } - ], - "networks": { - "t1": { + ], + "networks": [ + { + "name": "t1", "static_ips": [ "10.89.1.2" ], "interface_name": "eth0" }, - "t2": { + { + "name": "t2", "static_ips": [ "10.89.2.2" ], "interface_name": "eth1" } - }, + ], "network_info": { "t1": { "name": "t1",