diff options
author | Alex Auvolat <alex@adnab.me> | 2022-05-13 19:36:17 +0200 |
---|---|---|
committer | Alex Auvolat <alex@adnab.me> | 2022-05-13 19:36:17 +0200 |
commit | 8033bdb0b4577133cff7d4d90a811ed8f3e13365 (patch) | |
tree | 5f4abf43f18322c64db68787d91a227aa4421b97 | |
parent | 5a535788fc0a69950bbfdc6f189597c5e37a6e3b (diff) | |
download | garage-8033bdb0b4577133cff7d4d90a811ed8f3e13365.tar.gz garage-8033bdb0b4577133cff7d4d90a811ed8f3e13365.zip |
More precisions in errors & small refactoring
-rw-r--r-- | src/api/admin/bucket.rs | 9 | ||||
-rw-r--r-- | src/api/admin/error.rs | 16 | ||||
-rw-r--r-- | src/api/admin/key.rs | 25 | ||||
-rw-r--r-- | src/api/common_error.rs | 16 | ||||
-rw-r--r-- | src/api/k2v/error.rs | 6 | ||||
-rw-r--r-- | src/api/s3/error.rs | 6 |
6 files changed, 35 insertions, 43 deletions
diff --git a/src/api/admin/bucket.rs b/src/api/admin/bucket.rs index c5518e4e..00450319 100644 --- a/src/api/admin/bucket.rs +++ b/src/api/admin/bucket.rs @@ -248,11 +248,10 @@ pub async fn handle_create_bucket( } let key = garage - .key_table - .get(&EmptyKey, &la.access_key_id) - .await? - .ok_or(Error::NoSuchAccessKey)?; - let state = key.state.as_option().ok_or(Error::NoSuchAccessKey)?; + .key_helper() + .get_existing_key(&la.access_key_id) + .await?; + let state = key.state.as_option().unwrap(); if matches!(state.local_aliases.get(&la.alias), Some(_)) { return Err(Error::bad_request("Local alias already exists")); } diff --git a/src/api/admin/error.rs b/src/api/admin/error.rs index 38dfe5b6..cd7e6af7 100644 --- a/src/api/admin/error.rs +++ b/src/api/admin/error.rs @@ -18,8 +18,8 @@ pub enum Error { // Category: cannot process /// The API access key does not exist - #[error(display = "Access key not found")] - NoSuchAccessKey, + #[error(display = "Access key not found: {}", _0)] + NoSuchAccessKey(String), } impl<T> From<T> for Error @@ -38,9 +38,11 @@ impl From<HelperError> for Error { match err { HelperError::Internal(i) => Self::CommonError(CommonError::InternalError(i)), HelperError::BadRequest(b) => Self::CommonError(CommonError::BadRequest(b)), - HelperError::InvalidBucketName(_) => Self::CommonError(CommonError::InvalidBucketName), - HelperError::NoSuchBucket(_) => Self::CommonError(CommonError::NoSuchBucket), - HelperError::NoSuchAccessKey(_) => Self::NoSuchAccessKey, + HelperError::InvalidBucketName(n) => { + Self::CommonError(CommonError::InvalidBucketName(n)) + } + HelperError::NoSuchBucket(n) => Self::CommonError(CommonError::NoSuchBucket(n)), + HelperError::NoSuchAccessKey(n) => Self::NoSuchAccessKey(n), } } } @@ -49,7 +51,7 @@ impl Error { fn code(&self) -> &'static str { match self { Error::CommonError(c) => c.aws_code(), - Error::NoSuchAccessKey => "NoSuchAccessKey", + Error::NoSuchAccessKey(_) => "NoSuchAccessKey", } } } @@ -59,7 +61,7 @@ impl ApiError for Error { fn http_status_code(&self) -> StatusCode { match self { Error::CommonError(c) => c.http_status_code(), - Error::NoSuchAccessKey => StatusCode::NOT_FOUND, + Error::NoSuchAccessKey(_) => StatusCode::NOT_FOUND, } } diff --git a/src/api/admin/key.rs b/src/api/admin/key.rs index 8060bf1a..1e910d52 100644 --- a/src/api/admin/key.rs +++ b/src/api/admin/key.rs @@ -50,17 +50,12 @@ pub async fn handle_get_key_info( search: Option<String>, ) -> Result<Response<Body>, Error> { let key = if let Some(id) = id { - garage - .key_table - .get(&EmptyKey, &id) - .await? - .ok_or(Error::NoSuchAccessKey)? + garage.key_helper().get_existing_key(&id).await? } else if let Some(search) = search { garage .key_helper() .get_existing_matching_key(&search) - .await - .map_err(|_| Error::NoSuchAccessKey)? + .await? } else { unreachable!(); }; @@ -92,13 +87,9 @@ pub async fn handle_update_key( ) -> Result<Response<Body>, Error> { let req = parse_json_body::<UpdateKeyRequest>(req).await?; - let mut key = garage - .key_table - .get(&EmptyKey, &id) - .await? - .ok_or(Error::NoSuchAccessKey)?; + let mut key = garage.key_helper().get_existing_key(&id).await?; - let key_state = key.state.as_option_mut().ok_or(Error::NoSuchAccessKey)?; + let key_state = key.state.as_option_mut().unwrap(); if let Some(new_name) = req.name { key_state.name.update(new_name); @@ -127,13 +118,9 @@ struct UpdateKeyRequest { } pub async fn handle_delete_key(garage: &Arc<Garage>, id: String) -> Result<Response<Body>, Error> { - let mut key = garage - .key_table - .get(&EmptyKey, &id) - .await? - .ok_or(Error::NoSuchAccessKey)?; + let mut key = garage.key_helper().get_existing_key(&id).await?; - key.state.as_option().ok_or(Error::NoSuchAccessKey)?; + key.state.as_option().unwrap(); garage.key_helper().delete_key(&mut key).await?; diff --git a/src/api/common_error.rs b/src/api/common_error.rs index b6dbf059..20f9f266 100644 --- a/src/api/common_error.rs +++ b/src/api/common_error.rs @@ -32,8 +32,8 @@ pub enum CommonError { // These have to be error codes referenced in the S3 spec here: // https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html#ErrorCodeList /// The bucket requested don't exists - #[error(display = "Bucket not found")] - NoSuchBucket, + #[error(display = "Bucket not found: {}", _0)] + NoSuchBucket(String), /// Tried to create a bucket that already exist #[error(display = "Bucket already exists")] @@ -45,8 +45,8 @@ pub enum CommonError { // Category: bad request /// Bucket name is not valid according to AWS S3 specs - #[error(display = "Invalid bucket name")] - InvalidBucketName, + #[error(display = "Invalid bucket name: {}", _0)] + InvalidBucketName(String), } impl CommonError { @@ -62,9 +62,9 @@ impl CommonError { } CommonError::BadRequest(_) => StatusCode::BAD_REQUEST, CommonError::Forbidden(_) => StatusCode::FORBIDDEN, - CommonError::NoSuchBucket => StatusCode::NOT_FOUND, + CommonError::NoSuchBucket(_) => StatusCode::NOT_FOUND, CommonError::BucketNotEmpty | CommonError::BucketAlreadyExists => StatusCode::CONFLICT, - CommonError::InvalidBucketName => StatusCode::BAD_REQUEST, + CommonError::InvalidBucketName(_) => StatusCode::BAD_REQUEST, } } @@ -80,10 +80,10 @@ impl CommonError { "InternalError" } CommonError::BadRequest(_) => "InvalidRequest", - CommonError::NoSuchBucket => "NoSuchBucket", + CommonError::NoSuchBucket(_) => "NoSuchBucket", CommonError::BucketAlreadyExists => "BucketAlreadyExists", CommonError::BucketNotEmpty => "BucketNotEmpty", - CommonError::InvalidBucketName => "InvalidBucketName", + CommonError::InvalidBucketName(_) => "InvalidBucketName", } } diff --git a/src/api/k2v/error.rs b/src/api/k2v/error.rs index 85d5de9d..dbd9a5f5 100644 --- a/src/api/k2v/error.rs +++ b/src/api/k2v/error.rs @@ -59,8 +59,10 @@ impl From<HelperError> for Error { match err { HelperError::Internal(i) => Self::CommonError(CommonError::InternalError(i)), HelperError::BadRequest(b) => Self::CommonError(CommonError::BadRequest(b)), - HelperError::InvalidBucketName(_) => Self::CommonError(CommonError::InvalidBucketName), - HelperError::NoSuchBucket(_) => Self::CommonError(CommonError::NoSuchBucket), + HelperError::InvalidBucketName(n) => { + Self::CommonError(CommonError::InvalidBucketName(n)) + } + HelperError::NoSuchBucket(n) => Self::CommonError(CommonError::NoSuchBucket(n)), e => Self::CommonError(CommonError::BadRequest(format!("{}", e))), } } diff --git a/src/api/s3/error.rs b/src/api/s3/error.rs index 4edff3a1..f2096bea 100644 --- a/src/api/s3/error.rs +++ b/src/api/s3/error.rs @@ -91,8 +91,10 @@ impl From<HelperError> for Error { match err { HelperError::Internal(i) => Self::CommonError(CommonError::InternalError(i)), HelperError::BadRequest(b) => Self::CommonError(CommonError::BadRequest(b)), - HelperError::InvalidBucketName(_) => Self::CommonError(CommonError::InvalidBucketName), - HelperError::NoSuchBucket(_) => Self::CommonError(CommonError::NoSuchBucket), + HelperError::InvalidBucketName(n) => { + Self::CommonError(CommonError::InvalidBucketName(n)) + } + HelperError::NoSuchBucket(n) => Self::CommonError(CommonError::NoSuchBucket(n)), e => Self::bad_request(format!("{}", e)), } } |