From 33aad3be1e8e1559ea5702d2a0ad88b9e93702ec Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Fri, 11 Oct 2024 17:07:18 +0200 Subject: [PATCH 01/10] Add support for multiple connection files per logical interface --- .dockerignore | 1 + Cargo.lock | 7 ++ Cargo.toml | 1 + src/apply_conf.rs | 59 ++++++++++---- src/generate_conf.rs | 80 +++++++++++++++---- src/types.rs | 3 + .../generate/expected/br1-br.nmconnection | 21 +++++ .../generate/expected/eth1-port.nmconnection | 11 +++ testdata/generate/expected/eth1.nmconnection | 12 +++ testdata/generate/expected/host_config.yaml | 19 +++++ .../generate/expected/ovs0-if.nmconnection | 25 ++++++ .../generate/expected/ovs0-port.nmconnection | 11 +++ testdata/generate/node1.yaml | 20 +++++ 13 files changed, 236 insertions(+), 34 deletions(-) create mode 100644 .dockerignore create mode 100644 testdata/generate/expected/br1-br.nmconnection create mode 100644 testdata/generate/expected/eth1-port.nmconnection create mode 100644 testdata/generate/expected/eth1.nmconnection create mode 100644 testdata/generate/expected/ovs0-if.nmconnection create mode 100644 testdata/generate/expected/ovs0-port.nmconnection diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..9f97022 --- /dev/null +++ b/.dockerignore @@ -0,0 +1 @@ +target/ \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 4d451ca..eb01230 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -212,6 +212,12 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "configparser" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e57e3272f0190c3f1584272d613719ba5fc7df7f4942fe542e63d949cf3a649b" + [[package]] name = "crossbeam-utils" version = "0.8.20" @@ -754,6 +760,7 @@ version = "0.3.1" dependencies = [ "anyhow", "clap", + "configparser", "env_logger", "log", "network-interface", diff --git a/Cargo.toml b/Cargo.toml index 6b88611..41271f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,3 +16,4 @@ network-interface = "2.0.0" nmstate = { version = "2.2.36", features = ["gen_conf"] } serde = { version = "1.0.210", features = ["derive"] } serde_yaml = "0.9.34" +configparser = "3.1.0" diff --git a/src/apply_conf.rs b/src/apply_conf.rs index 3aab891..44a604e 100644 --- a/src/apply_conf.rs +++ b/src/apply_conf.rs @@ -183,29 +183,33 @@ fn copy_connection_files( for interface in &host.interfaces { info!("Processing interface '{}'...", &interface.logical_name); + let connections = &interface.connection_ids.clone().unwrap_or(vec![interface.logical_name.clone()]); - let mut filename = &interface.logical_name; + for connection in connections { + info!("Processing connection '{}'...", connection); + let mut filename = connection.clone(); - let filepath = keyfile_path(host_config_dir, filename) - .ok_or_else(|| anyhow!("Determining source keyfile path"))?; + let filepath = keyfile_path(host_config_dir, &filename) + .ok_or_else(|| anyhow!("Determining source keyfile path"))?; - let mut contents = fs::read_to_string(filepath).context("Reading file")?; + let mut contents = fs::read_to_string(filepath).context("Reading file")?; - // Update the name and all references of the host NIC in the settings file if there is a difference from the static config. - match local_interfaces.get(&interface.logical_name) { - None => {} - Some(local_name) => { - info!( - "Using interface name '{}' instead of the preconfigured '{}'", - local_name, interface.logical_name - ); - - contents = contents.replace(&interface.logical_name, local_name); - filename = local_name; + // Update the name and all references of the host NIC in the settings file if there is a difference from the static config. + match local_interfaces.get(&interface.logical_name) { + None => {} + Some(local_name) => { + info!( + "Using interface name '{}' instead of the preconfigured '{}'", + local_name, interface.logical_name + ); + + contents = contents.replace(&interface.logical_name, local_name); + filename = filename.replace(&interface.logical_name, local_name); + } } - } - store_connection_file(filename, contents, destination_dir).context("Storing file")?; + store_connection_file(&filename, contents, destination_dir).context("Storing file")?; + } } Ok(()) @@ -302,6 +306,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }], }, Host { @@ -310,6 +315,7 @@ mod tests { logical_name: "".to_string(), mac_address: Option::from("10:10:10:10:10:10".to_string()), interface_type: "".to_string(), + connection_ids: None, }], }, ]; @@ -336,6 +342,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }] ); } @@ -349,6 +356,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("10:20:30:40:50:60".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }], }, Host { @@ -357,6 +365,7 @@ mod tests { logical_name: "".to_string(), mac_address: Option::from("00:10:20:30:40:50".to_string()), interface_type: "".to_string(), + connection_ids: None, }], }, ]; @@ -389,21 +398,25 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth1".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("36:5e:6b:a2:ed:80".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:aa:44:58".to_string()), interface_type: "bond".to_string(), + connection_ids: None, }, ], }, @@ -414,11 +427,13 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("36:5e:6b:a2:ed:81".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), + connection_ids: None, }, ], }, @@ -435,26 +450,31 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), + connection_ids: None, }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth2.bridge".to_string(), mac_address: None, interface_type: "linux-bridge".to_string(), + connection_ids: None, }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), + connection_ids: None, }, ], }; @@ -522,26 +542,31 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), + connection_ids: None, }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth1".to_string(), mac_address: Option::from("00:11:22:33:44:57".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), + connection_ids: None, }, ], }; diff --git a/src/generate_conf.rs b/src/generate_conf.rs index 3deb335..515c731 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::ffi::OsStr; use std::fs; use std::path::Path; @@ -5,7 +6,7 @@ use std::path::Path; use anyhow::{anyhow, Context}; use log::{info, warn}; use nmstate::{InterfaceType, NetworkState}; - +use configparser::ini::Ini; use crate::types::{Host, Interface}; use crate::{ALL_HOSTS_DIR, ALL_HOSTS_FILE, HOST_MAPPING_FILE}; @@ -76,19 +77,35 @@ fn generate_config( ) -> Result<(Vec, NetworkConfig), anyhow::Error> { let network_state = NetworkState::new_from_yaml(&data)?; - let interfaces = extract_interfaces(&network_state); - validate_interfaces(&interfaces, require_mac_addresses)?; - let config = network_state .gen_conf()? .get("NetworkManager") .ok_or_else(|| anyhow!("Invalid NM configuration"))? .to_owned(); + let interfaces = extract_interfaces(&network_state, &config); + validate_interfaces(&interfaces, require_mac_addresses)?; + Ok((interfaces, config)) } -fn extract_interfaces(network_state: &NetworkState) -> Vec { +fn extract_interfaces(network_state: &NetworkState, config_files: &NetworkConfig) -> Vec { + + let mut config_files_map :HashMap> = HashMap::new(); + + for (_filename, content) in config_files { + let mut config = Ini::new(); + config.read(content.to_string()).expect("Unable to read nmconnection file"); + let id = config.get("connection", "id"); + let interface_name = config.get("connection", "interface-name"); + if id.is_some() && interface_name.is_some() + { + config_files_map.entry(interface_name.unwrap().to_string()) + .or_insert_with(|| Vec::new()) + .push(id.unwrap().clone()); + } + } + network_state .interfaces .iter() @@ -97,6 +114,7 @@ fn extract_interfaces(network_state: &NetworkState) -> Vec { logical_name: i.name().to_owned(), mac_address: i.base_iface().mac_address.clone(), interface_type: i.iface_type().to_string(), + connection_ids: config_files_map.get(i.name()).map(Vec::clone).or_else(|| Some(Vec::new())) }) .collect() } @@ -174,10 +192,7 @@ fn store_network_mapping( mod tests { use std::fs; use std::path::Path; - - use crate::generate_conf::{ - extract_hostname, extract_interfaces, generate, generate_config, validate_interfaces, - }; + use crate::generate_conf::{extract_hostname, extract_interfaces, generate, generate_config, validate_interfaces}; use crate::types::{Host, Interface}; use crate::HOST_MAPPING_FILE; @@ -190,16 +205,10 @@ mod tests { assert!(generate(config_dir, out_dir).is_ok()); - // verify contents of *.nmconnection files - let exp_eth0_conn = fs::read_to_string(exp_output_path.join("eth0.nmconnection"))?; - let exp_bridge_conn = fs::read_to_string(exp_output_path.join("bridge0.nmconnection"))?; + // verify contents of lo.nmconnection files let exp_lo_conn = fs::read_to_string(exp_output_path.join("lo.nmconnection"))?; - let eth0_conn = fs::read_to_string(output_path.join("eth0.nmconnection"))?; - let bridge_conn = fs::read_to_string(output_path.join("bridge0.nmconnection"))?; let lo_conn = fs::read_to_string(output_path.join("lo.nmconnection"))?; - assert_eq!(exp_eth0_conn, eth0_conn); - assert_eq!(exp_bridge_conn, bridge_conn); assert_eq!(exp_lo_conn, lo_conn); // verify contents of the host mapping file @@ -224,6 +233,17 @@ mod tests { assert_eq!(exp_hosts, hosts); + // verify contents of *.nmconnection files based on interface.connection_ids + hosts.iter_mut() + .flat_map(|h| h.interfaces.iter()) + .flat_map(|interface| &interface.connection_ids) + .flat_map(|conns| conns.iter()) + .for_each(|conn_id| { + let exp_conn = fs::read_to_string(exp_output_path.join(format!("{conn_id}.nmconnection"))).unwrap(); + let conn = fs::read_to_string(output_path.join(format!("{conn_id}.nmconnection"))).unwrap(); + assert_eq!(exp_conn, conn); + }); + // cleanup fs::remove_dir_all(out_dir)?; @@ -269,7 +289,10 @@ mod tests { "#, )?; - let mut interfaces = extract_interfaces(&net_state); + let config_files = vec![generate_config_file("eth1".to_string(), "eth1".to_string()), + generate_config_file("bridge0".to_string(), "bridge0".to_string())]; + + let mut interfaces = extract_interfaces(&net_state, &config_files); interfaces.sort_by(|a, b| a.logical_name.cmp(&b.logical_name)); assert_eq!( @@ -279,11 +302,13 @@ mod tests { logical_name: "bridge0".to_string(), mac_address: Option::from("FE:C4:05:42:8B:AB".to_string()), interface_type: "linux-bridge".to_string(), + connection_ids: Some(vec!["bridge0".to_string()]), }, Interface { logical_name: "eth1".to_string(), mac_address: Option::from("FE:C4:05:42:8B:AA".to_string()), interface_type: "ethernet".to_string(), + connection_ids: Some(vec!["eth1".to_string()]), }, ] ); @@ -291,6 +316,16 @@ mod tests { Ok(()) } + fn generate_config_file(logical_name: String, connection_id: String) -> (String, String){ + let filename = format!("{connection_id}.nmconnection"); + + let mut config = configparser::ini::Ini::new(); + config.set("connection", "id", Some(connection_id)); + config.set("connection", "interface-name", Some(logical_name)); + + (filename, config.writes()) + } + #[test] fn validate_interfaces_missing_ethernet_interfaces() { let interfaces = vec![ @@ -298,11 +333,13 @@ mod tests { logical_name: "eth3.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), + connection_ids: None, }, Interface { logical_name: "bond0".to_string(), mac_address: None, interface_type: "bond".to_string(), + connection_ids: None, }, ]; @@ -317,31 +354,37 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth1".to_string(), mac_address: None, interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth3".to_string(), mac_address: None, interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth3.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), + connection_ids: None, }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), + connection_ids: None, }, ]; @@ -362,16 +405,19 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), + connection_ids: None, }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), + connection_ids: None, }, Interface { logical_name: "bond0".to_string(), mac_address: None, interface_type: "bond".to_string(), + connection_ids: None, }, ]; diff --git a/src/types.rs b/src/types.rs index 7dee144..817e8b2 100644 --- a/src/types.rs +++ b/src/types.rs @@ -13,6 +13,9 @@ pub struct Interface { pub(crate) logical_name: String, #[serde(skip_serializing_if = "Option::is_none")] #[serde(default)] + pub(crate) connection_ids: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] pub(crate) mac_address: Option, pub(crate) interface_type: String, } diff --git a/testdata/generate/expected/br1-br.nmconnection b/testdata/generate/expected/br1-br.nmconnection new file mode 100644 index 0000000..d6e75e3 --- /dev/null +++ b/testdata/generate/expected/br1-br.nmconnection @@ -0,0 +1,21 @@ +[connection] +autoconnect=true +autoconnect-slaves=1 +id=br1-br +interface-name=br1 +type=ovs-bridge +uuid=327127e8-ee26-5178-bacd-ebbb191b1bdd + +[ipv4] +dhcp-timeout=2147483647 +method=disabled + +[ipv6] +dhcp-timeout=2147483647 +method=disabled + +[ovs-bridge] +stp-enable=true + +[user] +nmstate.interface.description=ovs bridge with eth1 as a port and ovs0 as an internal interface diff --git a/testdata/generate/expected/eth1-port.nmconnection b/testdata/generate/expected/eth1-port.nmconnection new file mode 100644 index 0000000..2c81d1c --- /dev/null +++ b/testdata/generate/expected/eth1-port.nmconnection @@ -0,0 +1,11 @@ +[connection] +autoconnect=true +autoconnect-slaves=-1 +id=eth1-port +interface-name=eth1 +master=br1 +slave-type=ovs-bridge +type=ovs-port +uuid=5e4ac41f-d8dd-5cbd-98b3-122629af91e0 + +[ovs-port] diff --git a/testdata/generate/expected/eth1.nmconnection b/testdata/generate/expected/eth1.nmconnection new file mode 100644 index 0000000..2720221 --- /dev/null +++ b/testdata/generate/expected/eth1.nmconnection @@ -0,0 +1,12 @@ +[connection] +autoconnect=true +autoconnect-slaves=-1 +id=eth1 +interface-name=eth1 +master=eth1 +slave-type=ovs-port +type=802-3-ethernet +uuid=0523c0a1-5f5e-5603-bcf2-68155d5d322e + +[ethernet] +cloned-mac-address=5C:C7:C9:5E:FB:EC diff --git a/testdata/generate/expected/host_config.yaml b/testdata/generate/expected/host_config.yaml index e23faae..06aece8 100644 --- a/testdata/generate/expected/host_config.yaml +++ b/testdata/generate/expected/host_config.yaml @@ -1,8 +1,27 @@ - hostname: node1 interfaces: + - logical_name: br1 + connection_ids: + - br1-br + interface_type: ovs-bridge - logical_name: bridge0 + connection_ids: + - bridge0 mac_address: FE:C4:05:42:8B:AA interface_type: linux-bridge + - logical_name: ovs0 + connection_ids: + - ovs0-port + - ovs0-if + interface_type: ovs-interface - logical_name: eth0 + connection_ids: + - eth0 mac_address: 0E:4D:C6:B8:C4:72 interface_type: ethernet + - logical_name: eth1 + connection_ids: + - eth1-port + - eth1 + mac_address: 5c:c7:c9:5e:fb:ec + interface_type: ethernet diff --git a/testdata/generate/expected/ovs0-if.nmconnection b/testdata/generate/expected/ovs0-if.nmconnection new file mode 100644 index 0000000..d17e12a --- /dev/null +++ b/testdata/generate/expected/ovs0-if.nmconnection @@ -0,0 +1,25 @@ +[connection] +autoconnect=true +autoconnect-slaves=-1 +id=ovs0-if +interface-name=ovs0 +master=ovs0 +slave-type=ovs-port +type=ovs-interface +uuid=94e89542-80b4-59a0-b84a-7d82c89c9ed4 + +[ipv4] +dhcp-client-id=mac +dhcp-send-hostname=true +dhcp-timeout=2147483647 +ignore-auto-dns=false +ignore-auto-routes=false +method=auto +never-default=false + +[ipv6] +dhcp-timeout=2147483647 +method=disabled + +[ovs-interface] +type=internal diff --git a/testdata/generate/expected/ovs0-port.nmconnection b/testdata/generate/expected/ovs0-port.nmconnection new file mode 100644 index 0000000..6516819 --- /dev/null +++ b/testdata/generate/expected/ovs0-port.nmconnection @@ -0,0 +1,11 @@ +[connection] +autoconnect=true +autoconnect-slaves=-1 +id=ovs0-port +interface-name=ovs0 +master=br1 +slave-type=ovs-bridge +type=ovs-port +uuid=dde94eac-b114-55b9-8f5f-7d53334bcb78 + +[ovs-port] diff --git a/testdata/generate/node1.yaml b/testdata/generate/node1.yaml index 1dc8c97..af71c54 100644 --- a/testdata/generate/node1.yaml +++ b/testdata/generate/node1.yaml @@ -61,3 +61,23 @@ interfaces: address: - ip: ::1 prefix-length: 128 + - name: eth1 + type: ethernet + state: up + mac-address: 5c:c7:c9:5e:fb:ec + - name: ovs0 + type: ovs-interface + state: up + ipv4: + dhcp: true + enabled: true + - name: br1 + description: ovs bridge with eth1 as a port and ovs0 as an internal interface + type: ovs-bridge + state: up + bridge: + options: + stp: true + port: + - name: eth1 + - name: ovs0 \ No newline at end of file From 9a8b97ddac0584fcc1a21c2c0ff683ff7911316b Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Fri, 11 Oct 2024 17:38:09 +0200 Subject: [PATCH 02/10] Fix formatting --- src/apply_conf.rs | 7 ++++-- src/generate_conf.rs | 55 ++++++++++++++++++++++++++++---------------- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/apply_conf.rs b/src/apply_conf.rs index 44a604e..d1d342e 100644 --- a/src/apply_conf.rs +++ b/src/apply_conf.rs @@ -183,7 +183,10 @@ fn copy_connection_files( for interface in &host.interfaces { info!("Processing interface '{}'...", &interface.logical_name); - let connections = &interface.connection_ids.clone().unwrap_or(vec![interface.logical_name.clone()]); + let connections = &interface + .connection_ids + .clone() + .unwrap_or(vec![interface.logical_name.clone()]); for connection in connections { info!("Processing connection '{}'...", connection); @@ -209,7 +212,7 @@ fn copy_connection_files( } store_connection_file(&filename, contents, destination_dir).context("Storing file")?; - } + } } Ok(()) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index 515c731..c838246 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -3,12 +3,12 @@ use std::ffi::OsStr; use std::fs; use std::path::Path; +use crate::types::{Host, Interface}; +use crate::{ALL_HOSTS_DIR, ALL_HOSTS_FILE, HOST_MAPPING_FILE}; use anyhow::{anyhow, Context}; +use configparser::ini::Ini; use log::{info, warn}; use nmstate::{InterfaceType, NetworkState}; -use configparser::ini::Ini; -use crate::types::{Host, Interface}; -use crate::{ALL_HOSTS_DIR, ALL_HOSTS_FILE, HOST_MAPPING_FILE}; /// `NetworkConfig` contains the generated configurations in the /// following format: `Vec<(config_file_name, config_content>)` @@ -89,18 +89,22 @@ fn generate_config( Ok((interfaces, config)) } -fn extract_interfaces(network_state: &NetworkState, config_files: &NetworkConfig) -> Vec { - - let mut config_files_map :HashMap> = HashMap::new(); +fn extract_interfaces( + network_state: &NetworkState, + config_files: &NetworkConfig, +) -> Vec { + let mut config_files_map: HashMap> = HashMap::new(); for (_filename, content) in config_files { let mut config = Ini::new(); - config.read(content.to_string()).expect("Unable to read nmconnection file"); + config + .read(content.to_string()) + .expect("Unable to read nmconnection file"); let id = config.get("connection", "id"); let interface_name = config.get("connection", "interface-name"); - if id.is_some() && interface_name.is_some() - { - config_files_map.entry(interface_name.unwrap().to_string()) + if id.is_some() && interface_name.is_some() { + config_files_map + .entry(interface_name.unwrap().to_string()) .or_insert_with(|| Vec::new()) .push(id.unwrap().clone()); } @@ -114,7 +118,10 @@ fn extract_interfaces(network_state: &NetworkState, config_files: &NetworkConfig logical_name: i.name().to_owned(), mac_address: i.base_iface().mac_address.clone(), interface_type: i.iface_type().to_string(), - connection_ids: config_files_map.get(i.name()).map(Vec::clone).or_else(|| Some(Vec::new())) + connection_ids: config_files_map + .get(i.name()) + .map(Vec::clone) + .or_else(|| Some(Vec::new())), }) .collect() } @@ -190,11 +197,13 @@ fn store_network_mapping( #[cfg(test)] mod tests { - use std::fs; - use std::path::Path; - use crate::generate_conf::{extract_hostname, extract_interfaces, generate, generate_config, validate_interfaces}; + use crate::generate_conf::{ + extract_hostname, extract_interfaces, generate, generate_config, validate_interfaces, + }; use crate::types::{Host, Interface}; use crate::HOST_MAPPING_FILE; + use std::fs; + use std::path::Path; #[test] fn generate_successfully() -> Result<(), anyhow::Error> { @@ -234,13 +243,17 @@ mod tests { assert_eq!(exp_hosts, hosts); // verify contents of *.nmconnection files based on interface.connection_ids - hosts.iter_mut() + hosts + .iter_mut() .flat_map(|h| h.interfaces.iter()) .flat_map(|interface| &interface.connection_ids) .flat_map(|conns| conns.iter()) .for_each(|conn_id| { - let exp_conn = fs::read_to_string(exp_output_path.join(format!("{conn_id}.nmconnection"))).unwrap(); - let conn = fs::read_to_string(output_path.join(format!("{conn_id}.nmconnection"))).unwrap(); + let exp_conn = + fs::read_to_string(exp_output_path.join(format!("{conn_id}.nmconnection"))) + .unwrap(); + let conn = fs::read_to_string(output_path.join(format!("{conn_id}.nmconnection"))) + .unwrap(); assert_eq!(exp_conn, conn); }); @@ -289,8 +302,10 @@ mod tests { "#, )?; - let config_files = vec![generate_config_file("eth1".to_string(), "eth1".to_string()), - generate_config_file("bridge0".to_string(), "bridge0".to_string())]; + let config_files = vec![ + generate_config_file("eth1".to_string(), "eth1".to_string()), + generate_config_file("bridge0".to_string(), "bridge0".to_string()), + ]; let mut interfaces = extract_interfaces(&net_state, &config_files); interfaces.sort_by(|a, b| a.logical_name.cmp(&b.logical_name)); @@ -316,7 +331,7 @@ mod tests { Ok(()) } - fn generate_config_file(logical_name: String, connection_id: String) -> (String, String){ + fn generate_config_file(logical_name: String, connection_id: String) -> (String, String) { let filename = format!("{connection_id}.nmconnection"); let mut config = configparser::ini::Ini::new(); From 425f1f571aa7f87c34fe61c8268a501bc78d9720 Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Fri, 11 Oct 2024 17:48:12 +0200 Subject: [PATCH 03/10] Fix clippy issues --- src/generate_conf.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index c838246..0aae95b 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -100,13 +100,13 @@ fn extract_interfaces( config .read(content.to_string()) .expect("Unable to read nmconnection file"); - let id = config.get("connection", "id"); - let interface_name = config.get("connection", "interface-name"); - if id.is_some() && interface_name.is_some() { - config_files_map - .entry(interface_name.unwrap().to_string()) - .or_insert_with(|| Vec::new()) - .push(id.unwrap().clone()); + if let Some(interface_name) = config.get("connection", "interface-name") { + if let Some(id) = config.get("connection", "id") { + config_files_map + .entry(interface_name.to_string()) + .or_default() + .push(id.clone()); + } } } @@ -120,7 +120,7 @@ fn extract_interfaces( interface_type: i.iface_type().to_string(), connection_ids: config_files_map .get(i.name()) - .map(Vec::clone) + .cloned() .or_else(|| Some(Vec::new())), }) .collect() From 4ad178b937ee4512038de31ae391af1d7fc7fa8a Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Mon, 21 Oct 2024 12:52:05 +0200 Subject: [PATCH 04/10] Address review comments Signed-off-by: Koen de Laat --- src/apply_conf.rs | 86 ++++++++++++++------- src/generate_conf.rs | 10 +-- testdata/apply/config/host_config.yaml | 29 +++++++ testdata/apply/node1/br1-br.nmconnection | 18 +++++ testdata/apply/node1/eth2-port.nmconnection | 11 +++ testdata/apply/node1/eth2.nmconnection | 28 ++----- 6 files changed, 129 insertions(+), 53 deletions(-) create mode 100644 testdata/apply/node1/br1-br.nmconnection create mode 100644 testdata/apply/node1/eth2-port.nmconnection diff --git a/src/apply_conf.rs b/src/apply_conf.rs index d1d342e..0a5f5f7 100644 --- a/src/apply_conf.rs +++ b/src/apply_conf.rs @@ -181,12 +181,11 @@ fn copy_connection_files( .to_str() .ok_or_else(|| anyhow!("Determining host config path"))?; - for interface in &host.interfaces { + for interface in host.interfaces { info!("Processing interface '{}'...", &interface.logical_name); let connections = &interface .connection_ids - .clone() - .unwrap_or(vec![interface.logical_name.clone()]); + .unwrap_or_else(Vec::new); for connection in connections { info!("Processing connection '{}'...", connection); @@ -309,7 +308,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0".to_string()]), }], }, Host { @@ -318,7 +317,7 @@ mod tests { logical_name: "".to_string(), mac_address: Option::from("10:10:10:10:10:10".to_string()), interface_type: "".to_string(), - connection_ids: None, + connection_ids: Option::from(Vec::new()), }], }, ]; @@ -345,7 +344,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0".to_string()]), }] ); } @@ -359,7 +358,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("10:20:30:40:50:60".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0".to_string()]), }], }, Host { @@ -368,7 +367,7 @@ mod tests { logical_name: "".to_string(), mac_address: Option::from("00:10:20:30:40:50".to_string()), interface_type: "".to_string(), - connection_ids: None, + connection_ids: Option::from(Vec::new()), }], }, ]; @@ -401,25 +400,25 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0".to_string()]), }, Interface { logical_name: "eth1".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth1".to_string()]), }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("36:5e:6b:a2:ed:80".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth2".to_string()]), }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:aa:44:58".to_string()), interface_type: "bond".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["bond0".to_string()]), }, ], }, @@ -430,13 +429,36 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("36:5e:6b:a2:ed:81".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0".to_string()]), }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0.1365".to_string()]), + }, + ], + }, + Host { + hostname: "node3".to_string(), + interfaces: vec![ + Interface { + logical_name: "br1".to_string(), + mac_address: None, + interface_type: "ovs-bridge".to_string(), + connection_ids: Option::from(vec!["br1-br".to_string()]), + }, + Interface { + logical_name: "ovs0".to_string(), + mac_address: None, + interface_type: "ovs-interface".to_string(), + connection_ids: Option::from(vec!["ovs0-port".to_string(), "ovs0-if".to_string()]), + }, + Interface { + logical_name: "eth0".to_string(), + mac_address: Option::from("95:b2:92:88:1d:3f".to_string()), + interface_type: "ethernet".to_string(), + connection_ids: Option::from(vec!["eth0".to_string(), "eth0-port".to_string()]), }, ], }, @@ -453,31 +475,31 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0".to_string()]), }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0.1365".to_string()]), }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth2".to_string()]), }, Interface { logical_name: "eth2.bridge".to_string(), mac_address: None, interface_type: "linux-bridge".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth2.bridge".to_string()]), }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["bond0".to_string()]), }, ], }; @@ -545,38 +567,44 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0".to_string()]), }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth0.1365".to_string()]), + }, + Interface { + logical_name: "br1".to_string(), + mac_address: None, + interface_type: "ovs-bridge".to_string(), + connection_ids: Option::from(vec!["br1-br".to_string()]), }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth2".to_string(), "eth2-port".to_string()]), }, Interface { logical_name: "eth1".to_string(), mac_address: Option::from("00:11:22:33:44:57".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["eth1".to_string()]), }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), - connection_ids: None, + connection_ids: Option::from(vec!["bond0".to_string()]), }, ], }; let detected_interfaces = HashMap::from([("eth2".to_string(), "eth4".to_string())]); assert!( - copy_connection_files(host, detected_interfaces, source_dir, destination_dir).is_ok() + copy_connection_files(host, detected_interfaces.clone(), source_dir, destination_dir).is_ok() ); let source_path = Path::new(source_dir).join("node1"); @@ -588,9 +616,11 @@ mod tests { let mut input = fs::read_to_string(entry.path())?; // Adjust the name and content for the "eth2"->"eth4" edge case. - if entry.path().file_stem().is_some_and(|stem| stem == "eth2") { - filename = filename.replace("eth2", "eth4"); - input = input.replace("eth2", "eth4"); + for (src_stem, dst_stem) in detected_interfaces.iter() { + if entry.path().file_stem().is_some_and(|stem| stem.to_str().unwrap().contains(src_stem)) { + filename = filename.replace(src_stem, dst_stem); + input = input.replace(src_stem, dst_stem); + } } let output = fs::read_to_string(destination_path.join(&filename))?; diff --git a/src/generate_conf.rs b/src/generate_conf.rs index 0aae95b..cbfa32d 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -93,19 +93,19 @@ fn extract_interfaces( network_state: &NetworkState, config_files: &NetworkConfig, ) -> Vec { - let mut config_files_map: HashMap> = HashMap::new(); + let mut connection_ids: HashMap> = HashMap::new(); - for (_filename, content) in config_files { + for (_, content) in config_files { let mut config = Ini::new(); config .read(content.to_string()) .expect("Unable to read nmconnection file"); if let Some(interface_name) = config.get("connection", "interface-name") { if let Some(id) = config.get("connection", "id") { - config_files_map + connection_ids .entry(interface_name.to_string()) .or_default() - .push(id.clone()); + .push(id); } } } @@ -118,7 +118,7 @@ fn extract_interfaces( logical_name: i.name().to_owned(), mac_address: i.base_iface().mac_address.clone(), interface_type: i.iface_type().to_string(), - connection_ids: config_files_map + connection_ids: connection_ids .get(i.name()) .cloned() .or_else(|| Some(Vec::new())), diff --git a/testdata/apply/config/host_config.yaml b/testdata/apply/config/host_config.yaml index a870694..339d4e3 100644 --- a/testdata/apply/config/host_config.yaml +++ b/testdata/apply/config/host_config.yaml @@ -3,19 +3,48 @@ - logical_name: eth0 mac_address: 00:11:22:33:44:55 interface_type: ethernet + connection_ids: + - eth0 - logical_name: eth1 mac_address: 00:11:22:33:44:58 interface_type: ethernet + connection_ids: + - eth1 - logical_name: eth2 mac_address: 36:5e:6b:a2:ed:80 interface_type: ethernet + connection_ids: + - eth2 - logical_name: bond0 mac_address: 00:11:22:AA:44:58 interface_type: bond + connection_ids: + - bond0 - hostname: node2 interfaces: - logical_name: eth0 mac_address: 36:5E:6B:A2:ED:81 interface_type: ethernet + connection_ids: + - eth0 - logical_name: eth0.1365 interface_type: vlan + connection_ids: + - eth0.1365 +- hostname: node3 + interfaces: + - logical_name: br1 + connection_ids: + - br1-br + interface_type: ovs-bridge + - logical_name: ovs0 + connection_ids: + - ovs0-port + - ovs0-if + interface_type: ovs-interface + - logical_name: eth0 + connection_ids: + - eth0 + - eth0-port + mac_address: 95:B2:92:88:1D:3F + interface_type: ethernet \ No newline at end of file diff --git a/testdata/apply/node1/br1-br.nmconnection b/testdata/apply/node1/br1-br.nmconnection new file mode 100644 index 0000000..d04a3d8 --- /dev/null +++ b/testdata/apply/node1/br1-br.nmconnection @@ -0,0 +1,18 @@ +[connection] +autoconnect=true +autoconnect-slaves=1 +id=br1-br +interface-name=br1 +type=ovs-bridge +uuid=327127e8-ee26-5178-bacd-ebbb191b1bdd + +[ipv4] +dhcp-timeout=2147483647 +method=disabled + +[ipv6] +dhcp-timeout=2147483647 +method=disabled + +[ovs-bridge] +stp-enable=true diff --git a/testdata/apply/node1/eth2-port.nmconnection b/testdata/apply/node1/eth2-port.nmconnection new file mode 100644 index 0000000..bb36b72 --- /dev/null +++ b/testdata/apply/node1/eth2-port.nmconnection @@ -0,0 +1,11 @@ +[connection] +autoconnect=true +autoconnect-slaves=-1 +id=eth2-port +interface-name=eth2 +master=br1 +slave-type=ovs-bridge +type=ovs-port +uuid=5e4ac41f-d8dd-5cbd-98b3-122629af91e0 + +[ovs-port] diff --git a/testdata/apply/node1/eth2.nmconnection b/testdata/apply/node1/eth2.nmconnection index d7d5be7..3994c3e 100644 --- a/testdata/apply/node1/eth2.nmconnection +++ b/testdata/apply/node1/eth2.nmconnection @@ -1,23 +1,11 @@ [connection] -id = eth2 -uuid = ad451df9-e022-4ce4-9ba1-4bc691c9abc1 -type = ethernet -interface-name = eth2 +autoconnect=true +autoconnect-slaves=-1 +id=eth2 +interface-name=eth2 +master=eth2 +slave-type=ovs-port +type=802-3-ethernet +uuid=0523c0a1-5f5e-5603-bcf2-68155d5d322e [ethernet] - -[ipv4] -address1 = 192.168.123.3/24 -dns = 192.168.123.100 -dns-priority = 40 -method = manual -route1 = 0.0.0.0/0,192.168.123.3 -route1_options = table=254 - -[ipv6] -addr-gen-mode = eui64 -dhcp-duid = ll -dhcp-iaid = mac -method = disabled - -[proxy] From 13a168b15539cba6ac26dfdd514f178ce5975952 Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Mon, 21 Oct 2024 13:14:43 +0200 Subject: [PATCH 05/10] Rework generate_config Signed-off-by: Koen de Laat --- src/apply_conf.rs | 30 +++++++++++++++------- src/generate_conf.rs | 61 ++++++++++++++++++++++---------------------- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/apply_conf.rs b/src/apply_conf.rs index 0a5f5f7..2a63f2b 100644 --- a/src/apply_conf.rs +++ b/src/apply_conf.rs @@ -183,9 +183,7 @@ fn copy_connection_files( for interface in host.interfaces { info!("Processing interface '{}'...", &interface.logical_name); - let connections = &interface - .connection_ids - .unwrap_or_else(Vec::new); + let connections = &interface.connection_ids.unwrap_or_else(Vec::new); for connection in connections { info!("Processing connection '{}'...", connection); @@ -452,13 +450,19 @@ mod tests { logical_name: "ovs0".to_string(), mac_address: None, interface_type: "ovs-interface".to_string(), - connection_ids: Option::from(vec!["ovs0-port".to_string(), "ovs0-if".to_string()]), + connection_ids: Option::from(vec![ + "ovs0-port".to_string(), + "ovs0-if".to_string() + ]), }, Interface { logical_name: "eth0".to_string(), mac_address: Option::from("95:b2:92:88:1d:3f".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth0".to_string(), "eth0-port".to_string()]), + connection_ids: Option::from(vec![ + "eth0".to_string(), + "eth0-port".to_string() + ]), }, ], }, @@ -603,9 +607,13 @@ mod tests { }; let detected_interfaces = HashMap::from([("eth2".to_string(), "eth4".to_string())]); - assert!( - copy_connection_files(host, detected_interfaces.clone(), source_dir, destination_dir).is_ok() - ); + assert!(copy_connection_files( + host, + detected_interfaces.clone(), + source_dir, + destination_dir + ) + .is_ok()); let source_path = Path::new(source_dir).join("node1"); let destination_path = Path::new(destination_dir); @@ -617,7 +625,11 @@ mod tests { // Adjust the name and content for the "eth2"->"eth4" edge case. for (src_stem, dst_stem) in detected_interfaces.iter() { - if entry.path().file_stem().is_some_and(|stem| stem.to_str().unwrap().contains(src_stem)) { + if entry + .path() + .file_stem() + .is_some_and(|stem| stem.to_str().unwrap().contains(src_stem)) + { filename = filename.replace(src_stem, dst_stem); input = input.replace(src_stem, dst_stem); } diff --git a/src/generate_conf.rs b/src/generate_conf.rs index cbfa32d..f4c00bf 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use std::ffi::OsStr; use std::fs; use std::path::Path; @@ -77,39 +76,40 @@ fn generate_config( ) -> Result<(Vec, NetworkConfig), anyhow::Error> { let network_state = NetworkState::new_from_yaml(&data)?; + let mut interfaces = extract_interfaces(&network_state); + validate_interfaces(&interfaces, require_mac_addresses)?; + let config = network_state .gen_conf()? .get("NetworkManager") .ok_or_else(|| anyhow!("Invalid NM configuration"))? .to_owned(); - let interfaces = extract_interfaces(&network_state, &config); - validate_interfaces(&interfaces, require_mac_addresses)?; + populate_connection_ids(&mut interfaces, &config); Ok((interfaces, config)) } -fn extract_interfaces( - network_state: &NetworkState, - config_files: &NetworkConfig, -) -> Vec { - let mut connection_ids: HashMap> = HashMap::new(); - - for (_, content) in config_files { +fn populate_connection_ids(interfaces: &mut [Interface], config: &NetworkConfig) { + for (_, content) in config { let mut config = Ini::new(); config .read(content.to_string()) .expect("Unable to read nmconnection file"); if let Some(interface_name) = config.get("connection", "interface-name") { if let Some(id) = config.get("connection", "id") { - connection_ids - .entry(interface_name.to_string()) - .or_default() - .push(id); + interfaces + .iter_mut() + .filter(|x| x.logical_name == interface_name) + .for_each(|interface| { + interface.connection_ids.as_mut().unwrap().push(id.clone()) + }); } } } +} +fn extract_interfaces(network_state: &NetworkState) -> Vec { network_state .interfaces .iter() @@ -118,10 +118,7 @@ fn extract_interfaces( logical_name: i.name().to_owned(), mac_address: i.base_iface().mac_address.clone(), interface_type: i.iface_type().to_string(), - connection_ids: connection_ids - .get(i.name()) - .cloned() - .or_else(|| Some(Vec::new())), + connection_ids: Some(Vec::new()), }) .collect() } @@ -198,7 +195,8 @@ fn store_network_mapping( #[cfg(test)] mod tests { use crate::generate_conf::{ - extract_hostname, extract_interfaces, generate, generate_config, validate_interfaces, + extract_hostname, extract_interfaces, generate, generate_config, populate_connection_ids, + validate_interfaces, }; use crate::types::{Host, Interface}; use crate::HOST_MAPPING_FILE; @@ -307,7 +305,8 @@ mod tests { generate_config_file("bridge0".to_string(), "bridge0".to_string()), ]; - let mut interfaces = extract_interfaces(&net_state, &config_files); + let mut interfaces = extract_interfaces(&net_state); + populate_connection_ids(&mut interfaces, &config_files); interfaces.sort_by(|a, b| a.logical_name.cmp(&b.logical_name)); assert_eq!( @@ -348,13 +347,13 @@ mod tests { logical_name: "eth3.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: None, + connection_ids: Some(vec!["eth3.1365".to_string()]), }, Interface { logical_name: "bond0".to_string(), mac_address: None, interface_type: "bond".to_string(), - connection_ids: None, + connection_ids: Some(vec!["bond0".to_string()]), }, ]; @@ -369,37 +368,37 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Some(vec!["eth0".to_string()]), }, Interface { logical_name: "eth1".to_string(), mac_address: None, interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Some(vec!["eth1".to_string()]), }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Some(vec!["eth2".to_string()]), }, Interface { logical_name: "eth3".to_string(), mac_address: None, interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Some(vec!["eth3".to_string()]), }, Interface { logical_name: "eth3.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: None, + connection_ids: Some(vec!["eth3.1365".to_string()]), }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), - connection_ids: None, + connection_ids: Some(vec!["bond0".to_string()]), }, ]; @@ -420,19 +419,19 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: None, + connection_ids: Some(vec!["eth0".to_string()]), }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: None, + connection_ids: Some(vec!["eth0.1365".to_string()]), }, Interface { logical_name: "bond0".to_string(), mac_address: None, interface_type: "bond".to_string(), - connection_ids: None, + connection_ids: Some(vec!["bond0".to_string()]), }, ]; From d7a5d2b4ecaaf78d7a16ecdc1a38944318611278 Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Wed, 30 Oct 2024 15:01:35 +0100 Subject: [PATCH 06/10] Proper error handling Signed-off-by: Koen de Laat --- src/apply_conf.rs | 58 ++++++++---------- src/generate_conf.rs | 141 +++++++++++++++++++++++++++++++------------ src/types.rs | 3 +- 3 files changed, 128 insertions(+), 74 deletions(-) diff --git a/src/apply_conf.rs b/src/apply_conf.rs index 2a63f2b..d32b971 100644 --- a/src/apply_conf.rs +++ b/src/apply_conf.rs @@ -183,7 +183,7 @@ fn copy_connection_files( for interface in host.interfaces { info!("Processing interface '{}'...", &interface.logical_name); - let connections = &interface.connection_ids.unwrap_or_else(Vec::new); + let connections = &interface.connection_ids; for connection in connections { info!("Processing connection '{}'...", connection); @@ -306,7 +306,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }], }, Host { @@ -315,7 +315,7 @@ mod tests { logical_name: "".to_string(), mac_address: Option::from("10:10:10:10:10:10".to_string()), interface_type: "".to_string(), - connection_ids: Option::from(Vec::new()), + connection_ids: Vec::new(), }], }, ]; @@ -342,7 +342,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }] ); } @@ -356,7 +356,7 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("10:20:30:40:50:60".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }], }, Host { @@ -365,7 +365,7 @@ mod tests { logical_name: "".to_string(), mac_address: Option::from("00:10:20:30:40:50".to_string()), interface_type: "".to_string(), - connection_ids: Option::from(Vec::new()), + connection_ids: Vec::new(), }], }, ]; @@ -398,25 +398,25 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }, Interface { logical_name: "eth1".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth1".to_string()]), + connection_ids: vec!["eth1".to_string()], }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("36:5e:6b:a2:ed:80".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth2".to_string()]), + connection_ids: vec!["eth2".to_string()], }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:aa:44:58".to_string()), interface_type: "bond".to_string(), - connection_ids: Option::from(vec!["bond0".to_string()]), + connection_ids: vec!["bond0".to_string()], }, ], }, @@ -427,13 +427,13 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("36:5e:6b:a2:ed:81".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: Option::from(vec!["eth0.1365".to_string()]), + connection_ids: vec!["eth0.1365".to_string()], }, ], }, @@ -444,25 +444,19 @@ mod tests { logical_name: "br1".to_string(), mac_address: None, interface_type: "ovs-bridge".to_string(), - connection_ids: Option::from(vec!["br1-br".to_string()]), + connection_ids: vec!["br1-br".to_string()], }, Interface { logical_name: "ovs0".to_string(), mac_address: None, interface_type: "ovs-interface".to_string(), - connection_ids: Option::from(vec![ - "ovs0-port".to_string(), - "ovs0-if".to_string() - ]), + connection_ids: vec!["ovs0-port".to_string(), "ovs0-if".to_string()], }, Interface { logical_name: "eth0".to_string(), mac_address: Option::from("95:b2:92:88:1d:3f".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec![ - "eth0".to_string(), - "eth0-port".to_string() - ]), + connection_ids: vec!["eth0".to_string(), "eth0-port".to_string()], }, ], }, @@ -479,31 +473,31 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: Option::from(vec!["eth0.1365".to_string()]), + connection_ids: vec!["eth0.1365".to_string()], }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth2".to_string()]), + connection_ids: vec!["eth2".to_string()], }, Interface { logical_name: "eth2.bridge".to_string(), mac_address: None, interface_type: "linux-bridge".to_string(), - connection_ids: Option::from(vec!["eth2.bridge".to_string()]), + connection_ids: vec!["eth2.bridge".to_string()], }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), - connection_ids: Option::from(vec!["bond0".to_string()]), + connection_ids: vec!["bond0".to_string()], }, ], }; @@ -571,37 +565,37 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: Option::from(vec!["eth0.1365".to_string()]), + connection_ids: vec!["eth0.1365".to_string()], }, Interface { logical_name: "br1".to_string(), mac_address: None, interface_type: "ovs-bridge".to_string(), - connection_ids: Option::from(vec!["br1-br".to_string()]), + connection_ids: vec!["br1-br".to_string()], }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth2".to_string(), "eth2-port".to_string()]), + connection_ids: vec!["eth2".to_string(), "eth2-port".to_string()], }, Interface { logical_name: "eth1".to_string(), mac_address: Option::from("00:11:22:33:44:57".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Option::from(vec!["eth1".to_string()]), + connection_ids: vec!["eth1".to_string()], }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), - connection_ids: Option::from(vec!["bond0".to_string()]), + connection_ids: vec!["bond0".to_string()], }, ], }; diff --git a/src/generate_conf.rs b/src/generate_conf.rs index f4c00bf..5d1114f 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -15,7 +15,7 @@ type NetworkConfig = Vec<(String, String)>; /// Generate network configurations from all YAML files in the `config_dir` /// and store the result *.nmconnection files and host mapping (if applicable) under `output_dir`. -pub(crate) fn generate(config_dir: &str, output_dir: &str) -> Result<(), anyhow::Error> { +pub(crate) fn generate(config_dir: &str, output_dir: &str) -> anyhow::Result<()> { let files_count = fs::read_dir(config_dir)?.count(); if files_count == 0 { @@ -73,7 +73,7 @@ fn extract_hostname(path: &Path) -> Option<&OsStr> { fn generate_config( data: String, require_mac_addresses: bool, -) -> Result<(Vec, NetworkConfig), anyhow::Error> { +) -> anyhow::Result<(Vec, NetworkConfig)> { let network_state = NetworkState::new_from_yaml(&data)?; let mut interfaces = extract_interfaces(&network_state); @@ -85,28 +85,58 @@ fn generate_config( .ok_or_else(|| anyhow!("Invalid NM configuration"))? .to_owned(); - populate_connection_ids(&mut interfaces, &config); + populate_connection_ids(&mut interfaces, &config)?; + validate_connection_ids(&interfaces)?; Ok((interfaces, config)) } -fn populate_connection_ids(interfaces: &mut [Interface], config: &NetworkConfig) { - for (_, content) in config { - let mut config = Ini::new(); - config - .read(content.to_string()) - .expect("Unable to read nmconnection file"); - if let Some(interface_name) = config.get("connection", "interface-name") { - if let Some(id) = config.get("connection", "id") { - interfaces - .iter_mut() - .filter(|x| x.logical_name == interface_name) - .for_each(|interface| { - interface.connection_ids.as_mut().unwrap().push(id.clone()) - }); - } - } - } +fn validate_connection_ids(interfaces: &[Interface]) -> anyhow::Result<()> { + let empty_connection_ids: Vec = interfaces + .iter() + .filter(|i| i.connection_ids.is_empty()) + .map(|i| i.logical_name.to_owned()) + .collect(); + + if !empty_connection_ids.is_empty() { + return Err(anyhow!( + "Detected interfaces without connection files: {}", + empty_connection_ids.join(", ") + )); + }; + + Ok(()) +} + +fn populate_connection_ids( + interfaces: &mut [Interface], + config: &NetworkConfig, +) -> anyhow::Result<()> { + config + .iter() + .filter(|(filename, _)| filename != "lo.nmconnection") + .try_for_each(|(filename, content)| { + let mut config = Ini::new(); + config.read(content.to_string()).map_err(|e| anyhow!(e))?; + let interface_name = config.get("connection", "interface-name").ok_or_else(|| { + anyhow!("No interface-name found in connection file: {}", filename) + })?; + let connection_id = config.get("connection", "id").ok_or_else(|| { + anyhow!("No connection id found in connection file: {}", filename) + })?; + interfaces + .iter_mut() + .find(|x| x.logical_name == interface_name) + .ok_or_else(|| { + anyhow!( + "No matching interface found for connection file: {}", + filename + ) + })? + .connection_ids + .push(connection_id); + Ok(()) + }) } fn extract_interfaces(network_state: &NetworkState) -> Vec { @@ -118,7 +148,7 @@ fn extract_interfaces(network_state: &NetworkState) -> Vec { logical_name: i.name().to_owned(), mac_address: i.base_iface().mac_address.clone(), interface_type: i.iface_type().to_string(), - connection_ids: Some(Vec::new()), + connection_ids: Vec::new(), }) .collect() } @@ -160,7 +190,7 @@ fn store_network_config( output_dir: &str, hostname: &str, config: NetworkConfig, -) -> Result<(), anyhow::Error> { +) -> anyhow::Result<()> { let path = Path::new(output_dir).join(hostname); fs::create_dir_all(&path).context("Creating output dir")?; @@ -176,7 +206,7 @@ fn store_network_mapping( output_dir: &str, hostname: String, interfaces: Vec, -) -> Result<(), anyhow::Error> { +) -> anyhow::Result<()> { let path = Path::new(output_dir); let mapping_file = fs::OpenOptions::new() @@ -196,7 +226,7 @@ fn store_network_mapping( mod tests { use crate::generate_conf::{ extract_hostname, extract_interfaces, generate, generate_config, populate_connection_ids, - validate_interfaces, + validate_connection_ids, validate_interfaces, }; use crate::types::{Host, Interface}; use crate::HOST_MAPPING_FILE; @@ -210,7 +240,7 @@ mod tests { let out_dir = "_out"; let output_path = Path::new("_out").join("node1"); - assert!(generate(config_dir, out_dir).is_ok()); + generate(config_dir, out_dir)?; // verify contents of lo.nmconnection files let exp_lo_conn = fs::read_to_string(exp_output_path.join("lo.nmconnection"))?; @@ -245,7 +275,6 @@ mod tests { .iter_mut() .flat_map(|h| h.interfaces.iter()) .flat_map(|interface| &interface.connection_ids) - .flat_map(|conns| conns.iter()) .for_each(|conn_id| { let exp_conn = fs::read_to_string(exp_output_path.join(format!("{conn_id}.nmconnection"))) @@ -306,7 +335,7 @@ mod tests { ]; let mut interfaces = extract_interfaces(&net_state); - populate_connection_ids(&mut interfaces, &config_files); + populate_connection_ids(&mut interfaces, &config_files).expect("populate ids"); interfaces.sort_by(|a, b| a.logical_name.cmp(&b.logical_name)); assert_eq!( @@ -316,13 +345,13 @@ mod tests { logical_name: "bridge0".to_string(), mac_address: Option::from("FE:C4:05:42:8B:AB".to_string()), interface_type: "linux-bridge".to_string(), - connection_ids: Some(vec!["bridge0".to_string()]), + connection_ids: vec!["bridge0".to_string()], }, Interface { logical_name: "eth1".to_string(), mac_address: Option::from("FE:C4:05:42:8B:AA".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Some(vec!["eth1".to_string()]), + connection_ids: vec!["eth1".to_string()], }, ] ); @@ -347,13 +376,13 @@ mod tests { logical_name: "eth3.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: Some(vec!["eth3.1365".to_string()]), + connection_ids: vec!["eth3.1365".to_string()], }, Interface { logical_name: "bond0".to_string(), mac_address: None, interface_type: "bond".to_string(), - connection_ids: Some(vec!["bond0".to_string()]), + connection_ids: vec!["bond0".to_string()], }, ]; @@ -368,37 +397,37 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Some(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }, Interface { logical_name: "eth1".to_string(), mac_address: None, interface_type: "ethernet".to_string(), - connection_ids: Some(vec!["eth1".to_string()]), + connection_ids: vec!["eth1".to_string()], }, Interface { logical_name: "eth2".to_string(), mac_address: Option::from("00:11:22:33:44:56".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Some(vec!["eth2".to_string()]), + connection_ids: vec!["eth2".to_string()], }, Interface { logical_name: "eth3".to_string(), mac_address: None, interface_type: "ethernet".to_string(), - connection_ids: Some(vec!["eth3".to_string()]), + connection_ids: vec!["eth3".to_string()], }, Interface { logical_name: "eth3.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: Some(vec!["eth3.1365".to_string()]), + connection_ids: vec!["eth3.1365".to_string()], }, Interface { logical_name: "bond0".to_string(), mac_address: Option::from("00:11:22:33:44:58".to_string()), interface_type: "bond".to_string(), - connection_ids: Some(vec!["bond0".to_string()]), + connection_ids: vec!["bond0".to_string()], }, ]; @@ -412,6 +441,37 @@ mod tests { assert!(validate_interfaces(&interfaces, false).is_ok()) } + #[test] + fn validate_interfaces_missing_connection_ids() { + let interfaces = vec![ + Interface { + logical_name: "eth0".to_string(), + mac_address: Option::from("00:11:22:33:44:55".to_string()), + interface_type: "ethernet".to_string(), + connection_ids: vec!["eth0".to_string()], + }, + Interface { + logical_name: "eth0.1365".to_string(), + mac_address: None, + interface_type: "vlan".to_string(), + connection_ids: vec!["eth0.1365".to_string()], + }, + Interface { + logical_name: "bond0".to_string(), + mac_address: None, + interface_type: "bond".to_string(), + connection_ids: Vec::new(), + }, + ]; + + assert_eq!( + validate_connection_ids(&interfaces) + .unwrap_err() + .to_string(), + "Detected interfaces without connection files: bond0" + ); + } + #[test] fn validate_interfaces_successfully() { let interfaces = vec![ @@ -419,24 +479,25 @@ mod tests { logical_name: "eth0".to_string(), mac_address: Option::from("00:11:22:33:44:55".to_string()), interface_type: "ethernet".to_string(), - connection_ids: Some(vec!["eth0".to_string()]), + connection_ids: vec!["eth0".to_string()], }, Interface { logical_name: "eth0.1365".to_string(), mac_address: None, interface_type: "vlan".to_string(), - connection_ids: Some(vec!["eth0.1365".to_string()]), + connection_ids: vec!["eth0.1365".to_string()], }, Interface { logical_name: "bond0".to_string(), mac_address: None, interface_type: "bond".to_string(), - connection_ids: Some(vec!["bond0".to_string()]), + connection_ids: vec!["bond0".to_string()], }, ]; assert!(validate_interfaces(&interfaces, true).is_ok()); assert!(validate_interfaces(&interfaces, false).is_ok()); + assert!(validate_connection_ids(&interfaces).is_ok()); } #[test] diff --git a/src/types.rs b/src/types.rs index 817e8b2..02d3403 100644 --- a/src/types.rs +++ b/src/types.rs @@ -11,9 +11,8 @@ pub struct Host { #[cfg_attr(test, derive(PartialEq))] pub struct Interface { pub(crate) logical_name: String, - #[serde(skip_serializing_if = "Option::is_none")] #[serde(default)] - pub(crate) connection_ids: Option>, + pub(crate) connection_ids: Vec, #[serde(skip_serializing_if = "Option::is_none")] #[serde(default)] pub(crate) mac_address: Option, From ed6ded9823b3c65d967bb3c11d07c963bdb99a12 Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Thu, 31 Oct 2024 17:54:37 +0100 Subject: [PATCH 07/10] Filter loopback connections Signed-off-by: Koen de Laat --- src/generate_conf.rs | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index 5d1114f..b04c6fc 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -114,25 +114,27 @@ fn populate_connection_ids( ) -> anyhow::Result<()> { config .iter() - .filter(|(filename, _)| filename != "lo.nmconnection") - .try_for_each(|(filename, content)| { - let mut config = Ini::new(); - config.read(content.to_string()).map_err(|e| anyhow!(e))?; - let interface_name = config.get("connection", "interface-name").ok_or_else(|| { - anyhow!("No interface-name found in connection file: {}", filename) - })?; - let connection_id = config.get("connection", "id").ok_or_else(|| { - anyhow!("No connection id found in connection file: {}", filename) - })?; + .map(|(f, content)| { + let mut c = Ini::new(); + c.read(content.to_string()).map_err(|e| anyhow!(e))?; + Ok((f, c)) + }) + .filter(|x| { + !x.as_ref() + .is_ok_and(|(_, c)| c.get("connection", "type").is_some_and(|t| t == "loopback")) + }) + .try_for_each(|x: anyhow::Result<(&String, Ini)>| { + let (f, c) = x?; + let interface_name = c + .get("connection", "interface-name") + .ok_or_else(|| anyhow!("No interface-name found in connection file: {}", f))?; + let connection_id = c + .get("connection", "id") + .ok_or_else(|| anyhow!("No connection id found in connection file: {}", f))?; interfaces .iter_mut() .find(|x| x.logical_name == interface_name) - .ok_or_else(|| { - anyhow!( - "No matching interface found for connection file: {}", - filename - ) - })? + .ok_or_else(|| anyhow!("No matching interface found for connection file: {}", f))? .connection_ids .push(connection_id); Ok(()) From 8e27a6356fd9a389c32386a819b187e8395baa25 Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Thu, 31 Oct 2024 18:30:43 +0100 Subject: [PATCH 08/10] Add check for empty connection_ids Signed-off-by: Koen de Laat --- src/apply_conf.rs | 45 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/src/apply_conf.rs b/src/apply_conf.rs index d32b971..4771d4f 100644 --- a/src/apply_conf.rs +++ b/src/apply_conf.rs @@ -48,7 +48,7 @@ pub(crate) fn apply(source_dir: &str) -> Result<(), anyhow::Error> { source_dir, STATIC_SYSTEM_CONNECTIONS_DIR, ) - .context("Copying connection files")?; + .context("Copying connection files")?; } disable_wired_connections(CONFIG_DIR, RUNTIME_SYSTEM_CONNECTIONS_DIR) @@ -141,10 +141,10 @@ fn copy_unified_connection_files( if entry.metadata()?.is_dir() || path - .extension() - .and_then(OsStr::to_str) - .unwrap_or_default() - .ne(CONNECTION_FILE_EXT) + .extension() + .and_then(OsStr::to_str) + .unwrap_or_default() + .ne(CONNECTION_FILE_EXT) { warn!("Ignoring unexpected entry: {path:?}"); continue; @@ -185,6 +185,10 @@ fn copy_connection_files( info!("Processing interface '{}'...", &interface.logical_name); let connections = &interface.connection_ids; + if connections.is_empty() { + return Err(anyhow!("Missing connection ids for {}", &interface.logical_name)); + } + for connection in connections { info!("Processing connection '{}'...", connection); let mut filename = connection.clone(); @@ -606,8 +610,7 @@ mod tests { detected_interfaces.clone(), source_dir, destination_dir - ) - .is_ok()); + ).is_ok()); let source_path = Path::new(source_dir).join("node1"); let destination_path = Path::new(destination_dir); @@ -638,6 +641,34 @@ mod tests { fs::remove_dir_all(destination_dir) } + #[test] + fn copy_connection_files_missing_connection_ids() -> io::Result<()> { + let source_dir = "testdata/apply"; + let destination_dir = "_out2"; + + let host = Host { + hostname: "node1".to_string(), + interfaces: vec![ + Interface { + logical_name: "eth0".to_string(), + mac_address: Option::from("00:11:22:33:44:55".to_string()), + interface_type: "ethernet".to_string(), + connection_ids: Vec::new(), + }, + ], + }; + + assert!(copy_connection_files( + host, + HashMap::new(), + source_dir, + destination_dir + ).is_err_and(|e| e.to_string().contains("Missing connection ids"))); + + // cleanup + fs::remove_dir_all(destination_dir) + } + #[test] fn generate_keyfile_path() { assert_eq!( From 68418be18927d8f81418c1081c6ddade093450de Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Tue, 5 Nov 2024 09:09:45 +0100 Subject: [PATCH 09/10] Fix linting Signed-off-by: Koen de Laat --- src/apply_conf.rs | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/apply_conf.rs b/src/apply_conf.rs index 4771d4f..e64e342 100644 --- a/src/apply_conf.rs +++ b/src/apply_conf.rs @@ -48,7 +48,7 @@ pub(crate) fn apply(source_dir: &str) -> Result<(), anyhow::Error> { source_dir, STATIC_SYSTEM_CONNECTIONS_DIR, ) - .context("Copying connection files")?; + .context("Copying connection files")?; } disable_wired_connections(CONFIG_DIR, RUNTIME_SYSTEM_CONNECTIONS_DIR) @@ -141,10 +141,10 @@ fn copy_unified_connection_files( if entry.metadata()?.is_dir() || path - .extension() - .and_then(OsStr::to_str) - .unwrap_or_default() - .ne(CONNECTION_FILE_EXT) + .extension() + .and_then(OsStr::to_str) + .unwrap_or_default() + .ne(CONNECTION_FILE_EXT) { warn!("Ignoring unexpected entry: {path:?}"); continue; @@ -186,7 +186,10 @@ fn copy_connection_files( let connections = &interface.connection_ids; if connections.is_empty() { - return Err(anyhow!("Missing connection ids for {}", &interface.logical_name)); + return Err(anyhow!( + "Missing connection ids for {}", + &interface.logical_name + )); } for connection in connections { @@ -610,7 +613,8 @@ mod tests { detected_interfaces.clone(), source_dir, destination_dir - ).is_ok()); + ) + .is_ok()); let source_path = Path::new(source_dir).join("node1"); let destination_path = Path::new(destination_dir); @@ -648,22 +652,18 @@ mod tests { let host = Host { hostname: "node1".to_string(), - interfaces: vec![ - Interface { - logical_name: "eth0".to_string(), - mac_address: Option::from("00:11:22:33:44:55".to_string()), - interface_type: "ethernet".to_string(), - connection_ids: Vec::new(), - }, - ], + interfaces: vec![Interface { + logical_name: "eth0".to_string(), + mac_address: Option::from("00:11:22:33:44:55".to_string()), + interface_type: "ethernet".to_string(), + connection_ids: Vec::new(), + }], }; - assert!(copy_connection_files( - host, - HashMap::new(), - source_dir, - destination_dir - ).is_err_and(|e| e.to_string().contains("Missing connection ids"))); + assert!( + copy_connection_files(host, HashMap::new(), source_dir, destination_dir) + .is_err_and(|e| e.to_string().contains("Missing connection ids")) + ); // cleanup fs::remove_dir_all(destination_dir) From cd7f3b371e7fc2aa954f4c9f3f261301fe3239b3 Mon Sep 17 00:00:00 2001 From: Koen de Laat Date: Wed, 6 Nov 2024 13:43:08 +0100 Subject: [PATCH 10/10] Refactor `populate_connection_ids` Co-authored-by: Atanas Dinov Signed-off-by: Koen de Laat --- src/generate_conf.rs | 55 ++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index b04c6fc..72f1828 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -112,33 +112,34 @@ fn populate_connection_ids( interfaces: &mut [Interface], config: &NetworkConfig, ) -> anyhow::Result<()> { - config - .iter() - .map(|(f, content)| { - let mut c = Ini::new(); - c.read(content.to_string()).map_err(|e| anyhow!(e))?; - Ok((f, c)) - }) - .filter(|x| { - !x.as_ref() - .is_ok_and(|(_, c)| c.get("connection", "type").is_some_and(|t| t == "loopback")) - }) - .try_for_each(|x: anyhow::Result<(&String, Ini)>| { - let (f, c) = x?; - let interface_name = c - .get("connection", "interface-name") - .ok_or_else(|| anyhow!("No interface-name found in connection file: {}", f))?; - let connection_id = c - .get("connection", "id") - .ok_or_else(|| anyhow!("No connection id found in connection file: {}", f))?; - interfaces - .iter_mut() - .find(|x| x.logical_name == interface_name) - .ok_or_else(|| anyhow!("No matching interface found for connection file: {}", f))? - .connection_ids - .push(connection_id); - Ok(()) - }) + for (filename, content) in config { + let mut c = Ini::new(); + c.read(content.to_string()).map_err(|e| anyhow!(e))?; + + if c.get("connection", "type").is_some_and(|t| t == "loopback") { + continue; + } + + let interface_name = c + .get("connection", "interface-name") + .ok_or_else(|| anyhow!("No interface-name found in connection file: {}", filename))?; + let connection_id = c + .get("connection", "id") + .ok_or_else(|| anyhow!("No connection id found in connection file: {}", filename))?; + interfaces + .iter_mut() + .find(|x| x.logical_name == interface_name) + .ok_or_else(|| { + anyhow!( + "No matching interface found for connection file: {}", + filename + ) + })? + .connection_ids + .push(connection_id); + } + + Ok(()) } fn extract_interfaces(network_state: &NetworkState) -> Vec {