Merge pull request #503 from rustfs/refactor/unify-bucket-metadata-errors

refactor(ecstore): simplify bucket metadata error handling by using C…

ecstore/src/bucket/error.rs

@@ -32,19 +32,9 @@ impl BucketMetadataError {
     }
 }
 
-impl From<BucketMetadataError> for Error {
-    fn from(e: BucketMetadataError) -> Self {
-        match e {
-            BucketMetadataError::BucketPolicyNotFound => Error::BucketPolicyNotFound,
-            _ => Error::other(e),
-        }
-    }
-}
-
 impl From<Error> for BucketMetadataError {
     fn from(e: Error) -> Self {
         match e {
-            Error::BucketPolicyNotFound => BucketMetadataError::BucketPolicyNotFound,
             Error::Io(e) => e.into(),
             _ => BucketMetadataError::other(e),
         }

ecstore/src/bucket/metadata_sys.rs

@@ -1,10 +1,4 @@
-use std::collections::HashSet;
-use std::sync::OnceLock;
-use std::time::Duration;
-use std::{collections::HashMap, sync::Arc};
-
 use crate::StorageAPI;
-use crate::bucket::error::BucketMetadataError;
 use crate::bucket::metadata::{BUCKET_LIFECYCLE_CONFIG, load_bucket_metadata_parse};
 use crate::bucket::utils::{deserialize, is_meta_bucketname};
 use crate::cmd::bucket_targets;
@@ -18,6 +12,10 @@ use s3s::dto::{
     BucketLifecycleConfiguration, NotificationConfiguration, ObjectLockConfiguration, ReplicationConfiguration,
     ServerSideEncryptionConfiguration, Tagging, VersioningConfiguration,
 };
+use std::collections::HashSet;
+use std::sync::OnceLock;
+use std::time::Duration;
+use std::{collections::HashMap, sync::Arc};
 use time::OffsetDateTime;
 use tokio::sync::RwLock;
 use tokio::time::sleep;
@@ -400,85 +398,46 @@ impl BucketMetadataSys {
     }
 
     pub async fn get_bucket_policy(&self, bucket: &str) -> Result<(BucketPolicy, OffsetDateTime)> {
-        let bm = match self.get_config(bucket).await {
+        let (bm, _) = self.get_config(bucket).await?;
-            Ok((res, _)) => res,
-            Err(err) => {
-                warn!("get_bucket_policy err {:?}", &err);
-                return if err == Error::ConfigNotFound {
-                    Err(BucketMetadataError::BucketPolicyNotFound.into())
-                } else {
-                    Err(err)
-                };
-            }
-        };
 
         if let Some(config) = &bm.policy_config {
             Ok((config.clone(), bm.policy_config_updated_at))
         } else {
-            Err(BucketMetadataError::BucketPolicyNotFound.into())
+            Err(Error::ConfigNotFound)
         }
     }
 
     pub async fn get_tagging_config(&self, bucket: &str) -> Result<(Tagging, OffsetDateTime)> {
-        let bm = match self.get_config(bucket).await {
+        let (bm, _) = self.get_config(bucket).await?;
-            Ok((res, _)) => res,
-            Err(err) => {
-                warn!("get_tagging_config err {:?}", &err);
-                return if err == Error::ConfigNotFound {
-                    Err(BucketMetadataError::TaggingNotFound.into())
-                } else {
-                    Err(err)
-                };
-            }
-        };
 
         if let Some(config) = &bm.tagging_config {
             Ok((config.clone(), bm.tagging_config_updated_at))
         } else {
-            Err(BucketMetadataError::TaggingNotFound.into())
+            Err(Error::ConfigNotFound)
         }
     }
 
     pub async fn get_object_lock_config(&self, bucket: &str) -> Result<(ObjectLockConfiguration, OffsetDateTime)> {
-        let bm = match self.get_config(bucket).await {
+        let (bm, _) = self.get_config(bucket).await?;
-            Ok((res, _)) => res,
-            Err(err) => {
-                return if err == Error::ConfigNotFound {
-                    Err(BucketMetadataError::BucketObjectLockConfigNotFound.into())
-                } else {
-                    Err(err)
-                };
-            }
-        };
 
         if let Some(config) = &bm.object_lock_config {
             Ok((config.clone(), bm.object_lock_config_updated_at))
         } else {
-            Err(BucketMetadataError::BucketObjectLockConfigNotFound.into())
+            Err(Error::ConfigNotFound)
         }
     }
 
     pub async fn get_lifecycle_config(&self, bucket: &str) -> Result<(BucketLifecycleConfiguration, OffsetDateTime)> {
-        let bm = match self.get_config(bucket).await {
+        let (bm, _) = self.get_config(bucket).await?;
-            Ok((res, _)) => res,
-            Err(err) => {
-                warn!("get_lifecycle_config err {:?}", &err);
-                return if err == Error::ConfigNotFound {
-                    Err(BucketMetadataError::BucketLifecycleNotFound.into())
-                } else {
-                    Err(err)
-                };
-            }
-        };
 
         if let Some(config) = &bm.lifecycle_config {
             if config.rules.is_empty() {
-                Err(BucketMetadataError::BucketLifecycleNotFound.into())
+                Err(Error::ConfigNotFound)
             } else {
                 Ok((config.clone(), bm.lifecycle_config_updated_at))
             }
         } else {
-            Err(BucketMetadataError::BucketLifecycleNotFound.into())
+            Err(Error::ConfigNotFound)
         }
     }
 
@@ -499,22 +458,12 @@ impl BucketMetadataSys {
     }
 
     pub async fn get_sse_config(&self, bucket: &str) -> Result<(ServerSideEncryptionConfiguration, OffsetDateTime)> {
-        let bm = match self.get_config(bucket).await {
+        let (bm, _) = self.get_config(bucket).await?;
-            Ok((res, _)) => res,
-            Err(err) => {
-                warn!("get_sse_config err {:?}", &err);
-                return if err == Error::ConfigNotFound {
-                    Err(BucketMetadataError::BucketSSEConfigNotFound.into())
-                } else {
-                    Err(err)
-                };
-            }
-        };
 
         if let Some(config) = &bm.sse_config {
             Ok((config.clone(), bm.encryption_config_updated_at))
         } else {
-            Err(BucketMetadataError::BucketSSEConfigNotFound.into())
+            Err(Error::ConfigNotFound)
         }
     }
 
@@ -530,42 +479,17 @@ impl BucketMetadataSys {
     }
 
     pub async fn get_quota_config(&self, bucket: &str) -> Result<(BucketQuota, OffsetDateTime)> {
-        let bm = match self.get_config(bucket).await {
+        let (bm, _) = self.get_config(bucket).await?;
-            Ok((res, _)) => res,
-            Err(err) => {
-                warn!("get_quota_config err {:?}", &err);
-                return if err == Error::ConfigNotFound {
-                    Err(BucketMetadataError::BucketQuotaConfigNotFound.into())
-                } else {
-                    Err(err)
-                };
-            }
-        };
 
         if let Some(config) = &bm.quota_config {
             Ok((config.clone(), bm.quota_config_updated_at))
         } else {
-            Err(BucketMetadataError::BucketQuotaConfigNotFound.into())
+            Err(Error::ConfigNotFound)
         }
     }
 
     pub async fn get_replication_config(&self, bucket: &str) -> Result<(ReplicationConfiguration, OffsetDateTime)> {
-        let (bm, reload) = match self.get_config(bucket).await {
+        let (bm, reload) = self.get_config(bucket).await?;
-            Ok(res) => {
-                if res.0.replication_config.is_none() {
-                    return Err(BucketMetadataError::BucketReplicationConfigNotFound.into());
-                }
-                res
-            }
-            Err(err) => {
-                warn!("get_replication_config err {:?}", &err);
-                return if err == Error::ConfigNotFound {
-                    Err(BucketMetadataError::BucketReplicationConfigNotFound.into())
-                } else {
-                    Err(err)
-                };
-            }
-        };
 
         if let Some(config) = &bm.replication_config {
             if reload {
@@ -574,24 +498,12 @@ impl BucketMetadataSys {
             //println!("549 {:?}", config.clone());
             Ok((config.clone(), bm.replication_config_updated_at))
         } else {
-            Err(BucketMetadataError::BucketReplicationConfigNotFound.into())
+            Err(Error::ConfigNotFound)
         }
     }
 
     pub async fn get_bucket_targets_config(&self, bucket: &str) -> Result<BucketTargets> {
-        let (bm, reload) = match self.get_config(bucket).await {
+        let (bm, reload) = self.get_config(bucket).await?;
-            Ok(res) => res,
-            Err(err) => {
-                warn!("get_replication_config err {:?}", &err);
-                return if err == Error::ConfigNotFound {
-                    Err(BucketMetadataError::BucketRemoteTargetNotFound.into())
-                } else {
-                    Err(err)
-                };
-            }
-        };
-
-        println!("573");
 
         if let Some(config) = &bm.bucket_target_config {
             if reload {
@@ -601,7 +513,7 @@ impl BucketMetadataSys {
 
             Ok(config.clone())
         } else {
-            Err(BucketMetadataError::BucketRemoteTargetNotFound.into())
+            Err(Error::ConfigNotFound)
         }
     }
 }

ecstore/src/error.rs

@@ -2,6 +2,7 @@ use s3s::{S3Error, S3ErrorCode};
 
 use rustfs_utils::path::decode_dir_object;
 
+use crate::bucket::error::BucketMetadataError;
 use crate::disk::error::DiskError;
 
 pub type Error = StorageError;
@@ -166,9 +167,6 @@ pub enum StorageError {
     #[error("first disk wait")]
     FirstDiskWait,
 
-    #[error("Bucket policy not found")]
-    BucketPolicyNotFound,
-
     #[error("Io error: {0}")]
     Io(std::io::Error),
 }
@@ -257,6 +255,22 @@ impl From<StorageError> for DiskError {
     }
 }
 
+impl From<BucketMetadataError> for Error {
+    fn from(e: BucketMetadataError) -> Self {
+        match e {
+            BucketMetadataError::TaggingNotFound => Error::ConfigNotFound,
+            BucketMetadataError::BucketPolicyNotFound => Error::ConfigNotFound,
+            BucketMetadataError::BucketObjectLockConfigNotFound => Error::ConfigNotFound,
+            BucketMetadataError::BucketLifecycleNotFound => Error::ConfigNotFound,
+            BucketMetadataError::BucketSSEConfigNotFound => Error::ConfigNotFound,
+            BucketMetadataError::BucketQuotaConfigNotFound => Error::ConfigNotFound,
+            BucketMetadataError::BucketReplicationConfigNotFound => Error::ConfigNotFound,
+            BucketMetadataError::BucketRemoteTargetNotFound => Error::ConfigNotFound,
+            _ => Error::other(e),
+        }
+    }
+}
+
 impl From<std::io::Error> for StorageError {
     fn from(e: std::io::Error) -> Self {
         match e.downcast::<StorageError>() {
@@ -381,7 +395,6 @@ impl Clone for StorageError {
             StorageError::FirstDiskWait => StorageError::FirstDiskWait,
             StorageError::TooManyOpenFiles => StorageError::TooManyOpenFiles,
             StorageError::NoHealRequired => StorageError::NoHealRequired,
-            StorageError::BucketPolicyNotFound => StorageError::BucketPolicyNotFound,
         }
     }
 }
@@ -444,7 +457,6 @@ impl StorageError {
             StorageError::ConfigNotFound => 0x35,
             StorageError::TooManyOpenFiles => 0x36,
             StorageError::NoHealRequired => 0x37,
-            StorageError::BucketPolicyNotFound => 0x38,
         }
     }
 
@@ -509,7 +521,6 @@ impl StorageError {
             0x35 => Some(StorageError::ConfigNotFound),
             0x36 => Some(StorageError::TooManyOpenFiles),
             0x37 => Some(StorageError::NoHealRequired),
-            0x38 => Some(StorageError::BucketPolicyNotFound),
             _ => None,
         }
     }

rustfs/src/storage/ecfs.rs

@@ -1613,7 +1613,7 @@ impl S3 for FS {
         let cfg = match PolicySys::get(&bucket).await {
             Ok(res) => res,
             Err(err) => {
-                if StorageError::BucketPolicyNotFound == err {
+                if StorageError::ConfigNotFound == err {
                     return Err(s3_error!(NoSuchBucketPolicy));
                 }
                 return Err(S3Error::with_message(S3ErrorCode::InternalError, err.to_string()));