From c33807f8fc11145c0f15751fa0e443f06d2a0b12 Mon Sep 17 00:00:00 2001 From: BoolPurist Date: Sat, 10 Jun 2023 18:17:50 +0200 Subject: [PATCH] Replaced error logs with error propagation Replaced pancis to error propagation in LDAP modules Converted panics to error propagation in slurm modules --- src/dir.rs | 91 ++- src/ldap.rs | 528 ++++++++---------- src/lib.rs | 12 +- src/slurm.rs | 167 +++--- src/util.rs | 24 +- src/util/result_accumalator.rs | 66 +++ ...cumalator__testing__resolve_to_errors.snap | 8 + 7 files changed, 498 insertions(+), 398 deletions(-) create mode 100644 src/util/result_accumalator.rs create mode 100644 src/util/snapshots/usermgmt__util__result_accumalator__testing__resolve_to_errors.snap diff --git a/src/dir.rs b/src/dir.rs index 1a9ec5a..2342f22 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -1,5 +1,6 @@ +use crate::util::ResultAccumalator; /// Module for directory management -use log::{debug, error, info, warn}; +use log::{debug, info, warn}; use crate::config::MgmtConfig; use crate::prelude::AppResult; @@ -88,22 +89,35 @@ fn handle_compute_nodes( } } - if mkdir_exit_codes.iter().all(|&x| x == 0) { - if owner_exit_codes - .iter() - .all(|x| x.as_ref().is_ok_and(|code| *code == 0)) - { - if quota_exit_codes.iter().all(|&x| x == 0) { - info!("Successfully created directories on compute nodes."); - } else if can_set_quota { - error!("Not all compute nodes returned exit code 0 during quota setup!"); - } - } else { - error!("Not all compute nodes returned exit code 0 during ownership change!"); - } - } else { - error!("Not all compute nodes returned exit code 0 during directory creation!"); - } + let mut errors_from_codes = + ResultAccumalator::new("Failed at creating directories on compute nodes.".to_owned()); + + let all_exit_codes_are_zero = mkdir_exit_codes.iter().all(|&x| x == 0); + + errors_from_codes.add_err_if_false( + all_exit_codes_are_zero, + "Not all compute nodes returned exit code 0 during directory creation!".to_owned(), + ); + + let all_owner_exit_codes_are_zero = owner_exit_codes + .iter() + .all(|x| x.as_ref().is_ok_and(|code| *code == 0)); + + errors_from_codes.add_err_if_false( + all_owner_exit_codes_are_zero, + "Not all compute nodes returned exit code 0 during ownership change!".to_owned(), + ); + + let all_quota_exit_codes_are_zero = quota_exit_codes.iter().all(|&x| x == 0); + + errors_from_codes.add_err_if_false( + all_quota_exit_codes_are_zero, + "Not all compute nodes returned exit code 0 during quota setup!".to_owned(), + ); + + AppResult::from(errors_from_codes)?; + + info!("Successfully created directories on compute nodes."); Ok(()) } @@ -142,7 +156,10 @@ fn handle_nfs(entity: &Entity, config: &MgmtConfig, credentials: &SshCredential) let directory = format!("{}/{}/{}", config.nfs_root_dir, group_dir, entity.username); let dir_exit_code = make_directory(&sess, &directory)?; - if dir_exit_code == 0 { + let mut detected_errors = + ResultAccumalator::new("Errors in creating directories for NFS occured".to_owned()); + let no_error_make_dir = dir_exit_code == 0; + if no_error_make_dir { // Give ownership to user let owner_exit_code = change_ownership( &sess, @@ -151,12 +168,16 @@ fn handle_nfs(entity: &Entity, config: &MgmtConfig, credentials: &SshCredential) &entity.group.to_string(), )?; if owner_exit_code != 0 { - error!("NFS host did not return with exit code 0 during ownership change!"); + detected_errors.add_err( + "NFS host did not return with exit code 0 during ownership change!".to_owned(), + ); } else { info!("Successfully created user directory on NFS host."); } } else { - error!("NFS host did not return with exit code 0 during directory creation!"); + detected_errors.add_err( + "NFS host did not return with exit code 0 during directory creation!".to_owned(), + ); } // Set user quota @@ -168,11 +189,15 @@ fn handle_nfs(entity: &Entity, config: &MgmtConfig, credentials: &SshCredential) &config.quota_nfs_hardlimit, &config.nfs_filesystem, )?; - if quota_exit_code != 0 { - error!("NFS host did not return with exit code 0 during quota setup!") - } + + detected_errors.add_err_if_false( + quota_exit_code == 0, + "NFS host did not return with exit code 0 during quota setup!".to_owned(), + ) } + AppResult::from(detected_errors)?; + Ok(()) } @@ -207,6 +232,9 @@ fn handle_home(entity: &Entity, config: &MgmtConfig, credentials: &SshCredential make_directory(&sess, &directory) }?; + let mut detected_errors = + ResultAccumalator::new("Errors in creating the home folder of user occured".to_owned()); + if dir_exit_code == 0 { // Give ownership to user let owner_exit_code = change_ownership( @@ -216,12 +244,16 @@ fn handle_home(entity: &Entity, config: &MgmtConfig, credentials: &SshCredential &entity.group.to_string(), )?; if owner_exit_code != 0 { - error!("Home host did not return with exit code 0 during ownership change!"); + detected_errors.add_err( + "Home host did not return with exit code 0 during ownership change!".to_owned(), + ); } else { info!("Successfully created user home directory."); } } else { - error!("Home host did not return with exit code 0 during directory creation!"); + detected_errors.add_err( + "Home host did not return with exit code 0 during directory creation!".to_owned(), + ); } // Set user quota @@ -233,11 +265,14 @@ fn handle_home(entity: &Entity, config: &MgmtConfig, credentials: &SshCredential &config.quota_home_hardlimit, &config.home_filesystem, )?; - if quota_exit_code != 0 { - error!("Home host did not return with exit code 0 during quota setup!") - } + detected_errors.add_err_if_false( + quota_exit_code != 0, + "Home host did not return with exit code 0 during quota setup!".to_owned(), + ); } + AppResult::from(detected_errors)?; + Ok(()) } diff --git a/src/ldap.rs b/src/ldap.rs index fdafebd..ecfd363 100644 --- a/src/ldap.rs +++ b/src/ldap.rs @@ -14,7 +14,7 @@ use crate::{Entity, MgmtConfig, Modifiable}; /// LDAP operations using the ldap3 lib use ldap3::controls::{MakeCritical, RelaxRules}; use ldap3::{LdapConn, LdapError, LdapResult, Mod, Scope, SearchEntry}; -use log::{debug, error, info, warn}; +use log::{debug, info, warn}; use maplit::hashset; use std::collections::HashSet; @@ -24,217 +24,197 @@ fn make_ldap_connection(server: &str) -> Result { LdapConn::new(server) } -/// TODO: Bubble up error instead of just logging it -pub fn add_ldap_user(entity: &Entity, config: &MgmtConfig) { +pub fn add_ldap_user(entity: &Entity, config: &MgmtConfig) -> AppResult { if entity.publickey.is_empty() { warn!("No publickey supplied! Don't forget to manually add it in LDAP (or via the modify operation) afterwards.") } let ldap_config = LDAPConfig::new(config, &None, &None); - if username_exists(&entity.username, &ldap_config) { + let exitence_of_username = username_exists(&entity.username, &ldap_config)?; + if exitence_of_username { warn!( "User {} already exists in LDAP. Skipping LDAP user creation.", &entity.username ); - return; + return Ok(()); } - let uid_result = find_next_available_uid(&ldap_config, entity.group.clone()); - let uid_number = match uid_result { - Ok(r) => r, - Err(e) => { - error!("No users found or LDAP query failed. Unable to assign uid. Aborting..."); - error!("{}", e); - return; - } - }; + let uid_number = find_next_available_uid(&ldap_config, entity.group.clone()) + .context("No users found or LDAP query failed. Unable to assign uid. Aborting...")?; + + let mut ldap_connection = make_ldap_connection(&ldap_config.ldap_server)?; + + ldap_connection.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass)?; + debug!("LDAP connection established to {}", ldap_config.bind()); + + add_to_ldap_db(entity, uid_number, ldap_connection, config, &ldap_config)?; + + info!("Added LDAP user {}", entity.username); + return Ok(()); + + fn add_to_ldap_db( + entity: &Entity, + uid_number: u32, + mut ldap_connection: LdapConn, + config: &MgmtConfig, + ldap_config: &LDAPConfig, + ) -> AppResult { + let un = &*entity.username.to_owned(); + let gid = &*format!("{}", entity.gid); + let uid = &*format!("{}", uid_number); + let ln = &*entity.lastname.to_owned(); + let gn = &*entity.firstname.to_owned(); + let mail = &*entity.mail.to_owned(); + let def_qos = &*entity.default_qos.to_owned(); + let home = &*format!("/home/{}", entity.username); + let qos = entity.qos.to_owned(); + let pubkey = &*entity.publickey.to_owned(); + + let result_form_adding = ldap_connection.add( + &format!("uid={},{}", entity.username, ldap_config.base()), + vec![ + ("cn", hashset! {un}), + ( + "objectClass", + hashset_from_vec_str(&config.objectclass_common).to_owned(), + ), + ("gidNumber", hashset! {gid}), + ("uidNumber", hashset! {uid}), + ("uid", hashset! {un}), + ("sn", hashset! {ln}), + ("givenName", hashset! {gn}), + ("mail", hashset! {mail}), + ("slurmDefaultQos", hashset! {def_qos}), + ("homeDirectory", hashset! {home}), + ("slurmQos", hashset_from_vec_str(&qos).to_owned()), + ("sshPublicKey", hashset! {pubkey}), + ("loginShell", hashset! {config.login_shell.as_str()}), + ], + ); - match make_ldap_connection(&ldap_config.ldap_server) { - Ok(mut ldap) => { - match ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass) { - Ok(_bind) => debug!("LDAP connection established to {}", ldap_config.bind()), - Err(e) => error!("{}", e), - } - let un = &*entity.username.to_owned(); - let gid = &*format!("{}", entity.gid); - let uid = &*format!("{}", uid_number); - let ln = &*entity.lastname.to_owned(); - let gn = &*entity.firstname.to_owned(); - let mail = &*entity.mail.to_owned(); - let def_qos = &*entity.default_qos.to_owned(); - let home = &*format!("/home/{}", entity.username); - let qos = entity.qos.to_owned(); - let pubkey = &*entity.publickey.to_owned(); - - let ldap_result = ldap.add( - &format!("uid={},{}", entity.username, ldap_config.base()), - vec![ - ("cn", hashset! {un}), - ( - "objectClass", - hashset_from_vec_str(&config.objectclass_common).to_owned(), - ), - ("gidNumber", hashset! {gid}), - ("uidNumber", hashset! {uid}), - ("uid", hashset! {un}), - ("sn", hashset! {ln}), - ("givenName", hashset! {gn}), - ("mail", hashset! {mail}), - ("slurmDefaultQos", hashset! {def_qos}), - ("homeDirectory", hashset! {home}), - ("slurmQos", hashset_from_vec_str(&qos).to_owned()), - ("sshPublicKey", hashset! {pubkey}), - ("loginShell", hashset! {config.login_shell.as_str()}), - ], - ); - - match ldap_is_success(ldap_result) { - Ok(_) => info!("Added LDAP user {}", entity.username), - Err(error) => error!("Unable to create LDAP user! {error}"), - } - } - Err(e) => error!("{}", e), + ldap_is_success(result_form_adding).context("Unable to create LDAP user!")?; + Ok(()) } - - debug!("add_ldap_user done"); } -/// TODO: Bubble up error instead of just logging it -pub fn delete_ldap_user(username: &str, config: &MgmtConfig) { +pub fn delete_ldap_user(username: &str, config: &MgmtConfig) -> AppResult { let ldap_config = LDAPConfig::new(config, &None, &None); // get dn for uid - match find_dn_by_uid(username, &ldap_config) { - Some(dn) => { - match make_ldap_connection(&ldap_config.ldap_server) { - Ok(mut ldap) => { - match ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass) { - Ok(_bind) => { - debug!("LDAP connection established to {}", ldap_config.bind()) - } - Err(e) => error!("{}", e), - } - // delete user by dn - match ldap_is_success(ldap.delete(&dn)) { - Ok(_) => info!("Successfully deleted DN {}", dn), - Err(e) => error!("User deletion in LDAP failed! {}", e), - } - } - Err(e) => error!("{}", e), - } + let dn = find_dn_by_uid(username, &ldap_config) + .with_context(|| format!("No DN found for username {}!", username))?; + let mut ldap = make_ldap_connection(&ldap_config.ldap_server)?; + debug!("LDAP connection established to {}", ldap_config.bind()); + + let _ = ldap + .simple_bind(ldap_config.bind(), &ldap_config.ldap_pass) + .with_context(|| { + format!( + "Failed to establish ldap connection via the bind {}", + ldap_config.bind() + ) + })?; + + match &dn { + Some(dn_to_delete) => { + ldap_is_success(ldap.delete(dn_to_delete)).context("User deletion in LDAP failed!")?; + info!("Successfully deleted DN {}", dn_to_delete); + } + None => { + warn!("No dn found to delete under the username {}", username); } - None => error!("No DN found for username {}!", username), } - debug!("delete_ldap_user done"); + + Ok(()) } -/// TODO: Bubble up error instead of just logging it pub fn modify_ldap_user(modifiable: &Modifiable, config: &MgmtConfig) -> AppResult { let ldap_config = LDAPConfig::new(config, &None, &None); - // get dn for uid - match find_dn_by_uid(&modifiable.username, &ldap_config) { - Some(dn) => { - match make_ldap_connection(&ldap_config.ldap_server) { - Ok(mut ldap) => { - match ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass) { - Ok(_bind) => { - debug!("LDAP connection established to {}", ldap_config.bind()) - } - Err(e) => error!("{}", e), - } - // Prepare replace operation - let old_qos = find_qos_by_uid( - &modifiable.username, - config, - ldap_config.username(), - &ldap_config.ldap_pass, - )?; - let mod_vec = make_modification_vec(modifiable, &old_qos); - - // Replace userPassword at given dn - let res = ldap - .with_controls(RelaxRules.critical()) - .modify(&dn, mod_vec); - - match ldap_is_success(res) { - Ok(_) => { - info!("Successfully modified user {} in LDAP", modifiable.username) - } - Err(e) => info!("User modification in LDAP failed! {}", e), - } - } - Err(e) => error!("{}", e), - } - } - None => error!( - "No DN found for username {}! Unable to modify user.", - modifiable.username - ), - } - debug!("modify_ldap_user done"); + let dn = find_dn_by_uid(&modifiable.username, &ldap_config) + .with_context(|| { + format!( + "No DN found for username {}! Unable to modify user.", + modifiable.username + ) + })? + .ok_or(anyhow!("No dn found for uid"))?; + + let mut ldap = make_ldap_connection(&ldap_config.ldap_server)?; + ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass)?; + debug!("LDAP connection established to {}", ldap_config.bind()); + + // Prepare replace operation + let old_qos = find_qos_by_uid( + &modifiable.username, + config, + ldap_config.username(), + &ldap_config.ldap_pass, + )?; + let mod_vec = make_modification_vec(modifiable, &old_qos); + + // Replace userPassword at given dn + let res = ldap + .with_controls(RelaxRules.critical()) + .modify(&dn, mod_vec); + + ldap_is_success(res).context("User modification in LDAP failed!")?; + info!("Successfully modified user {} in LDAP", modifiable.username); Ok(()) } /// List all LDAP users and some attributes /// -/// TODO: improve output format in readability. /// It currently outputs all values in line separated by commas. -/// TODO: Bubble up error instead of just logging it pub fn list_ldap_users(config: &MgmtConfig, simple_output_ldap: bool) -> AppResult { let ldap_config = LdapReadonlyConfig::new(config); // Establish LDAP connection and bind - match make_ldap_connection(ldap_config.ldap_server()) { - Ok(mut ldap) => { - match ldap.simple_bind(ldap_config.bind(), ldap_config.ldap_pass()) { - Ok(_bind) => { - debug!( - "LDAP connection established to {}. Will search under {}", - ldap_config.bind(), - ldap_config.base() - ); - let attrs = { - // Make sure the keys are sorted alphabetic - // This way the order fields in the final output deterministic - let mut to_sort = vec![ - "uid", - "uidNumber", - "givenName", - "sn", - "mail", - "slurmDefaultQos", - "slurmQos", - ]; - to_sort.sort(); - to_sort - }; - // Search for all entities under base dn - let search_result = ldap.search( - ldap_config.base(), - Scope::OneLevel, - "(objectclass=*)", - attrs.clone(), - ); - match search_result { - // Parse search results and print - Ok(result) => { - let output = if simple_output_ldap { - text_list_output::ldap_simple_output(&attrs, &result) - } else { - text_list_output::ldap_search_to_pretty_table(&attrs, &result) - }; - println!("{}", output); - Ok(()) - } - Err(e) => Err(anyhow::Error::new(e).context("Error during LDAP search!")), - } - } - Err(e) => Err(anyhow::Error::new(e).context("Error during LDAP binding!")), - } - } - Err(e) => Err(anyhow::Error::new(e).context("Error while connecting via LDAP !")), - } + let mut ldap = make_ldap_connection(ldap_config.ldap_server()) + .context("Error while connecting via LDAP !")?; + + ldap.simple_bind(ldap_config.bind(), ldap_config.ldap_pass()) + .context("Error during LDAP binding!")?; + + debug!( + "LDAP connection established to {}. Will search under {}", + ldap_config.bind(), + ldap_config.base() + ); + let attrs = { + // Make sure the keys are sorted alphabetic + // This way the order fields in the final output deterministic + let mut to_sort = vec![ + "uid", + "uidNumber", + "givenName", + "sn", + "mail", + "slurmDefaultQos", + "slurmQos", + ]; + to_sort.sort(); + to_sort + }; + + // Search for all entities under base dn + let search_result = ldap + .search( + ldap_config.base(), + Scope::OneLevel, + "(objectclass=*)", + attrs.clone(), + ) + .context("Error during LDAP search!")?; + + let output = if simple_output_ldap { + text_list_output::ldap_simple_output(&attrs, &search_result) + } else { + text_list_output::ldap_search_to_pretty_table(&attrs, &search_result) + }; + + println!("{}", output); + Ok(()) } fn make_modification_vec<'a>( @@ -287,86 +267,70 @@ fn make_modification_vec<'a>( } /// Do a LDAP search to determine the next available uid -fn find_next_available_uid(ldap_config: &LDAPConfig, group: crate::Group) -> Result { - match make_ldap_connection(&ldap_config.ldap_server) { - Ok(mut ldap) => { - debug!( - "Binding with dn: {}, pw: {}", - ldap_config.bind(), - ldap_config.ldap_pass - ); - match ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass) { - Ok(r) => debug!( - "find_next_available_uid: LDAP connection established to {}, {}", - ldap_config.bind(), - r - ), - Err(e) => error!("{}", e), - } - debug!("Search under {}", ldap_config.base()); - // Search for all uidNumbers under base dn - let search_result = ldap.search( - ldap_config.base(), - Scope::OneLevel, - "(objectclass=*)", - vec!["uidNumber"], - ); - match search_result { - // Parse search results into ints - Ok(result) => { - let mut uids: Vec = Vec::new(); - for elem in result.0.iter() { - let search_result = SearchEntry::construct(elem.to_owned()); - debug!("UID: {:?}", SearchEntry::construct(elem.to_owned())); - let uid = &search_result.attrs["uidNumber"][0].parse::().unwrap(); - uids.push(*uid); - } - // Find max uid and add 1 - get_new_uid(&uids, group) - } - Err(e) => Err(format!("Error during uid search! {}", e)), - } - } - Err(e) => Err(format!("Error during uid search! {}", e)), +fn find_next_available_uid(ldap_config: &LDAPConfig, group: crate::Group) -> AppResult { + let mut ldap = + make_ldap_connection(&ldap_config.ldap_server).context("Error during uid search!")?; + + debug!( + "Binding with dn: {}, pw: {}", + ldap_config.bind(), + ldap_config.ldap_pass + ); + + let bind = ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass)?; + debug!( + "find_next_available_uid: LDAP connection established to {}, {}", + ldap_config.bind(), + bind + ); + + debug!("Search under {}", ldap_config.base()); + + // Search for all uidNumbers under base dn + let search_result = ldap + .search( + ldap_config.base(), + Scope::OneLevel, + "(objectclass=*)", + vec!["uidNumber"], + ) + .context("Error during uid search!")?; + + let mut uids: Vec = Vec::new(); + for elem in search_result.0.iter() { + let search_result = SearchEntry::construct(elem.to_owned()); + debug!("UID: {:?}", SearchEntry::construct(elem.to_owned())); + let uid = &search_result.attrs["uidNumber"][0].parse::().unwrap(); + uids.push(*uid); } + + get_new_uid(&uids, group) } /// Search for a specific uid and return the corresponding dn. -fn find_dn_by_uid(username: &str, ldap_config: &LDAPConfig) -> Option { - let mut dn_result = None; - match make_ldap_connection(&ldap_config.ldap_server) { - Ok(mut ldap) => { - match ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass) { - Ok(_bind) => debug!("LDAP connection established to {}", ldap_config.bind()), - Err(e) => error!("{}", e), - } - - // Search for all uids under base dn and return dn of user - let search = ldap.search( - ldap_config.base(), - Scope::OneLevel, - &format!("(uid={username})"), - vec!["dn"], - ); - - match search { - Ok(result) => { - // Only deal with the first element retrieved from search - match result.0.into_iter().next() { - Some(entry) => { - let sr = SearchEntry::construct(entry); - debug!("SR for deletion: {:?}", sr); - dn_result = Some(sr.dn); - } - None => error!("No LDAP entry found for user {}", username), - } - } - Err(e) => error!("{}", e), - } - } - Err(e) => error!("{}", e), - } - dn_result +fn find_dn_by_uid(username: &str, ldap_config: &LDAPConfig) -> AppResult> { + let mut ldap = make_ldap_connection(&ldap_config.ldap_server)?; + ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass)?; + debug!("LDAP connection established to {}", ldap_config.bind()); + + // Search for all uids under base dn and return dn of user + let search = ldap.search( + ldap_config.base(), + Scope::OneLevel, + &format!("(uid={username})"), + vec!["dn"], + )?; + + let entry = search + .0 + .into_iter() + .next() + .with_context(|| format!("No LDAP entry found for user {}", username))?; + + let sr = SearchEntry::construct(entry); + debug!("SR for deletion: {:?}", sr); + + Ok(Some(sr.dn)) } /// Search for a specific uid and return the corresponding qos. @@ -387,10 +351,9 @@ fn find_qos_by_uid( let mut ldap_connection = make_ldap_connection(ldap_server) .with_context(|| format!("Connection to {} failed", ldap_server))?; - match ldap_connection.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass) { - Ok(_bind) => debug!("LDAP connection established to {}", ldap_config.bind()), - Err(e) => error!("{}", e), - } + ldap_connection.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass)?; + + debug!("LDAP connection established to {}", ldap_config.bind()); // Search for all uid under base dn and return dn of user let search = ldap_connection @@ -422,41 +385,28 @@ fn find_qos_by_uid( /// Check if username already exists in ldap. /// Must be an exact match on the uid attribute. /// TODO: Bubble up error instead of just logging it -fn username_exists(username: &String, ldap_config: &LDAPConfig) -> bool { +fn username_exists(username: &String, ldap_config: &LDAPConfig) -> AppResult { let mut username_exists = false; - match make_ldap_connection(&ldap_config.ldap_server) { - Ok(mut ldap) => { - match ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass) { - Ok(_bind) => debug!("LDAP connection established to {}", ldap_config.bind()), - Err(e) => error!("{}", e), - } - - // Search for all uid under base dn and return dn of user - let search = ldap.search( - ldap_config.base(), - Scope::OneLevel, - &format!("(uid={username})"), - vec!["dn"], - ); - - match search { - Ok(result) => { - // Only deal with the first element retrieved from search - match result.0.into_iter().next() { - Some(entry) => { - // User found. Good. - debug!("Found user: {:?}", SearchEntry::construct(entry)); - username_exists = true - } - None => debug!("No LDAP entry found for user {}", username), - } - } - Err(e) => error!("{}", e), - } + let mut ldap = make_ldap_connection(&ldap_config.ldap_server)?; + ldap.simple_bind(ldap_config.bind(), &ldap_config.ldap_pass)?; + debug!("LDAP connection established to {}", ldap_config.bind()); + + // Search for all uid under base dn and return dn of user + let search_result = ldap.search( + ldap_config.base(), + Scope::OneLevel, + &format!("(uid={username})"), + vec!["dn"], + )?; + match search_result.0.into_iter().next() { + Some(entry) => { + // User found. Good. + debug!("Found user: {:?}", SearchEntry::construct(entry)); + username_exists = true } - Err(e) => error!("{}", e), + None => debug!("No LDAP entry found for user {}", username), } - username_exists + Ok(username_exists) } /// If ok is returned then ldap operation happened with zero error code, LDAP_SUCCESS diff --git a/src/lib.rs b/src/lib.rs index 3cf1ec8..9e6a4c4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -277,7 +277,7 @@ fn add_user(to_add: &UserToAdd, on_which_sys: &OnWhichSystem, config: &MgmtConfi let ssh_credentials = SshCredential::new(config); if on_which_sys.ldap() { - add_ldap_user(&entity, config); + add_ldap_user(&entity, config)?; } if on_which_sys.slurm() { @@ -307,16 +307,14 @@ fn delete_user(user: &str, on_which_sys: &OnWhichSystem, config: &MgmtConfig) -> let credentials = SshCredential::new(config); if on_which_sys.ldap() { - delete_ldap_user(user, config); + delete_ldap_user(user, config)?; } if on_which_sys.slurm() { if config.run_slurm_remote { - // Execute sacctmgr commands via SSH session slurm::remote::delete_slurm_user(user, config, &credentials)?; } else { - // Call sacctmgr binary directly via subprocess - slurm::local::delete_slurm_user(user, &config.sacctmgr_path); + slurm::local::delete_slurm_user(user, &config.sacctmgr_path)?; } } @@ -373,7 +371,7 @@ fn modify_user( slurm::remote::modify_slurm_user(&data, config, &credential)?; } else { // Call sacctmgr binary directly via subprocess - slurm::local::modify_slurm_user(&data, &sacctmgr_path); + slurm::local::modify_slurm_user(&data, &sacctmgr_path)?; } } @@ -396,7 +394,7 @@ fn list_users( if config.run_slurm_remote { slurm::remote::list_users(config, &credentials)?; } else { - slurm::local::list_users(&config.sacctmgr_path); + slurm::local::list_users(&config.sacctmgr_path)?; } } diff --git a/src/slurm.rs b/src/slurm.rs index 38dd171..7054f12 100644 --- a/src/slurm.rs +++ b/src/slurm.rs @@ -1,7 +1,15 @@ +use anyhow::{anyhow, Context}; +use log::info; + +use crate::{ + prelude::AppResult, + ssh::{self, SshSession}, +}; + pub mod local { use std::process::Command; - use log::{debug, error, info, warn}; + use log::{debug, info, warn}; use crate::prelude::*; use crate::{Entity, Modifiable}; @@ -13,6 +21,8 @@ pub mod local { /// /// - If no local sacctmgr binary can be found /// - If the execution of the local sacctmgr binary returns with non zero error code. + /// - if modifying the quality of service fails + /// pub fn add_slurm_user(entity: &Entity, sacctmgr_path: &str) -> AppResult { let account_spec = format!("Account={}", entity.group); let cmd = &[ @@ -48,23 +58,23 @@ pub mod local { debug!("Modifying user qos"); // Note: The order of execution is important here! // Slurm expects the user to have QOS, before it can set the default QOS - modify_qos(entity, sacctmgr_path, false); - modify_qos(entity, sacctmgr_path, true); + modify_qos(entity, sacctmgr_path, false)?; + modify_qos(entity, sacctmgr_path, true)?; Ok(()) } - /// TODO: Bubble up error instead of just logging it - pub fn delete_slurm_user(user: &str, sacctmgr_path: &str) { + /// Calls sacctmgr binary directly via subprocess + pub fn delete_slurm_user(user: &str, sacctmgr_path: &str) -> AppResult { let output = Command::new(sacctmgr_path) .arg("delete") .arg("user") .arg(user) .arg("--immediate") .output() - .expect( + .context( "Unable to execute sacctmgr command. Is the path specified in your config correct?", - ); + )?; debug!( "delete_slurm_user: {}", @@ -80,14 +90,15 @@ pub mod local { warn!("sacctmgr stdout: {}", out); } let err = String::from_utf8_lossy(&output.stderr); - if err.len() > 0 { - error!("sacctmgr stderr: {}", err); + if err.is_empty() { + bail!("sacctmgr stderr: {}", err); } } + + Ok(()) } - /// TODO: Bubble up error instead of just logging it - pub fn modify_slurm_user(modifiable: &Modifiable, sacctmgr_path: &str) { + pub fn modify_slurm_user(modifiable: &Modifiable, sacctmgr_path: &str) -> AppResult { debug!("Start modifying user qos"); match &modifiable.default_qos { Some(m) => { @@ -100,7 +111,7 @@ pub mod local { "Modifying default QOS for user {} in Slurm to {}.", modifiable.username, m ); - modify_qos(&entity, sacctmgr_path, true); + modify_qos(&entity, sacctmgr_path, true)?; } None => info!( "Did not modify default QOS for user {} in Slurm, since nothing was specified to modify.", @@ -114,31 +125,34 @@ pub mod local { qos: modifiable.qos.clone(), ..Default::default() }; - modify_qos(&entity, sacctmgr_path, false) + modify_qos(&entity, sacctmgr_path, false)?; } else { info!( "Did not modify QOS for user {} in Slurm, since nothing was specified to modify.", modifiable.username ); } + + Ok(()) } - /// TODO: Bubble up error instead of just logging it - pub fn list_users(sacctmgr_path: &str) { + /// Lists all user in slurm database on the local machine + pub fn list_users(sacctmgr_path: &str) -> AppResult { let output = Command::new(sacctmgr_path) .arg("show") .arg("assoc") .arg("format=User%30,Account,DefaultQOS,QOS%80") .output() - .expect( + .context( "Unable to execute sacctmgr command. Is the path specified in your config correct?", - ); + )?; // sacctmgr show assoc format=cluster,account,qos println!("{}", String::from_utf8_lossy(&output.stdout)); + Ok(()) } - /// TODO: Bubble up error instead of just logging it - fn modify_qos(entity: &Entity, sacctmgr_path: &str, default_qos: bool) { + /// Modifies the quality of use in database on a local machine + fn modify_qos(entity: &Entity, sacctmgr_path: &str, default_qos: bool) -> AppResult { let mut qos_str: String = "defaultQos=".to_owned(); if default_qos { qos_str += &entity.default_qos; @@ -155,9 +169,9 @@ pub mod local { .arg(qos_str) .arg("--immediate") .output() - .expect( + .context( "Unable to execute sacctmgr command. Is the path specified in your config correct?", - ); + )?; debug!("modify_qos: {}", String::from_utf8_lossy(&output.stdout)); @@ -170,23 +184,25 @@ pub mod local { warn!("sacctmgr stdout: {}", out); } let err = String::from_utf8_lossy(&output.stderr); - if err.len() > 0 { - error!("sacctmgr stderr: {}", err); + if err.is_empty() { + bail!("sacctmgr stderr: {}", err); } } + + Ok(()) } } pub mod remote { - use log::{debug, error, info}; + use log::{debug, info}; use crate::config::MgmtConfig; use crate::prelude::AppResult; - use crate::ssh::{self, SshCredential, SshSession}; + use crate::ssh::{SshCredential, SshSession}; use crate::{Entity, Modifiable}; - /// TODO: Bubble up error instead of just logging it + /// Creates a user in slurm database on a remote machine over ssh pub fn add_slurm_user( entity: &Entity, config: &MgmtConfig, @@ -200,18 +216,22 @@ pub mod remote { "{} add user {} Account={} --immediate", config.sacctmgr_path, entity.username, entity.group ); - let exit_code = ssh::run_remote_command(&session, &cmd)?; - - match exit_code { - 0 => info!( - "Successfully created Slurm user {}:{}.", - entity.username, entity.group - ), - _ => error!( - "Failed to create Slurm user. Command '{}' did not exit with 0.", - cmd - ), - }; + super::run_remote_report_slurm_cmd( + &session, + &cmd, + || { + format!( + "Successfully created Slurm user {}:{}.", + entity.username, entity.group + ) + }, + || { + format!( + "Failed to create Slurm user. Command '{}' did not exit with 0.", + cmd + ) + }, + )?; debug!("Modifying user qos"); // Note: The order of execution is important here! @@ -222,31 +242,33 @@ pub mod remote { Ok(()) } - /// TODO: Bubble up error instead of just logging it + /// Deletes a user in a slurm database via SSH session on a remote machine pub fn delete_slurm_user( user: &str, config: &MgmtConfig, credentials: &SshCredential, ) -> AppResult { // Connect to the SSH server and authenticate - info!("Connecting to {}", config.head_node); let sess = SshSession::from_head_node(config, credentials); let cmd = format!("{} delete user {} --immediate", config.sacctmgr_path, user); - let exit_code = ssh::run_remote_command(&sess, &cmd)?; - - match exit_code { - 0 => info!("Successfully deleted Slurm user {}.", user), - _ => error!( - "Failed to delete Slurm user. Command '{}' did not exit with 0.", - cmd - ), - }; + super::run_remote_report_slurm_cmd( + &sess, + &cmd, + || format!("Successfully deleted Slurm user {}.", user), + || { + format!( + "Failed to delete Slurm user. Command '{}' did not exit with 0.", + cmd + ) + }, + )?; Ok(()) } - /// TODO: Bubble up error instead of just logging it + /// Modifies a user in a slurm database via SSH session on a remote machine + /// It currently only modifies the quality of services of a user ! pub fn modify_slurm_user( modifiable: &Modifiable, config: &MgmtConfig, @@ -292,7 +314,7 @@ pub mod remote { Ok(()) } - /// TODO: Bubble up error instead of just logging it + /// Lists all users in slurm database on a remote machine pub fn list_users(config: &MgmtConfig, credentials: &SshCredential) -> AppResult { let cmd = "sacctmgr show assoc format=User%30,Account,DefaultQOS,QOS%80"; @@ -304,7 +326,6 @@ pub mod remote { Ok(()) } - /// TODO: Bubble up error instead of just logging it fn modify_qos( entity: &Entity, config: &MgmtConfig, @@ -324,19 +345,39 @@ pub mod remote { "{} modify user {} set {} --immediate", config.sacctmgr_path, entity.username, qos_str ); - let exit_code = ssh::run_remote_command(sess, &cmd)?; - match exit_code { - 0 => info!( - "Successfully modified QOS of user {} in Slurm.", - entity.username - ), - _ => error!( - "Failed to modify Slurm user! Command '{}' did not exit with 0.", - cmd - ), - }; + super::run_remote_report_slurm_cmd( + sess, + &cmd, + || { + format!( + "Successfully modified QOS of user {} in Slurm.", + entity.username + ) + }, + || { + format!( + "Failed to modify Slurm user! Command '{}' did not exit with 0.", + cmd + ) + }, + )?; + + Ok(()) + } +} +fn run_remote_report_slurm_cmd( + session: &SshSession, + cmd: &str, + on_success: impl Fn() -> String, + on_error: impl Fn() -> String, +) -> AppResult { + let exit_code = ssh::run_remote_command(session, cmd).with_context(&on_error)?; + if exit_code == 0 { + info!("{}", on_success()); Ok(()) + } else { + Err(anyhow!(on_error())) } } diff --git a/src/util.rs b/src/util.rs index 1a3b569..b975e9f 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,11 +1,16 @@ +mod result_accumalator; +pub use result_accumalator::ResultAccumalator; + pub mod io_util { + use crate::prelude::AppResult; use crate::Group; + use anyhow::bail; use log::debug; use std::collections::HashSet; use std::io; - const STUDENT_UID: i32 = 10_000; - const STAFF_UID: i32 = 1_000; + const STUDENT_UID: u32 = 10_000; + const STAFF_UID: u32 = 1_000; pub fn user_input() -> String { let mut input = String::new(); @@ -26,13 +31,12 @@ pub mod io_util { } /// Returns uid which can be used for a new user. + /// /// # Errors /// - if next uid would cause overflow because of its size /// - if next staff uid would be so big that it becomes a student id /// - /// TODO: replace String as error with type implementing error trait. - /// TODO: could u32 be better than i32 ? is there a need for a negative uid. - pub fn get_new_uid(uids: &[i32], group: crate::Group) -> Result { + pub fn get_new_uid(uids: &[u32], group: crate::Group) -> AppResult { // students start at 10000, staff at 1000 let result = if group == Group::Student { uids.iter() @@ -52,13 +56,11 @@ pub mod io_util { let (next_uid, has_overflow) = max.overflowing_add(1); if has_overflow { - return Err( - "Next uid would cause an overflow for an unsigned integer 32".to_string(), - ); + bail!("Next uid would cause an overflow for an unsigned integer 32".to_string(),) } if group == Group::Staff && next_uid >= STUDENT_UID { - return Err(format!("Next uid for staff goes into uid range of students !. Students range starts at {}", STUDENT_UID)); + bail!("Next uid for staff goes into uid range of students !. Students range starts at {}", STUDENT_UID); } Ok(next_uid) @@ -94,7 +96,7 @@ pub mod io_util { #[test] fn should_return_error_for_overflow() { - let actual = get_new_uid(&vec![i32::MAX], Group::Student); + let actual = get_new_uid(&vec![u32::MAX], Group::Student); assert!(actual.is_err()); } #[test] @@ -111,7 +113,7 @@ pub mod io_util { assert_eq!(expected, actual); } - fn assert_return_next_uid(uids: &[i32], group: crate::Group, expected_uid: i32) { + fn assert_return_next_uid(uids: &[u32], group: crate::Group, expected_uid: u32) { let actual = get_new_uid(uids, group); let actual_value = actual.expect("Should not be an error for valid input"); assert_eq!(actual_value, expected_uid); diff --git a/src/util/result_accumalator.rs b/src/util/result_accumalator.rs new file mode 100644 index 0000000..ee7f818 --- /dev/null +++ b/src/util/result_accumalator.rs @@ -0,0 +1,66 @@ +use crate::prelude::AppResult; +use anyhow::{anyhow, Context}; + +/// Allows to collect several errors messages which can be later combined into one error variant for +/// error propagation. If no error message is collected, it resolves to an Ok variant. +pub struct ResultAccumalator { + errs: Vec, + base_err_msg: String, +} + +impl ResultAccumalator { + pub fn new(error_msg: String) -> Self { + Self { + errs: Default::default(), + base_err_msg: error_msg, + } + } + + /// Collects the given error message as the parameter "err_msg" if the parameter "condition" is + /// false + pub fn add_err_if_false(&mut self, condition: bool, err_msg: String) { + if !condition { + self.errs.push(err_msg) + } + } + + /// Collects the given error message as the parameter "err_msg" + pub fn add_err(&mut self, err_msg: String) { + self.errs.push(err_msg) + } +} + +impl From for AppResult { + fn from(value: ResultAccumalator) -> Self { + if value.errs.is_empty() { + return Ok(()); + } + + let all_errs = Err(anyhow!("{}", value.base_err_msg)); + + all_errs.context(value.errs.join("\n")) + } +} + +#[cfg(test)] +mod testing { + use super::*; + + #[test] + fn resolve_to_ok_for_no_errors() { + let mut accumalator = ResultAccumalator::new("Should not resolve to an error.".to_owned()); + accumalator.add_err_if_false(true, "...".to_owned()); + let result = AppResult::from(accumalator); + assert!(result.is_ok()); + } + + #[test] + fn resolve_to_errors() { + let mut accumalator = ResultAccumalator::new("Should not resolve to an error.".to_owned()); + accumalator.add_err_if_false(false, "false".to_owned()); + accumalator.add_err_if_false(true, "true".to_owned()); + accumalator.add_err("added".to_owned()); + let result = AppResult::from(accumalator); + insta::assert_debug_snapshot!(result.err().unwrap()); + } +} diff --git a/src/util/snapshots/usermgmt__util__result_accumalator__testing__resolve_to_errors.snap b/src/util/snapshots/usermgmt__util__result_accumalator__testing__resolve_to_errors.snap new file mode 100644 index 0000000..a54c048 --- /dev/null +++ b/src/util/snapshots/usermgmt__util__result_accumalator__testing__resolve_to_errors.snap @@ -0,0 +1,8 @@ +--- +source: src/util/result_accumalator.rs +expression: result.err().unwrap() +--- +Error { + context: "false\nadded", + source: "Should not resolve to an error.", +}