add Error test, fix clippy

iam/src/error.rs

@@ -97,6 +97,61 @@ pub enum Error {
     Io(std::io::Error),
 }
 
+impl PartialEq for Error {
+    fn eq(&self, other: &Self) -> bool {
+        match (self, other) {
+            (Error::StringError(a), Error::StringError(b)) => a == b,
+            (Error::NoSuchUser(a), Error::NoSuchUser(b)) => a == b,
+            (Error::NoSuchAccount(a), Error::NoSuchAccount(b)) => a == b,
+            (Error::NoSuchServiceAccount(a), Error::NoSuchServiceAccount(b)) => a == b,
+            (Error::NoSuchTempAccount(a), Error::NoSuchTempAccount(b)) => a == b,
+            (Error::NoSuchGroup(a), Error::NoSuchGroup(b)) => a == b,
+            (Error::InvalidServiceType(a), Error::InvalidServiceType(b)) => a == b,
+            (Error::Io(a), Error::Io(b)) => a.kind() == b.kind() && a.to_string() == b.to_string(),
+            // For complex types like PolicyError, CryptoError, JWTError, compare string representations
+            (a, b) => std::mem::discriminant(a) == std::mem::discriminant(b) && a.to_string() == b.to_string(),
+        }
+    }
+}
+
+impl Clone for Error {
+    fn clone(&self) -> Self {
+        match self {
+            Error::PolicyError(e) => Error::StringError(e.to_string()), // Convert to string since PolicyError may not be cloneable
+            Error::StringError(s) => Error::StringError(s.clone()),
+            Error::CryptoError(e) => Error::StringError(format!("crypto: {}", e)), // Convert to string
+            Error::NoSuchUser(s) => Error::NoSuchUser(s.clone()),
+            Error::NoSuchAccount(s) => Error::NoSuchAccount(s.clone()),
+            Error::NoSuchServiceAccount(s) => Error::NoSuchServiceAccount(s.clone()),
+            Error::NoSuchTempAccount(s) => Error::NoSuchTempAccount(s.clone()),
+            Error::NoSuchGroup(s) => Error::NoSuchGroup(s.clone()),
+            Error::NoSuchPolicy => Error::NoSuchPolicy,
+            Error::PolicyInUse => Error::PolicyInUse,
+            Error::GroupNotEmpty => Error::GroupNotEmpty,
+            Error::InvalidArgument => Error::InvalidArgument,
+            Error::IamSysNotInitialized => Error::IamSysNotInitialized,
+            Error::InvalidServiceType(s) => Error::InvalidServiceType(s.clone()),
+            Error::ErrCredMalformed => Error::ErrCredMalformed,
+            Error::CredNotInitialized => Error::CredNotInitialized,
+            Error::InvalidAccessKeyLength => Error::InvalidAccessKeyLength,
+            Error::InvalidSecretKeyLength => Error::InvalidSecretKeyLength,
+            Error::ContainsReservedChars => Error::ContainsReservedChars,
+            Error::GroupNameContainsReservedChars => Error::GroupNameContainsReservedChars,
+            Error::JWTError(e) => Error::StringError(format!("jwt err {}", e)), // Convert to string
+            Error::NoAccessKey => Error::NoAccessKey,
+            Error::InvalidToken => Error::InvalidToken,
+            Error::InvalidAccessKey => Error::InvalidAccessKey,
+            Error::IAMActionNotAllowed => Error::IAMActionNotAllowed,
+            Error::InvalidExpiration => Error::InvalidExpiration,
+            Error::NoSecretKeyWithAccessKey => Error::NoSecretKeyWithAccessKey,
+            Error::NoAccessKeyWithSecretKey => Error::NoAccessKeyWithSecretKey,
+            Error::PolicyTooLarge => Error::PolicyTooLarge,
+            Error::ConfigNotFound => Error::ConfigNotFound,
+            Error::Io(e) => Error::Io(std::io::Error::new(e.kind(), e.to_string())),
+        }
+    }
+}
+
 impl Error {
     pub fn other<E>(error: E) -> Self
     where
@@ -225,3 +280,142 @@ pub fn is_err_no_such_service_account(err: &Error) -> bool {
 //         Error::msg(e.to_string())
 //     }
 // }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::io::{Error as IoError, ErrorKind};
+
+    #[test]
+    fn test_iam_error_to_io_error_conversion() {
+        let iam_errors = vec![
+            Error::NoSuchUser("testuser".to_string()),
+            Error::NoSuchAccount("testaccount".to_string()),
+            Error::InvalidArgument,
+            Error::IAMActionNotAllowed,
+            Error::PolicyTooLarge,
+            Error::ConfigNotFound,
+        ];
+
+        for iam_error in iam_errors {
+            let io_error: std::io::Error = iam_error.clone().into();
+
+            // Check that conversion creates an io::Error
+            assert_eq!(io_error.kind(), ErrorKind::Other);
+
+            // Check that the error message is preserved
+            assert!(io_error.to_string().contains(&iam_error.to_string()));
+        }
+    }
+
+    #[test]
+    fn test_iam_error_from_storage_error() {
+        // Test conversion from StorageError
+        let storage_error = ecstore::error::StorageError::ConfigNotFound;
+        let iam_error: Error = storage_error.into();
+        assert_eq!(iam_error, Error::ConfigNotFound);
+
+        // Test reverse conversion
+        let back_to_storage: ecstore::error::StorageError = iam_error.into();
+        assert_eq!(back_to_storage, ecstore::error::StorageError::ConfigNotFound);
+    }
+
+    #[test]
+    fn test_iam_error_from_policy_error() {
+        use policy::error::Error as PolicyError;
+
+        let policy_errors = vec![
+            (PolicyError::NoSuchUser("user1".to_string()), Error::NoSuchUser("user1".to_string())),
+            (PolicyError::NoSuchPolicy, Error::NoSuchPolicy),
+            (PolicyError::InvalidArgument, Error::InvalidArgument),
+            (PolicyError::PolicyTooLarge, Error::PolicyTooLarge),
+        ];
+
+        for (policy_error, expected_iam_error) in policy_errors {
+            let converted_iam_error: Error = policy_error.into();
+            assert_eq!(converted_iam_error, expected_iam_error);
+        }
+    }
+
+    #[test]
+    fn test_iam_error_other_function() {
+        let custom_error = "Custom IAM error";
+        let iam_error = Error::other(custom_error);
+
+        match iam_error {
+            Error::Io(io_error) => {
+                assert!(io_error.to_string().contains(custom_error));
+                assert_eq!(io_error.kind(), ErrorKind::Other);
+            }
+            _ => panic!("Expected Io variant"),
+        }
+    }
+
+    #[test]
+    fn test_iam_error_from_serde_json() {
+        // Test conversion from serde_json::Error
+        let invalid_json = r#"{"invalid": json}"#;
+        let json_error = serde_json::from_str::<serde_json::Value>(invalid_json).unwrap_err();
+        let iam_error: Error = json_error.into();
+
+        match iam_error {
+            Error::Io(io_error) => {
+                assert_eq!(io_error.kind(), ErrorKind::Other);
+            }
+            _ => panic!("Expected Io variant"),
+        }
+    }
+
+    #[test]
+    fn test_helper_functions() {
+        // Test helper functions for error type checking
+        assert!(is_err_config_not_found(&Error::ConfigNotFound));
+        assert!(!is_err_config_not_found(&Error::NoSuchPolicy));
+
+        assert!(is_err_no_such_policy(&Error::NoSuchPolicy));
+        assert!(!is_err_no_such_policy(&Error::ConfigNotFound));
+
+        assert!(is_err_no_such_user(&Error::NoSuchUser("test".to_string())));
+        assert!(!is_err_no_such_user(&Error::NoSuchAccount("test".to_string())));
+
+        assert!(is_err_no_such_account(&Error::NoSuchAccount("test".to_string())));
+        assert!(!is_err_no_such_account(&Error::NoSuchUser("test".to_string())));
+
+        assert!(is_err_no_such_temp_account(&Error::NoSuchTempAccount("test".to_string())));
+        assert!(!is_err_no_such_temp_account(&Error::NoSuchAccount("test".to_string())));
+
+        assert!(is_err_no_such_group(&Error::NoSuchGroup("test".to_string())));
+        assert!(!is_err_no_such_group(&Error::NoSuchUser("test".to_string())));
+
+        assert!(is_err_no_such_service_account(&Error::NoSuchServiceAccount("test".to_string())));
+        assert!(!is_err_no_such_service_account(&Error::NoSuchAccount("test".to_string())));
+    }
+
+    #[test]
+    fn test_iam_error_io_preservation() {
+        // Test that Io variant preserves original io::Error
+        let original_io = IoError::new(ErrorKind::PermissionDenied, "access denied");
+        let iam_error = Error::Io(original_io);
+
+        let converted_io: std::io::Error = iam_error.into();
+        // Note: Our clone implementation creates a new io::Error with the same kind and message
+        // but it becomes ErrorKind::Other when cloned
+        assert_eq!(converted_io.kind(), ErrorKind::Other);
+        assert!(converted_io.to_string().contains("access denied"));
+    }
+
+    #[test]
+    fn test_error_display_format() {
+        let test_cases = vec![
+            (Error::NoSuchUser("testuser".to_string()), "user 'testuser' does not exist"),
+            (Error::NoSuchAccount("testaccount".to_string()), "account 'testaccount' does not exist"),
+            (Error::InvalidArgument, "invalid arguments specified"),
+            (Error::IAMActionNotAllowed, "action not allowed"),
+            (Error::ConfigNotFound, "config not found"),
+        ];
+
+        for (error, expected_message) in test_cases {
+            assert_eq!(error.to_string(), expected_message);
+        }
+    }
+}

iam/src/manager.rs

@@ -1,22 +1,22 @@
-use crate::error::{is_err_config_not_found, Error, Result};
+use crate::error::{Error, Result, is_err_config_not_found};
 use crate::{
     cache::{Cache, CacheEntity},
-    error::{is_err_no_such_group, is_err_no_such_policy, is_err_no_such_user, Error as IamError},
+    error::{Error as IamError, is_err_no_such_group, is_err_no_such_policy, is_err_no_such_user},
     get_global_action_cred,
-    store::{object::IAM_CONFIG_PREFIX, GroupInfo, MappedPolicy, Store, UserType},
+    store::{GroupInfo, MappedPolicy, Store, UserType, object::IAM_CONFIG_PREFIX},
     sys::{
-        UpdateServiceAccountOpts, MAX_SVCSESSION_POLICY_SIZE, SESSION_POLICY_NAME, SESSION_POLICY_NAME_EXTRACTED,
-        STATUS_DISABLED, STATUS_ENABLED,
+        MAX_SVCSESSION_POLICY_SIZE, SESSION_POLICY_NAME, SESSION_POLICY_NAME_EXTRACTED, STATUS_DISABLED, STATUS_ENABLED,
+        UpdateServiceAccountOpts,
     },
 };
 use ecstore::utils::{crypto::base64_encode, path::path_join_buf};
 use madmin::{AccountStatus, AddOrUpdateUserReq, GroupDesc};
 use policy::{
     arn::ARN,
-    auth::{self, get_claims_from_token_with_secret, is_secret_key_valid, jwt_sign, Credentials, UserIdentity},
+    auth::{self, Credentials, UserIdentity, get_claims_from_token_with_secret, is_secret_key_valid, jwt_sign},
     format::Format,
     policy::{
-        default::DEFAULT_POLICIES, iam_policy_claim_name_sa, Policy, PolicyDoc, EMBEDDED_POLICY_TYPE, INHERITED_POLICY_TYPE,
+        EMBEDDED_POLICY_TYPE, INHERITED_POLICY_TYPE, Policy, PolicyDoc, default::DEFAULT_POLICIES, iam_policy_claim_name_sa,
     },
 };
 use serde::{Deserialize, Serialize};
@@ -24,8 +24,8 @@ use serde_json::Value;
 use std::{
     collections::{HashMap, HashSet},
     sync::{
-        atomic::{AtomicBool, AtomicI64, Ordering},
         Arc,
+        atomic::{AtomicBool, AtomicI64, Ordering},
     },
     time::Duration,
 };
@@ -633,7 +633,7 @@ where
                     Cache::add_or_update(&self.cache.user_policies, name, p, OffsetDateTime::now_utc());
                     p.clone()
                 } else {
-                    let mp = match self.cache.sts_policies.load().get(name) {
+                    match self.cache.sts_policies.load().get(name) {
                         Some(p) => p.clone(),
                         None => {
                             let mut m = HashMap::new();
@@ -645,8 +645,7 @@ where
                                 MappedPolicy::default()
                             }
                         }
-                    };
-                    mp
+                    }
                 }
             }
         };

iam/src/store.rs

@@ -3,7 +3,7 @@ pub mod object;
 use crate::cache::Cache;
 use crate::error::Result;
 use policy::{auth::UserIdentity, policy::PolicyDoc};
-use serde::{de::DeserializeOwned, Deserialize, Serialize};
+use serde::{Deserialize, Serialize, de::DeserializeOwned};
 use std::collections::{HashMap, HashSet};
 use time::OffsetDateTime;
 
@@ -49,7 +49,7 @@ pub trait Store: Clone + Send + Sync + 'static {
         m: &mut HashMap<String, MappedPolicy>,
     ) -> Result<()>;
     async fn load_mapped_policys(&self, user_type: UserType, is_group: bool, m: &mut HashMap<String, MappedPolicy>)
-        -> Result<()>;
+    -> Result<()>;
 
     async fn load_all(&self, cache: &Cache) -> Result<()>;
 }

iam/src/store/object.rs

@@ -1,5 +1,5 @@
 use super::{GroupInfo, MappedPolicy, Store, UserType};
-use crate::error::{is_err_config_not_found, Error, Result};
+use crate::error::{Error, Result, is_err_config_not_found};
 use crate::{
     cache::{Cache, CacheEntity},
     error::{is_err_no_such_policy, is_err_no_such_user},
@@ -8,18 +8,18 @@ use crate::{
 };
 use ecstore::{
     config::{
-        com::{delete_config, read_config, read_config_with_metadata, save_config},
         RUSTFS_CONFIG_PREFIX,
+        com::{delete_config, read_config, read_config_with_metadata, save_config},
     },
     store::ECStore,
     store_api::{ObjectInfo, ObjectOptions},
     store_list_objects::{ObjectInfoOrErr, WalkOptions},
-    utils::path::{path_join_buf, SLASH_SEPARATOR},
+    utils::path::{SLASH_SEPARATOR, path_join_buf},
 };
 use futures::future::join_all;
 use lazy_static::lazy_static;
 use policy::{auth::UserIdentity, policy::PolicyDoc};
-use serde::{de::DeserializeOwned, Serialize};
+use serde::{Serialize, de::DeserializeOwned};
 use std::{collections::HashMap, sync::Arc};
 use tokio::sync::broadcast::{self, Receiver as B_Receiver};
 use tokio::sync::mpsc::{self, Sender};

iam/src/sys.rs

@@ -1,11 +1,11 @@
+use crate::error::Error as IamError;
 use crate::error::is_err_no_such_account;
 use crate::error::is_err_no_such_temp_account;
-use crate::error::Error as IamError;
 use crate::error::{Error, Result};
 use crate::get_global_action_cred;
+use crate::manager::IamCache;
 use crate::manager::extract_jwt_claims;
 use crate::manager::get_default_policyes;
-use crate::manager::IamCache;
 use crate::store::MappedPolicy;
 use crate::store::Store;
 use crate::store::UserType;
@@ -14,22 +14,22 @@ use ecstore::utils::crypto::base64_encode;
 use madmin::AddOrUpdateUserReq;
 use madmin::GroupDesc;
 use policy::arn::ARN;
+use policy::auth::ACCOUNT_ON;
+use policy::auth::Credentials;
+use policy::auth::UserIdentity;
 use policy::auth::contains_reserved_chars;
 use policy::auth::create_new_credentials_with_metadata;
 use policy::auth::generate_credentials;
 use policy::auth::is_access_key_valid;
 use policy::auth::is_secret_key_valid;
-use policy::auth::Credentials;
-use policy::auth::UserIdentity;
-use policy::auth::ACCOUNT_ON;
-use policy::policy::iam_policy_claim_name_sa;
 use policy::policy::Args;
-use policy::policy::Policy;
-use policy::policy::PolicyDoc;
 use policy::policy::EMBEDDED_POLICY_TYPE;
 use policy::policy::INHERITED_POLICY_TYPE;
-use serde_json::json;
+use policy::policy::Policy;
+use policy::policy::PolicyDoc;
+use policy::policy::iam_policy_claim_name_sa;
 use serde_json::Value;
+use serde_json::json;
 use std::collections::HashMap;
 use std::sync::Arc;
 use time::OffsetDateTime;

iam/src/utils.rs

@@ -1,6 +1,6 @@
 use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header};
 use rand::{Rng, RngCore};
-use serde::{de::DeserializeOwned, Serialize};
+use serde::{Serialize, de::DeserializeOwned};
 use std::io::{Error, Result};
 
 pub fn gen_access_key(length: usize) -> Result<String> {