From 07f0ed3d7d17265993ee5388779ac9a333bf527e Mon Sep 17 00:00:00 2001 From: Atanas Dinov Date: Wed, 20 Mar 2024 19:43:17 +0200 Subject: [PATCH 1/5] Improve &str -> String conversions Signed-off-by: Atanas Dinov --- src/generate_conf.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index ce4fdf7..9511e0c 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -9,8 +9,8 @@ use nmstate::{InterfaceType, NetworkState}; use crate::types::{Host, Interface}; use crate::HOST_MAPPING_FILE; -/// NetworkConfig contains the generated configurations in the -/// following format: Vec<(config_file_name, config_content>) +/// `NetworkConfig` contains the generated configurations in the +/// following format: `Vec<(config_file_name, config_content>)` type NetworkConfig = Vec<(String, String)>; /// Generate network configurations from all YAML files in the `config_dir` @@ -30,7 +30,7 @@ pub(crate) fn generate(config_dir: &str, output_dir: &str) -> Result<(), anyhow: let hostname = extract_hostname(&path) .and_then(OsStr::to_str) .ok_or_else(|| anyhow!("Invalid file path"))? - .to_string(); + .to_owned(); let data = fs::read_to_string(&path).context("Reading network config")?; @@ -72,7 +72,7 @@ fn extract_interfaces(network_state: &NetworkState) -> Vec { .iter() .filter(|i| i.iface_type() != InterfaceType::Loopback) .map(|i| Interface { - logical_name: i.name().to_string(), + logical_name: i.name().to_owned(), mac_address: i.base_iface().mac_address.clone(), interface_type: i.iface_type().to_string(), }) From e7e1f218f8059216295eb8ed9263687afd89904c Mon Sep 17 00:00:00 2001 From: Atanas Dinov Date: Wed, 20 Mar 2024 19:43:54 +0200 Subject: [PATCH 2/5] Ensure config directory is not empty Signed-off-by: Atanas Dinov --- src/generate_conf.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index 9511e0c..27699c1 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -16,6 +16,10 @@ 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 under `output_dir`. pub(crate) fn generate(config_dir: &str, output_dir: &str) -> Result<(), anyhow::Error> { + if fs::read_dir(config_dir)?.count() == 0 { + return Err(anyhow!("Empty config directory")); + }; + for entry in fs::read_dir(config_dir)? { let entry = entry?; let path = entry.path(); @@ -166,6 +170,16 @@ mod tests { Ok(()) } + #[test] + fn generate_fails_due_to_empty_dir() { + fs::create_dir_all("empty").unwrap(); + + let error = generate("empty", "_out").unwrap_err(); + assert_eq!(error.to_string(), "Empty config directory"); + + fs::remove_dir_all("empty").unwrap(); + } + #[test] fn generate_fails_due_to_missing_path() { let error = generate("", "_out").unwrap_err(); From 0664034a39ea977e0a7963699f6af5873c9385e8 Mon Sep 17 00:00:00 2001 From: Atanas Dinov Date: Wed, 20 Mar 2024 19:59:47 +0200 Subject: [PATCH 3/5] Ensure Ethernet interfaces are assigned a MAC address Signed-off-by: Atanas Dinov --- src/generate_conf.rs | 66 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index 27699c1..7a3b1da 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -61,6 +61,8 @@ fn generate_config(data: String) -> Result<(Vec, NetworkConfig), anyh let network_state = NetworkState::new_from_yaml(&data)?; let interfaces = extract_interfaces(&network_state); + validate_interfaces(&interfaces)?; + let config = network_state .gen_conf()? .get("NetworkManager") @@ -83,6 +85,24 @@ fn extract_interfaces(network_state: &NetworkState) -> Vec { .collect() } +fn validate_interfaces(interfaces: &[Interface]) -> anyhow::Result<()> { + let interfaces: Vec = interfaces + .iter() + .filter(|i| i.interface_type == InterfaceType::Ethernet.to_string()) + .filter(|i| i.mac_address.is_none()) + .map(|i| i.logical_name.to_owned()) + .collect(); + + if !interfaces.is_empty() { + return Err(anyhow!( + "Detected Ethernet interfaces without a MAC address: {}", + interfaces.join(", ") + )); + }; + + Ok(()) +} + fn store_network_config( output_dir: &str, hostname: String, @@ -117,7 +137,9 @@ mod tests { use std::fs; use std::path::Path; - use crate::generate_conf::{extract_hostname, extract_interfaces, generate, generate_config}; + use crate::generate_conf::{ + extract_hostname, extract_interfaces, generate, generate_config, validate_interfaces, + }; use crate::types::{Host, Interface}; use crate::HOST_MAPPING_FILE; @@ -231,6 +253,48 @@ mod tests { Ok(()) } + #[test] + fn validate_interfaces_missing_mac_addresses() { + 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(), + }, + Interface { + logical_name: "eth1".to_string(), + mac_address: None, + interface_type: "ethernet".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(), + }, + Interface { + logical_name: "eth3".to_string(), + mac_address: None, + interface_type: "ethernet".to_string(), + }, + Interface { + logical_name: "eth3.1365".to_string(), + mac_address: None, + interface_type: "vlan".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(), + }, + ]; + + let error = validate_interfaces(&interfaces).unwrap_err(); + assert_eq!( + error.to_string(), + "Detected Ethernet interfaces without a MAC address: eth1, eth3" + ) + } + #[test] fn extract_host_name() { assert_eq!(extract_hostname("".as_ref()), None); From b93c87e2f75670c4952cdccaf48bc27fe6652e8b Mon Sep 17 00:00:00 2001 From: Atanas Dinov Date: Wed, 20 Mar 2024 20:10:41 +0200 Subject: [PATCH 4/5] Ensure at least a single Ethernet interface is provided Signed-off-by: Atanas Dinov --- src/generate_conf.rs | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index 7a3b1da..e102c4d 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -86,17 +86,25 @@ fn extract_interfaces(network_state: &NetworkState) -> Vec { } fn validate_interfaces(interfaces: &[Interface]) -> anyhow::Result<()> { - let interfaces: Vec = interfaces + let ethernet_interfaces: Vec<&Interface> = interfaces .iter() .filter(|i| i.interface_type == InterfaceType::Ethernet.to_string()) + .collect(); + + if ethernet_interfaces.is_empty() { + return Err(anyhow!("No Ethernet interfaces were provided")); + } + + let ethernet_interfaces: Vec = ethernet_interfaces + .iter() .filter(|i| i.mac_address.is_none()) .map(|i| i.logical_name.to_owned()) .collect(); - if !interfaces.is_empty() { + if !ethernet_interfaces.is_empty() { return Err(anyhow!( "Detected Ethernet interfaces without a MAC address: {}", - interfaces.join(", ") + ethernet_interfaces.join(", ") )); }; @@ -227,7 +235,7 @@ mod tests { mac-address: FE:C4:05:42:8B:AB - name: lo type: loopback - mac-address: 00:00:00:00:00:00 + mac-address: 00:00:00:00:00:00 "#, )?; @@ -253,6 +261,25 @@ mod tests { Ok(()) } + #[test] + fn validate_interfaces_missing_ethernet_interfaces() { + let interfaces = vec![ + Interface { + logical_name: "eth3.1365".to_string(), + mac_address: None, + interface_type: "vlan".to_string(), + }, + Interface { + logical_name: "bond0".to_string(), + mac_address: None, + interface_type: "bond".to_string(), + }, + ]; + + let error = validate_interfaces(&interfaces).unwrap_err(); + assert_eq!(error.to_string(), "No Ethernet interfaces were provided") + } + #[test] fn validate_interfaces_missing_mac_addresses() { let interfaces = vec![ From 0e8fad197bd7a8382565be3e2712f9bee4fe70f5 Mon Sep 17 00:00:00 2001 From: Atanas Dinov Date: Wed, 20 Mar 2024 20:13:22 +0200 Subject: [PATCH 5/5] Add successful validation scenario Signed-off-by: Atanas Dinov --- src/generate_conf.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/generate_conf.rs b/src/generate_conf.rs index e102c4d..076166f 100644 --- a/src/generate_conf.rs +++ b/src/generate_conf.rs @@ -322,6 +322,29 @@ mod tests { ) } + #[test] + fn validate_interfaces_successfully() { + 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(), + }, + Interface { + logical_name: "eth0.1365".to_string(), + mac_address: None, + interface_type: "vlan".to_string(), + }, + Interface { + logical_name: "bond0".to_string(), + mac_address: None, + interface_type: "bond".to_string(), + }, + ]; + + assert!(validate_interfaces(&interfaces).is_ok()) + } + #[test] fn extract_host_name() { assert_eq!(extract_hostname("".as_ref()), None);