First version of admin API (#298)

**Spec:** - [x] Start writing - [x] Specify all layout endpoints - [x] Specify all endpoints for operations on keys - [x] Specify all endpoints for operations on key/bucket permissions - [x] Specify all endpoints for operations on buckets - [x] Specify all endpoints for operations on bucket aliases View rendered spec at <https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/admin-api/doc/drafts/admin-api.md> **Code:** - [x] Refactor code for admin api to use common api code that was created for K2V **General endpoints:** - [x] Metrics - [x] GetClusterStatus - [x] ConnectClusterNodes - [x] GetClusterLayout - [x] UpdateClusterLayout - [x] ApplyClusterLayout - [x] RevertClusterLayout **Key-related endpoints:** - [x] ListKeys - [x] CreateKey - [x] ImportKey - [x] GetKeyInfo - [x] UpdateKey - [x] DeleteKey **Bucket-related endpoints:** - [x] ListBuckets - [x] CreateBucket - [x] GetBucketInfo - [x] DeleteBucket - [x] PutBucketWebsite - [x] DeleteBucketWebsite **Operations on key/bucket permissions:** - [x] BucketAllowKey - [x] BucketDenyKey **Operations on bucket aliases:** - [x] GlobalAliasBucket - [x] GlobalUnaliasBucket - [x] LocalAliasBucket - [x] LocalUnaliasBucket **And also:** - [x] Separate error type for the admin API (this PR includes a quite big refactoring of error handling) - [x] Add management of website access - [ ] Check that nothing is missing wrt what can be done using the CLI - [ ] Improve formatting of the spec - [x] Make sure everyone is cool with the API design Fix #231 Fix #295 Co-authored-by: Alex Auvolat <alex@adnab.me> Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/298 Co-authored-by: Alex <alex@adnab.me> Co-committed-by: Alex <alex@adnab.me>
2026-08-27 23:47:29 +00:00 · 2022-05-24 12:16:39 +02:00
parent 64c193e3db
commit 382e74c798
65 changed files with 3253 additions and 986 deletions
@@ -8,14 +8,13 @@ use hyper::{Body, Method, Request, Response};

 use opentelemetry::{trace::SpanRef, KeyValue};

-use garage_table::util::*;
 use garage_util::error::Error as GarageError;

 use garage_model::garage::Garage;
 use garage_model::key_table::Key;

-use crate::error::*;
 use crate::generic_server::*;
+use crate::s3::error::*;

 use crate::signature::payload::check_payload_signature;
 use crate::signature::streaming::*;
@@ -75,6 +74,7 @@ impl ApiHandler for S3ApiServer {
 	const API_NAME_DISPLAY: &'static str = "S3";

 	type Endpoint = S3ApiEndpoint;
+	type Error = Error;

 	fn parse_endpoint(&self, req: &Request<Body>) -> Result<S3ApiEndpoint, Error> {
 		let authority = req
@@ -122,9 +122,8 @@ impl ApiHandler for S3ApiServer {
 		}

 		let (api_key, mut content_sha256) = check_payload_signature(&garage, "s3", &req).await?;
-		let api_key = api_key.ok_or_else(|| {
-			Error::Forbidden("Garage does not support anonymous access yet".to_string())
-		})?;
+		let api_key = api_key
+			.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?;

 		let req = parse_streaming_body(
 			&api_key,
@@ -148,13 +147,14 @@ 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)
-			.await?
-			.filter(|b| !b.state.is_deleted())
-			.ok_or(Error::NoSuchBucket)?;
+			.bucket_helper()
+			.get_existing_bucket(bucket_id)
+			.await?;

 		let allowed = match endpoint.authorization_type() {
 			Authorization::Read => api_key.allow_read(&bucket_id),
@@ -164,9 +164,7 @@ impl ApiHandler for S3ApiServer {
 		};

 		if !allowed {
-			return Err(Error::Forbidden(
-				"Operation is not allowed for this key.".to_string(),
-			));
+			return Err(Error::forbidden("Operation is not allowed for this key."));
 		}

 		// Look up what CORS rule might apply to response.
@@ -309,7 +307,7 @@ impl ApiHandler for S3ApiServer {
 					)
 					.await
 				} else {
-					Err(Error::BadRequest(format!(
+					Err(Error::bad_request(format!(
 						"Invalid endpoint: list-type={}",
 						list_type
 					)))
@@ -8,13 +8,13 @@ use garage_model::bucket_table::Bucket;
 use garage_model::garage::Garage;
 use garage_model::key_table::Key;
 use garage_model::permission::BucketKeyPerm;
-use garage_model::s3::object_table::ObjectFilter;
 use garage_table::util::*;
 use garage_util::crdt::*;
 use garage_util::data::*;
 use garage_util::time::*;

-use crate::error::*;
+use crate::common_error::CommonError;
+use crate::s3::error::*;
 use crate::s3::xml as s3_xml;
 use crate::signature::verify_signed_content;

@@ -130,7 +130,7 @@ pub async fn handle_create_bucket(

 	if let Some(location_constraint) = cmd {
 		if location_constraint != garage.config.s3_api.s3_region {
-			return Err(Error::BadRequest(format!(
+			return Err(Error::bad_request(format!(
 				"Cannot satisfy location constraint `{}`: buckets can only be created in region `{}`",
 				location_constraint,
 				garage.config.s3_api.s3_region
@@ -158,12 +158,12 @@ pub async fn handle_create_bucket(
 		// otherwise return a forbidden error.
 		let kp = api_key.bucket_permissions(&bucket_id);
 		if !(kp.allow_write || kp.allow_owner) {
-			return Err(Error::BucketAlreadyExists);
+			return Err(CommonError::BucketAlreadyExists.into());
 		}
 	} else {
 		// Create the bucket!
 		if !is_valid_bucket_name(&bucket_name) {
-			return Err(Error::BadRequest(format!(
+			return Err(Error::bad_request(format!(
 				"{}: {}",
 				bucket_name, INVALID_BUCKET_NAME_MESSAGE
 			)));
@@ -228,18 +228,8 @@ pub async fn handle_delete_bucket(
 		// Delete bucket

 		// Check bucket is empty
-		let objects = garage
-			.object_table
-			.get_range(
-				&bucket_id,
-				None,
-				Some(ObjectFilter::IsData),
-				10,
-				EnumerationOrder::Forward,
-			)
-			.await?;
-		if !objects.is_empty() {
-			return Err(Error::BucketNotEmpty);
+		if !garage.bucket_helper().is_bucket_empty(bucket_id).await? {
+			return Err(CommonError::BucketNotEmpty.into());
 		}

 		// --- done checking, now commit ---
@@ -18,8 +18,8 @@ use garage_model::s3::block_ref_table::*;
 use garage_model::s3::object_table::*;
 use garage_model::s3::version_table::*;

-use crate::error::*;
-use crate::helpers::{parse_bucket_key, resolve_bucket};
+use crate::helpers::parse_bucket_key;
+use crate::s3::error::*;
 use crate::s3::put::{decode_upload_id, get_headers};
 use crate::s3::xml::{self as s3_xml, xmlns_tag};

@@ -201,8 +201,8 @@ pub async fn handle_upload_part_copy(
 			let mut ranges = http_range::HttpRange::parse(range_str, source_version_meta.size)
 				.map_err(|e| (e, source_version_meta.size))?;
 			if ranges.len() != 1 {
-				return Err(Error::BadRequest(
-					"Invalid x-amz-copy-source-range header: exactly 1 range must be given".into(),
+				return Err(Error::bad_request(
+					"Invalid x-amz-copy-source-range header: exactly 1 range must be given",
 				));
 			} else {
 				ranges.pop().unwrap()
@@ -230,8 +230,8 @@ pub async fn handle_upload_part_copy(
 			// This is only for small files, we don't bother handling this.
 			// (in AWS UploadPartCopy works for parts at least 5MB which
 			// is never the case of an inline object)
-			return Err(Error::BadRequest(
-				"Source object is too small (minimum part size is 5Mb)".into(),
+			return Err(Error::bad_request(
+				"Source object is too small (minimum part size is 5Mb)",
 			));
 		}
 		ObjectVersionData::FirstBlock(_meta, _first_block_hash) => (),
@@ -250,7 +250,7 @@ pub async fn handle_upload_part_copy(
 	// Check this part number hasn't yet been uploaded
 	if let Some(dv) = dest_version {
 		if dv.has_part_number(part_number) {
-			return Err(Error::BadRequest(format!(
+			return Err(Error::bad_request(format!(
 				"Part number {} has already been uploaded",
 				part_number
 			)));
@@ -413,10 +413,13 @@ 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!(
+		return Err(Error::forbidden(format!(
 			"Reading from bucket {} not allowed for this key",
 			source_bucket
 		)));
@@ -536,8 +539,8 @@ impl CopyPreconditionHeaders {
 			(None, None, None, Some(ims)) => v_date > *ims,
 			(None, None, None, None) => true,
 			_ => {
-				return Err(Error::BadRequest(
-					"Invalid combination of x-amz-copy-source-if-xxxxx headers".into(),
+				return Err(Error::bad_request(
+					"Invalid combination of x-amz-copy-source-if-xxxxx headers",
 				))
 			}
 		};
@@ -9,13 +9,12 @@ use hyper::{header::HeaderName, Body, Method, Request, Response, StatusCode};

 use serde::{Deserialize, Serialize};

-use crate::error::*;
+use crate::s3::error::*;
 use crate::s3::xml::{to_xml_with_header, xmlns_tag, IntValue, Value};
 use crate::signature::verify_signed_content;

 use garage_model::bucket_table::{Bucket, CorsRule as GarageCorsRule};
 use garage_model::garage::Garage;
-use garage_table::*;
 use garage_util::data::*;

 pub async fn handle_get_cors(bucket: &Bucket) -> Result<Response<Body>, Error> {
@@ -48,14 +47,11 @@ pub async fn handle_delete_cors(
 	bucket_id: Uuid,
 ) -> Result<Response<Body>, Error> {
 	let mut bucket = garage
-		.bucket_table
-		.get(&EmptyKey, &bucket_id)
-		.await?
-		.ok_or(Error::NoSuchBucket)?;
+		.bucket_helper()
+		.get_existing_bucket(bucket_id)
+		.await?;

-	let param = bucket
-		.params_mut()
-		.ok_or_internal_error("Bucket should not be deleted at this point")?;
+	let param = bucket.params_mut().unwrap();

 	param.cors_config.update(None);
 	garage.bucket_table.insert(&bucket).await?;
@@ -78,14 +74,11 @@ pub async fn handle_put_cors(
 	}

 	let mut bucket = garage
-		.bucket_table
-		.get(&EmptyKey, &bucket_id)
-		.await?
-		.ok_or(Error::NoSuchBucket)?;
+		.bucket_helper()
+		.get_existing_bucket(bucket_id)
+		.await?;

-	let param = bucket
-		.params_mut()
-		.ok_or_internal_error("Bucket should not be deleted at this point")?;
+	let param = bucket.params_mut().unwrap();

 	let conf: CorsConfiguration = from_reader(&body as &[u8])?;
 	conf.validate()?;
@@ -119,12 +112,7 @@ pub async fn handle_options_s3api(
 		let helper = garage.bucket_helper();
 		let bucket_id = helper.resolve_global_bucket_name(&bn).await?;
 		if let Some(id) = bucket_id {
-			let bucket = garage
-				.bucket_table
-				.get(&EmptyKey, &id)
-				.await?
-				.filter(|b| !b.state.is_deleted())
-				.ok_or(Error::NoSuchBucket)?;
+			let bucket = garage.bucket_helper().get_existing_bucket(id).await?;
 			handle_options_for_bucket(req, &bucket)
 		} else {
 			// If there is a bucket name in the request, but that name
@@ -185,7 +173,7 @@ pub fn handle_options_for_bucket(
 		}
 	}

-	Err(Error::Forbidden("This CORS request is not allowed.".into()))
+	Err(Error::forbidden("This CORS request is not allowed."))
 }

 pub fn find_matching_cors_rule<'a>(
@@ -8,7 +8,7 @@ use garage_util::time::*;
 use garage_model::garage::Garage;
 use garage_model::s3::object_table::*;

-use crate::error::*;
+use crate::s3::error::*;
 use crate::s3::xml as s3_xml;
 use crate::signature::verify_signed_content;

@@ -0,0 +1,207 @@
+use std::convert::TryInto;
+
+use err_derive::Error;
+use hyper::header::HeaderValue;
+use hyper::{Body, HeaderMap, StatusCode};
+
+use garage_model::helper::error::Error as HelperError;
+
+use crate::common_error::CommonError;
+pub use crate::common_error::{CommonErrorDerivative, OkOrBadRequest, OkOrInternalError};
+use crate::generic_server::ApiError;
+use crate::s3::xml as s3_xml;
+use crate::signature::error::Error as SignatureError;
+
+/// Errors of this crate
+#[derive(Debug, Error)]
+pub enum Error {
+	#[error(display = "{}", _0)]
+	/// Error from common error
+	Common(CommonError),
+
+	// Category: cannot process
+	/// Authorization Header Malformed
+	#[error(display = "Authorization header malformed, expected scope: {}", _0)]
+	AuthorizationHeaderMalformed(String),
+
+	/// The object requested don't exists
+	#[error(display = "Key not found")]
+	NoSuchKey,
+
+	/// The multipart upload requested don't exists
+	#[error(display = "Upload not found")]
+	NoSuchUpload,
+
+	/// Precondition failed (e.g. x-amz-copy-source-if-match)
+	#[error(display = "At least one of the preconditions you specified did not hold")]
+	PreconditionFailed,
+
+	/// Parts specified in CMU request do not match parts actually uploaded
+	#[error(display = "Parts given to CompleteMultipartUpload do not match uploaded parts")]
+	InvalidPart,
+
+	/// Parts given to CompleteMultipartUpload were not in ascending order
+	#[error(display = "Parts given to CompleteMultipartUpload were not in ascending order")]
+	InvalidPartOrder,
+
+	/// In CompleteMultipartUpload: not enough data
+	/// (here we are more lenient than AWS S3)
+	#[error(display = "Proposed upload is smaller than the minimum allowed object size")]
+	EntityTooSmall,
+
+	// Category: bad request
+	/// The request contained an invalid UTF-8 sequence in its path or in other parameters
+	#[error(display = "Invalid UTF-8: {}", _0)]
+	InvalidUtf8Str(#[error(source)] std::str::Utf8Error),
+
+	/// The request used an invalid path
+	#[error(display = "Invalid UTF-8: {}", _0)]
+	InvalidUtf8String(#[error(source)] std::string::FromUtf8Error),
+
+	/// The client sent invalid XML data
+	#[error(display = "Invalid XML: {}", _0)]
+	InvalidXml(String),
+
+	/// The client sent a header with invalid value
+	#[error(display = "Invalid header value: {}", _0)]
+	InvalidHeader(#[error(source)] hyper::header::ToStrError),
+
+	/// The client sent a range header with invalid value
+	#[error(display = "Invalid HTTP range: {:?}", _0)]
+	InvalidRange(#[error(from)] (http_range::HttpRangeParseError, u64)),
+
+	/// 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
+where
+	CommonError: From<T>,
+{
+	fn from(err: T) -> Self {
+		Error::Common(CommonError::from(err))
+	}
+}
+
+impl CommonErrorDerivative for Error {}
+
+impl From<HelperError> for Error {
+	fn from(err: HelperError) -> Self {
+		match err {
+			HelperError::Internal(i) => Self::Common(CommonError::InternalError(i)),
+			HelperError::BadRequest(b) => Self::Common(CommonError::BadRequest(b)),
+			HelperError::InvalidBucketName(n) => Self::Common(CommonError::InvalidBucketName(n)),
+			HelperError::NoSuchBucket(n) => Self::Common(CommonError::NoSuchBucket(n)),
+			e => Self::bad_request(format!("{}", e)),
+		}
+	}
+}
+
+impl From<roxmltree::Error> for Error {
+	fn from(err: roxmltree::Error) -> Self {
+		Self::InvalidXml(format!("{}", err))
+	}
+}
+
+impl From<quick_xml::de::DeError> for Error {
+	fn from(err: quick_xml::de::DeError) -> Self {
+		Self::InvalidXml(format!("{}", err))
+	}
+}
+
+impl From<SignatureError> for Error {
+	fn from(err: SignatureError) -> Self {
+		match err {
+			SignatureError::Common(c) => Self::Common(c),
+			SignatureError::AuthorizationHeaderMalformed(c) => {
+				Self::AuthorizationHeaderMalformed(c)
+			}
+			SignatureError::InvalidUtf8Str(i) => Self::InvalidUtf8Str(i),
+			SignatureError::InvalidHeader(h) => Self::InvalidHeader(h),
+		}
+	}
+}
+
+impl From<multer::Error> for Error {
+	fn from(err: multer::Error) -> Self {
+		Self::bad_request(err)
+	}
+}
+
+impl Error {
+	pub fn aws_code(&self) -> &'static str {
+		match self {
+			Error::Common(c) => c.aws_code(),
+			Error::NoSuchKey => "NoSuchKey",
+			Error::NoSuchUpload => "NoSuchUpload",
+			Error::PreconditionFailed => "PreconditionFailed",
+			Error::InvalidPart => "InvalidPart",
+			Error::InvalidPartOrder => "InvalidPartOrder",
+			Error::EntityTooSmall => "EntityTooSmall",
+			Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed",
+			Error::NotImplemented(_) => "NotImplemented",
+			Error::InvalidXml(_) => "MalformedXML",
+			Error::InvalidRange(_) => "InvalidRange",
+			Error::InvalidUtf8Str(_) | Error::InvalidUtf8String(_) | Error::InvalidHeader(_) => {
+				"InvalidRequest"
+			}
+		}
+	}
+}
+
+impl ApiError for Error {
+	/// Get the HTTP status code that best represents the meaning of the error for the client
+	fn http_status_code(&self) -> StatusCode {
+		match self {
+			Error::Common(c) => c.http_status_code(),
+			Error::NoSuchKey | Error::NoSuchUpload => StatusCode::NOT_FOUND,
+			Error::PreconditionFailed => StatusCode::PRECONDITION_FAILED,
+			Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE,
+			Error::NotImplemented(_) => StatusCode::NOT_IMPLEMENTED,
+			Error::AuthorizationHeaderMalformed(_)
+			| Error::InvalidPart
+			| Error::InvalidPartOrder
+			| Error::EntityTooSmall
+			| Error::InvalidXml(_)
+			| Error::InvalidUtf8Str(_)
+			| Error::InvalidUtf8String(_)
+			| Error::InvalidHeader(_) => StatusCode::BAD_REQUEST,
+		}
+	}
+
+	fn add_http_headers(&self, header_map: &mut HeaderMap<HeaderValue>) {
+		use hyper::header;
+		#[allow(clippy::single_match)]
+		match self {
+			Error::InvalidRange((_, len)) => {
+				header_map.append(
+					header::CONTENT_RANGE,
+					format!("bytes */{}", len)
+						.try_into()
+						.expect("header value only contain ascii"),
+				);
+			}
+			_ => (),
+		}
+	}
+
+	fn http_body(&self, garage_region: &str, path: &str) -> Body {
+		let error = s3_xml::Error {
+			code: s3_xml::Value(self.aws_code().to_string()),
+			message: s3_xml::Value(format!("{}", self)),
+			resource: Some(s3_xml::Value(path.to_string())),
+			region: Some(s3_xml::Value(garage_region.to_string())),
+		};
+		Body::from(s3_xml::to_xml_with_header(&error).unwrap_or_else(|_| {
+			r#"
+<?xml version="1.0" encoding="UTF-8"?>
+<Error>
+	<Code>InternalError</Code>
+	<Message>XML encoding of error failed</Message>
+</Error>
+			"#
+			.into()
+		}))
+	}
+}
@@ -17,7 +17,7 @@ use garage_model::garage::Garage;
 use garage_model::s3::object_table::*;
 use garage_model::s3::version_table::*;

-use crate::error::*;
+use crate::s3::error::*;

 const X_AMZ_MP_PARTS_COUNT: &str = "x-amz-mp-parts-count";

@@ -210,8 +210,8 @@ pub async fn handle_get(

 	match (part_number, parse_range_header(req, last_v_meta.size)?) {
 		(Some(_), Some(_)) => {
-			return Err(Error::BadRequest(
-				"Cannot specify both partNumber and Range header".into(),
+			return Err(Error::bad_request(
+				"Cannot specify both partNumber and Range header",
 			));
 		}
 		(Some(pn), None) => {
@@ -302,9 +302,9 @@ async fn handle_get_range(
 				let body: Body = Body::from(bytes[begin as usize..end as usize].to_vec());
 				Ok(resp_builder.body(body)?)
 			} else {
-				None.ok_or_internal_error(
+				Err(Error::internal_error(
 					"Requested range not present in inline bytes when it should have been",
-				)
+				))
 			}
 		}
 		ObjectVersionData::FirstBlock(_meta, _first_block_hash) => {
@@ -16,8 +16,8 @@ use garage_model::s3::version_table::Version;
 use garage_table::{EmptyKey, EnumerationOrder};

 use crate::encoding::*;
-use crate::error::*;
 use crate::helpers::key_after_prefix;
+use crate::s3::error::*;
 use crate::s3::put as s3_put;
 use crate::s3::xml as s3_xml;

@@ -582,13 +582,19 @@ impl ListObjectsQuery {
 				// representing the key to start with.
 				(Some(token), _) => match &token[..1] {
 					"[" => Ok(RangeBegin::IncludingKey {
-						key: String::from_utf8(base64::decode(token[1..].as_bytes())?)?,
+						key: String::from_utf8(
+							base64::decode(token[1..].as_bytes())
+								.ok_or_bad_request("Invalid continuation token")?,
+						)?,
 						fallback_key: None,
 					}),
 					"]" => Ok(RangeBegin::AfterKey {
-						key: String::from_utf8(base64::decode(token[1..].as_bytes())?)?,
+						key: String::from_utf8(
+							base64::decode(token[1..].as_bytes())
+								.ok_or_bad_request("Invalid continuation token")?,
+						)?,
 					}),
-					_ => Err(Error::BadRequest("Invalid continuation token".to_string())),
+					_ => Err(Error::bad_request("Invalid continuation token")),
 				},

 				// StartAfter has defined semantics in the spec:
@@ -1,4 +1,5 @@
 pub mod api_server;
+pub mod error;

 mod bucket;
 mod copy;
@@ -14,8 +14,7 @@ use serde::Deserialize;

 use garage_model::garage::Garage;

-use crate::error::*;
-use crate::helpers::resolve_bucket;
+use crate::s3::error::*;
 use crate::s3::put::{get_headers, save_stream};
 use crate::s3::xml as s3_xml;
 use crate::signature::payload::{parse_date, verify_v4};
@@ -48,9 +47,7 @@ pub async fn handle_post_object(
 		let field = if let Some(field) = multipart.next_field().await? {
 			field
 		} else {
-			return Err(Error::BadRequest(
-				"Request did not contain a file".to_owned(),
-			));
+			return Err(Error::bad_request("Request did not contain a file"));
 		};
 		let name: HeaderName = if let Some(Ok(name)) = field.name().map(TryInto::try_into) {
 			name
@@ -66,14 +63,14 @@ pub async fn handle_post_object(
 				"tag" => (/* tag need to be reencoded, but we don't support them yet anyway */),
 				"acl" => {
 					if params.insert("x-amz-acl", content).is_some() {
-						return Err(Error::BadRequest(
-							"Field 'acl' provided more than one time".to_string(),
+						return Err(Error::bad_request(
+							"Field 'acl' provided more than one time",
 						));
 					}
 				}
 				_ => {
 					if params.insert(&name, content).is_some() {
-						return Err(Error::BadRequest(format!(
+						return Err(Error::bad_request(format!(
 							"Field '{}' provided more than one time",
 							name
 						)));
@@ -90,9 +87,7 @@ pub async fn handle_post_object(
 		.to_str()?;
 	let credential = params
 		.get("x-amz-credential")
-		.ok_or_else(|| {
-			Error::Forbidden("Garage does not support anonymous access yet".to_string())
-		})?
+		.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?
 		.to_str()?;
 	let policy = params
 		.get("policy")
@@ -129,15 +124,16 @@ 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(
-			"Operation is not allowed for this key.".to_string(),
-		));
+		return Err(Error::forbidden("Operation is not allowed for this key."));
 	}

-	let decoded_policy = base64::decode(&policy)?;
+	let decoded_policy = base64::decode(&policy).ok_or_bad_request("Invalid policy")?;
 	let decoded_policy: Policy =
 		serde_json::from_slice(&decoded_policy).ok_or_bad_request("Invalid policy")?;

@@ -145,9 +141,7 @@ pub async fn handle_post_object(
 		.ok_or_bad_request("Invalid expiration date")?
 		.into();
 	if Utc::now() - expiration > Duration::zero() {
-		return Err(Error::BadRequest(
-			"Expiration date is in the paste".to_string(),
-		));
+		return Err(Error::bad_request("Expiration date is in the paste"));
 	}

 	let mut conditions = decoded_policy.into_conditions()?;
@@ -159,7 +153,7 @@ pub async fn handle_post_object(
 			"policy" | "x-amz-signature" => (), // this is always accepted, as it's required to validate other fields
 			"content-type" => {
 				let conds = conditions.params.remove("content-type").ok_or_else(|| {
-					Error::BadRequest(format!("Key '{}' is not allowed in policy", param_key))
+					Error::bad_request(format!("Key '{}' is not allowed in policy", param_key))
 				})?;
 				for cond in conds {
 					let ok = match cond {
@@ -169,7 +163,7 @@ pub async fn handle_post_object(
 						}
 					};
 					if !ok {
-						return Err(Error::BadRequest(format!(
+						return Err(Error::bad_request(format!(
 							"Key '{}' has value not allowed in policy",
 							param_key
 						)));
@@ -178,7 +172,7 @@ pub async fn handle_post_object(
 			}
 			"key" => {
 				let conds = conditions.params.remove("key").ok_or_else(|| {
-					Error::BadRequest(format!("Key '{}' is not allowed in policy", param_key))
+					Error::bad_request(format!("Key '{}' is not allowed in policy", param_key))
 				})?;
 				for cond in conds {
 					let ok = match cond {
@@ -186,7 +180,7 @@ pub async fn handle_post_object(
 						Operation::StartsWith(s) => key.starts_with(&s),
 					};
 					if !ok {
-						return Err(Error::BadRequest(format!(
+						return Err(Error::bad_request(format!(
 							"Key '{}' has value not allowed in policy",
 							param_key
 						)));
@@ -201,7 +195,7 @@ pub async fn handle_post_object(
 					continue;
 				}
 				let conds = conditions.params.remove(&param_key).ok_or_else(|| {
-					Error::BadRequest(format!("Key '{}' is not allowed in policy", param_key))
+					Error::bad_request(format!("Key '{}' is not allowed in policy", param_key))
 				})?;
 				for cond in conds {
 					let ok = match cond {
@@ -209,7 +203,7 @@ pub async fn handle_post_object(
 						Operation::StartsWith(s) => value.to_str()?.starts_with(s.as_str()),
 					};
 					if !ok {
-						return Err(Error::BadRequest(format!(
+						return Err(Error::bad_request(format!(
 							"Key '{}' has value not allowed in policy",
 							param_key
 						)));
@@ -220,7 +214,7 @@ pub async fn handle_post_object(
 	}

 	if let Some((param_key, _)) = conditions.params.iter().next() {
-		return Err(Error::BadRequest(format!(
+		return Err(Error::bad_request(format!(
 			"Key '{}' is required in policy, but no value was provided",
 			param_key
 		)));
@@ -326,7 +320,7 @@ impl Policy {
 			match condition {
 				PolicyCondition::Equal(map) => {
 					if map.len() != 1 {
-						return Err(Error::BadRequest("Invalid policy item".to_owned()));
+						return Err(Error::bad_request("Invalid policy item"));
 					}
 					let (mut k, v) = map.into_iter().next().expect("size was verified");
 					k.make_ascii_lowercase();
@@ -334,7 +328,7 @@ impl Policy {
 				}
 				PolicyCondition::OtherOp([cond, mut key, value]) => {
 					if key.remove(0) != '$' {
-						return Err(Error::BadRequest("Invalid policy item".to_owned()));
+						return Err(Error::bad_request("Invalid policy item"));
 					}
 					key.make_ascii_lowercase();
 					match cond.as_str() {
@@ -347,7 +341,7 @@ impl Policy {
 								.or_default()
 								.push(Operation::StartsWith(value));
 						}
-						_ => return Err(Error::BadRequest("Invalid policy item".to_owned())),
+						_ => return Err(Error::bad_request("Invalid policy item")),
 					}
 				}
 				PolicyCondition::SizeRange(key, min, max) => {
@@ -355,7 +349,7 @@ impl Policy {
 						length.0 = length.0.max(min);
 						length.1 = length.1.min(max);
 					} else {
-						return Err(Error::BadRequest("Invalid policy item".to_owned()));
+						return Err(Error::bad_request("Invalid policy item"));
 					}
 				}
 			}
@@ -420,15 +414,15 @@ where
 				self.read += bytes.len() as u64;
 				// optimization to fail early when we know before the end it's too long
 				if self.length.end() < &self.read {
-					return Poll::Ready(Some(Err(Error::BadRequest(
-						"File size does not match policy".to_owned(),
+					return Poll::Ready(Some(Err(Error::bad_request(
+						"File size does not match policy",
 					))));
 				}
 			}
 			Poll::Ready(None) => {
 				if !self.length.contains(&self.read) {
-					return Poll::Ready(Some(Err(Error::BadRequest(
-						"File size does not match policy".to_owned(),
+					return Poll::Ready(Some(Err(Error::bad_request(
+						"File size does not match policy",
 					))));
 				}
 			}
@@ -19,7 +19,7 @@ use garage_model::s3::block_ref_table::*;
 use garage_model::s3::object_table::*;
 use garage_model::s3::version_table::*;

-use crate::error::*;
+use crate::s3::error::*;
 use crate::s3::xml as s3_xml;
 use crate::signature::verify_signed_content;

@@ -183,8 +183,8 @@ fn ensure_checksum_matches(
 ) -> Result<(), Error> {
 	if let Some(expected_sha256) = content_sha256 {
 		if expected_sha256 != data_sha256sum {
-			return Err(Error::BadRequest(
-				"Unable to validate x-amz-content-sha256".to_string(),
+			return Err(Error::bad_request(
+				"Unable to validate x-amz-content-sha256",
 			));
 		} else {
 			trace!("Successfully validated x-amz-content-sha256");
@@ -192,9 +192,7 @@ fn ensure_checksum_matches(
 	}
 	if let Some(expected_md5) = content_md5 {
 		if expected_md5.trim_matches('"') != base64::encode(data_md5sum) {
-			return Err(Error::BadRequest(
-				"Unable to validate content-md5".to_string(),
-			));
+			return Err(Error::bad_request("Unable to validate content-md5"));
 		} else {
 			trace!("Successfully validated content-md5");
 		}
@@ -428,7 +426,7 @@ pub async fn handle_put_part(
 	// Check part hasn't already been uploaded
 	if let Some(v) = version {
 		if v.has_part_number(part_number) {
-			return Err(Error::BadRequest(format!(
+			return Err(Error::bad_request(format!(
 				"Part number {} has already been uploaded",
 				part_number
 			)));
@@ -513,7 +511,7 @@ pub async fn handle_complete_multipart_upload(

 	let version = version.ok_or(Error::NoSuchKey)?;
 	if version.blocks.is_empty() {
-		return Err(Error::BadRequest("No data was uploaded".to_string()));
+		return Err(Error::bad_request("No data was uploaded"));
 	}

 	let headers = match object_version.state {
@@ -574,8 +572,8 @@ pub async fn handle_complete_multipart_upload(
 		.map(|x| x.part_number)
 		.eq(block_parts.into_iter());
 	if !same_parts {
-		return Err(Error::BadRequest(
-			"Part numbers in block list and part list do not match. This can happen if a part was partially uploaded. Please abort the multipart upload and try again.".into(),
+		return Err(Error::bad_request(
+			"Part numbers in block list and part list do not match. This can happen if a part was partially uploaded. Please abort the multipart upload and try again."
 		));
 	}

@@ -1,5 +1,3 @@
-use crate::error::{Error, OkOrBadRequest};
-
 use std::borrow::Cow;

 use hyper::header::HeaderValue;
@@ -7,6 +5,7 @@ use hyper::{HeaderMap, Method, Request};

 use crate::helpers::Authorization;
 use crate::router_macros::{generateQueryParameters, router_match};
+use crate::s3::error::*;

 router_match! {@func

@@ -343,7 +342,7 @@ impl Endpoint {
 			Method::POST => Self::from_post(key, &mut query)?,
 			Method::PUT => Self::from_put(key, &mut query, req.headers())?,
 			Method::DELETE => Self::from_delete(key, &mut query)?,
-			_ => return Err(Error::BadRequest("Unknown method".to_owned())),
+			_ => return Err(Error::bad_request("Unknown method")),
 		};

 		if let Some(message) = query.nonempty_message() {
@@ -4,13 +4,12 @@ use std::sync::Arc;
 use hyper::{Body, Request, Response, StatusCode};
 use serde::{Deserialize, Serialize};

-use crate::error::*;
+use crate::s3::error::*;
 use crate::s3::xml::{to_xml_with_header, xmlns_tag, IntValue, Value};
 use crate::signature::verify_signed_content;

 use garage_model::bucket_table::*;
 use garage_model::garage::Garage;
-use garage_table::*;
 use garage_util::data::*;

 pub async fn handle_get_website(bucket: &Bucket) -> Result<Response<Body>, Error> {
@@ -47,14 +46,11 @@ pub async fn handle_delete_website(
 	bucket_id: Uuid,
 ) -> Result<Response<Body>, Error> {
 	let mut bucket = garage
-		.bucket_table
-		.get(&EmptyKey, &bucket_id)
-		.await?
-		.ok_or(Error::NoSuchBucket)?;
+		.bucket_helper()
+		.get_existing_bucket(bucket_id)
+		.await?;

-	let param = bucket
-		.params_mut()
-		.ok_or_internal_error("Bucket should not be deleted at this point")?;
+	let param = bucket.params_mut().unwrap();

 	param.website_config.update(None);
 	garage.bucket_table.insert(&bucket).await?;
@@ -77,14 +73,11 @@ pub async fn handle_put_website(
 	}

 	let mut bucket = garage
-		.bucket_table
-		.get(&EmptyKey, &bucket_id)
-		.await?
-		.ok_or(Error::NoSuchBucket)?;
+		.bucket_helper()
+		.get_existing_bucket(bucket_id)
+		.await?;

-	let param = bucket
-		.params_mut()
-		.ok_or_internal_error("Bucket should not be deleted at this point")?;
+	let param = bucket.params_mut().unwrap();

 	let conf: WebsiteConfiguration = from_reader(&body as &[u8])?;
 	conf.validate()?;
@@ -176,8 +169,8 @@ impl WebsiteConfiguration {
 				|| self.index_document.is_some()
 				|| self.routing_rules.is_some())
 		{
-			return Err(Error::BadRequest(
-				"Bad XML: can't have RedirectAllRequestsTo and other fields".to_owned(),
+			return Err(Error::bad_request(
+				"Bad XML: can't have RedirectAllRequestsTo and other fields",
 			));
 		}
 		if let Some(ref ed) = self.error_document {
@@ -222,8 +215,8 @@ impl WebsiteConfiguration {
 impl Key {
 	pub fn validate(&self) -> Result<(), Error> {
 		if self.key.0.is_empty() {
-			Err(Error::BadRequest(
-				"Bad XML: error document specified but empty".to_owned(),
+			Err(Error::bad_request(
+				"Bad XML: error document specified but empty",
 			))
 		} else {
 			Ok(())
@@ -234,8 +227,8 @@ impl Key {
 impl Suffix {
 	pub fn validate(&self) -> Result<(), Error> {
 		if self.suffix.0.is_empty() | self.suffix.0.contains('/') {
-			Err(Error::BadRequest(
-				"Bad XML: index document is empty or contains /".to_owned(),
+			Err(Error::bad_request(
+				"Bad XML: index document is empty or contains /",
 			))
 		} else {
 			Ok(())
@@ -247,7 +240,7 @@ impl Target {
 	pub fn validate(&self) -> Result<(), Error> {
 		if let Some(ref protocol) = self.protocol {
 			if protocol.0 != "http" && protocol.0 != "https" {
-				return Err(Error::BadRequest("Bad XML: invalid protocol".to_owned()));
+				return Err(Error::bad_request("Bad XML: invalid protocol"));
 			}
 		}
 		Ok(())
@@ -269,19 +262,19 @@ impl Redirect {
 	pub fn validate(&self, has_prefix: bool) -> Result<(), Error> {
 		if self.replace_prefix.is_some() {
 			if self.replace_full.is_some() {
-				return Err(Error::BadRequest(
-					"Bad XML: both ReplaceKeyPrefixWith and ReplaceKeyWith are set".to_owned(),
+				return Err(Error::bad_request(
+					"Bad XML: both ReplaceKeyPrefixWith and ReplaceKeyWith are set",
 				));
 			}
 			if !has_prefix {
-				return Err(Error::BadRequest(
-					"Bad XML: ReplaceKeyPrefixWith is set, but  KeyPrefixEquals isn't".to_owned(),
+				return Err(Error::bad_request(
+					"Bad XML: ReplaceKeyPrefixWith is set, but  KeyPrefixEquals isn't",
 				));
 			}
 		}
 		if let Some(ref protocol) = self.protocol {
 			if protocol.0 != "http" && protocol.0 != "https" {
-				return Err(Error::BadRequest("Bad XML: invalid protocol".to_owned()));
+				return Err(Error::bad_request("Bad XML: invalid protocol"));
 			}
 		}
 		// TODO there are probably more invalide cases, but which ones?
@@ -1,7 +1,7 @@
 use quick_xml::se::to_string;
 use serde::{Deserialize, Serialize, Serializer};

-use crate::Error as ApiError;
+use crate::s3::error::Error as ApiError;

 pub fn to_xml_with_header<T: Serialize>(x: &T) -> Result<String, ApiError> {
 	let mut xml = r#"<?xml version="1.0" encoding="UTF-8"?>"#.to_string();