aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlex Auvolat <alex@adnab.me>2022-05-13 15:21:32 +0200
committerAlex Auvolat <alex@adnab.me>2022-05-13 15:21:32 +0200
commit7a5d329e49cc7018cbfa14d37589f51860f66cf0 (patch)
tree50be9a5eea1b2415bb541aff0c8cb33c0f47b8e4
parentf82b938033f1a01a136315b5f37ecb89b78bca0c (diff)
downloadgarage-7a5d329e49cc7018cbfa14d37589f51860f66cf0.tar.gz
garage-7a5d329e49cc7018cbfa14d37589f51860f66cf0.zip
More error refactoring
-rw-r--r--src/api/admin/bucket.rs4
-rw-r--r--src/api/admin/cluster.rs2
-rw-r--r--src/api/admin/error.rs5
-rw-r--r--src/api/admin/key.rs2
-rw-r--r--src/api/admin/mod.rs11
-rw-r--r--src/api/common_error.rs5
-rw-r--r--src/api/helpers.rs29
-rw-r--r--src/api/k2v/api_server.rs2
-rw-r--r--src/api/s3/api_server.rs2
-rw-r--r--src/api/s3/copy.rs4
-rw-r--r--src/api/s3/post_object.rs3
-rw-r--r--src/model/helper/bucket.rs22
12 files changed, 37 insertions, 54 deletions
diff --git a/src/api/admin/bucket.rs b/src/api/admin/bucket.rs
index db1fda0f..b226c015 100644
--- a/src/api/admin/bucket.rs
+++ b/src/api/admin/bucket.rs
@@ -18,7 +18,7 @@ use garage_model::s3::object_table::ObjectFilter;
use crate::admin::error::*;
use crate::admin::key::ApiBucketKeyPerm;
-use crate::admin::parse_json_body;
+use crate::helpers::parse_json_body;
pub async fn handle_list_buckets(garage: &Arc<Garage>) -> Result<Response<Body>, Error> {
let buckets = garage
@@ -333,7 +333,7 @@ pub async fn handle_delete_bucket(
)
.await?;
if !objects.is_empty() {
- return Err(Error::bad_request("Bucket is not empty"));
+ return Err(Error::BucketNotEmpty);
}
// --- done checking, now commit ---
diff --git a/src/api/admin/cluster.rs b/src/api/admin/cluster.rs
index db4d968d..91d99d8a 100644
--- a/src/api/admin/cluster.rs
+++ b/src/api/admin/cluster.rs
@@ -14,7 +14,7 @@ use garage_rpc::layout::*;
use garage_model::garage::Garage;
use crate::admin::error::*;
-use crate::admin::parse_json_body;
+use crate::helpers::parse_json_body;
pub async fn handle_get_cluster_status(garage: &Arc<Garage>) -> Result<Response<Body>, Error> {
let res = GetClusterStatusResponse {
diff --git a/src/api/admin/error.rs b/src/api/admin/error.rs
index 1f49fed5..1d68dc69 100644
--- a/src/api/admin/error.rs
+++ b/src/api/admin/error.rs
@@ -40,10 +40,6 @@ pub enum Error {
/// Bucket name is not valid according to AWS S3 specs
#[error(display = "Invalid bucket name")]
InvalidBucketName,
-
- /// The client sent a request for an action not supported by garage
- #[error(display = "Unimplemented action: {}", _0)]
- NotImplemented(String),
}
impl<T> From<T> for Error
@@ -75,7 +71,6 @@ impl ApiError for Error {
Error::NoSuchAccessKey | Error::NoSuchBucket => StatusCode::NOT_FOUND,
Error::BucketNotEmpty | Error::BucketAlreadyExists => StatusCode::CONFLICT,
Error::Forbidden(_) => StatusCode::FORBIDDEN,
- Error::NotImplemented(_) => StatusCode::NOT_IMPLEMENTED,
Error::InvalidBucketName => StatusCode::BAD_REQUEST,
}
}
diff --git a/src/api/admin/key.rs b/src/api/admin/key.rs
index e5f25601..8060bf1a 100644
--- a/src/api/admin/key.rs
+++ b/src/api/admin/key.rs
@@ -12,7 +12,7 @@ use garage_model::garage::Garage;
use garage_model::key_table::*;
use crate::admin::error::*;
-use crate::admin::parse_json_body;
+use crate::helpers::parse_json_body;
pub async fn handle_list_keys(garage: &Arc<Garage>) -> Result<Response<Body>, Error> {
let res = garage
diff --git a/src/api/admin/mod.rs b/src/api/admin/mod.rs
index 73700e6e..c4857c10 100644
--- a/src/api/admin/mod.rs
+++ b/src/api/admin/mod.rs
@@ -5,14 +5,3 @@ mod router;
mod bucket;
mod cluster;
mod key;
-
-use hyper::{Body, Request};
-use serde::Deserialize;
-
-use error::*;
-
-pub async fn parse_json_body<T: for<'de> Deserialize<'de>>(req: Request<Body>) -> Result<T, Error> {
- let body = hyper::body::to_bytes(req.into_body()).await?;
- let resp: T = serde_json::from_slice(&body).ok_or_bad_request("Invalid JSON")?;
- Ok(resp)
-}
diff --git a/src/api/common_error.rs b/src/api/common_error.rs
index eca27e6b..48106e03 100644
--- a/src/api/common_error.rs
+++ b/src/api/common_error.rs
@@ -38,6 +38,11 @@ impl CommonError {
CommonError::BadRequest(_) => StatusCode::BAD_REQUEST,
}
}
+
+
+ pub fn bad_request<M: ToString>(msg: M) -> Self {
+ CommonError::BadRequest(msg.to_string())
+ }
}
/// Trait to map error to the Bad Request error code
diff --git a/src/api/helpers.rs b/src/api/helpers.rs
index 121fbd5a..599be3f7 100644
--- a/src/api/helpers.rs
+++ b/src/api/helpers.rs
@@ -2,12 +2,7 @@ use hyper::{Body, Request};
use idna::domain_to_unicode;
use serde::Deserialize;
-use garage_util::data::*;
-
-use garage_model::garage::Garage;
-use garage_model::key_table::Key;
-
-use crate::s3::error::*;
+use crate::common_error::{*, CommonError as Error};
/// What kind of authorization is required to perform a given action
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -81,28 +76,6 @@ pub fn authority_to_host(authority: &str) -> Result<String, Error> {
authority.map(|h| domain_to_unicode(h).0)
}
-#[allow(clippy::ptr_arg)]
-pub async fn resolve_bucket(
- garage: &Garage,
- bucket_name: &String,
- api_key: &Key,
-) -> Result<Uuid, Error> {
- let api_key_params = api_key
- .state
- .as_option()
- .ok_or_internal_error("Key should not be deleted at this point")?;
-
- if let Some(Some(bucket_id)) = api_key_params.local_aliases.get(bucket_name) {
- Ok(*bucket_id)
- } else {
- Ok(garage
- .bucket_helper()
- .resolve_global_bucket_name(bucket_name)
- .await?
- .ok_or(Error::NoSuchBucket)?)
- }
-}
-
/// Extract the bucket name and the key name from an HTTP path and possibly a bucket provided in
/// the host header of the request
///
diff --git a/src/api/k2v/api_server.rs b/src/api/k2v/api_server.rs
index 9b4ad882..38ef8d45 100644
--- a/src/api/k2v/api_server.rs
+++ b/src/api/k2v/api_server.rs
@@ -100,7 +100,7 @@ impl ApiHandler for K2VApiServer {
"k2v",
)?;
- let bucket_id = resolve_bucket(&garage, &bucket_name, &api_key).await?;
+ let bucket_id = garage.bucket_helper().resolve_bucket(&bucket_name, &api_key).await?;
let bucket = garage
.bucket_table
.get(&EmptyKey, &bucket_id)
diff --git a/src/api/s3/api_server.rs b/src/api/s3/api_server.rs
index 77ac3879..6b565fd0 100644
--- a/src/api/s3/api_server.rs
+++ b/src/api/s3/api_server.rs
@@ -149,7 +149,7 @@ impl ApiHandler for S3ApiServer {
return handle_create_bucket(&garage, req, content_sha256, api_key, bucket_name).await;
}
- let bucket_id = resolve_bucket(&garage, &bucket_name, &api_key).await?;
+ let bucket_id = garage.bucket_helper().resolve_bucket(&bucket_name, &api_key).await?;
let bucket = garage
.bucket_table
.get(&EmptyKey, &bucket_id)
diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs
index abd90f4a..2468678e 100644
--- a/src/api/s3/copy.rs
+++ b/src/api/s3/copy.rs
@@ -19,7 +19,7 @@ use garage_model::s3::object_table::*;
use garage_model::s3::version_table::*;
use crate::s3::error::*;
-use crate::helpers::{parse_bucket_key, resolve_bucket};
+use crate::helpers::{parse_bucket_key};
use crate::s3::put::{decode_upload_id, get_headers};
use crate::s3::xml::{self as s3_xml, xmlns_tag};
@@ -413,7 +413,7 @@ async fn get_copy_source(
let copy_source = percent_encoding::percent_decode_str(copy_source).decode_utf8()?;
let (source_bucket, source_key) = parse_bucket_key(&copy_source, None)?;
- let source_bucket_id = resolve_bucket(garage, &source_bucket.to_string(), api_key).await?;
+ let source_bucket_id = garage.bucket_helper().resolve_bucket(&source_bucket.to_string(), api_key).await?;
if !api_key.allow_read(&source_bucket_id) {
return Err(Error::Forbidden(format!(
diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs
index 343aa366..c4b63452 100644
--- a/src/api/s3/post_object.rs
+++ b/src/api/s3/post_object.rs
@@ -15,7 +15,6 @@ use serde::Deserialize;
use garage_model::garage::Garage;
use crate::s3::error::*;
-use crate::helpers::resolve_bucket;
use crate::s3::put::{get_headers, save_stream};
use crate::s3::xml as s3_xml;
use crate::signature::payload::{parse_date, verify_v4};
@@ -129,7 +128,7 @@ pub async fn handle_post_object(
)
.await?;
- let bucket_id = resolve_bucket(&garage, &bucket, &api_key).await?;
+ let bucket_id = garage.bucket_helper().resolve_bucket(&bucket, &api_key).await?;
if !api_key.allow_write(&bucket_id) {
return Err(Error::Forbidden(
diff --git a/src/model/helper/bucket.rs b/src/model/helper/bucket.rs
index 788bf3a6..2f1c6ae9 100644
--- a/src/model/helper/bucket.rs
+++ b/src/model/helper/bucket.rs
@@ -6,6 +6,7 @@ use garage_util::time::*;
use crate::bucket_alias_table::*;
use crate::bucket_table::*;
+use crate::key_table::*;
use crate::garage::Garage;
use crate::helper::error::*;
use crate::helper::key::KeyHelper;
@@ -49,6 +50,27 @@ impl<'a> BucketHelper<'a> {
}
}
+ #[allow(clippy::ptr_arg)]
+ pub async fn resolve_bucket(
+ &self,
+ bucket_name: &String,
+ api_key: &Key,
+ ) -> Result<Uuid, Error> {
+ let api_key_params = api_key
+ .state
+ .as_option()
+ .ok_or_message("Key should not be deleted at this point")?;
+
+ if let Some(Some(bucket_id)) = api_key_params.local_aliases.get(bucket_name) {
+ Ok(*bucket_id)
+ } else {
+ Ok(self.
+ resolve_global_bucket_name(bucket_name)
+ .await?
+ .ok_or_else(|| Error::NoSuchBucket(bucket_name.to_string()))?)
+ }
+ }
+
/// Returns a Bucket if it is present in bucket table,
/// even if it is in deleted state. Querying a non-existing
/// bucket ID returns an internal error.