don't send empty 404 on GetBucketCORS/GetBucketLifecycle (#1378)

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1378
Reviewed-by: Alex <lx@deuxfleurs.fr>
Co-authored-by: trinity-1686a <trinity@deuxfleurs.fr>
Co-committed-by: trinity-1686a <trinity@deuxfleurs.fr>

src/api/s3/cors.rs

@@ -28,9 +28,7 @@ pub async fn handle_get_cors(ctx: ReqCtx) -> Result<Response<ResBody>, Error> {
 			.header(http::header::CONTENT_TYPE, "application/xml")
 			.body(string_body(xml))?)
 	} else {
-		Ok(Response::builder()
-			.status(StatusCode::NOT_FOUND)
-			.body(empty_body())?)
+		Err(Error::NoSuchCORSConfiguration)
 	}
 }
 

src/api/s3/error.rs

@@ -42,6 +42,14 @@ pub enum Error {
 	#[error("Upload not found")]
 	NoSuchUpload,
 
+	/// CORS configuration doesn't exist for this bucket
+	#[error("The CORS configuration does not exist")]
+	NoSuchCORSConfiguration,
+
+	/// CORS configuration doesn't exist for this bucket
+	#[error("The lifecycle configuration does not exist")]
+	NoSuchLifecycleConfiguration,
+
 	/// Precondition failed (e.g. x-amz-copy-source-if-match)
 	#[error("At least one of the preconditions you specified did not hold")]
 	PreconditionFailed,
@@ -151,6 +159,8 @@ impl Error {
 			Error::InvalidDigest(_) => "InvalidDigest",
 			Error::InvalidUtf8Str(_) | Error::InvalidUtf8String(_) => "InvalidRequest",
 			Error::InvalidEncryptionAlgorithm(_) => "InvalidEncryptionAlgorithmError",
+			Error::NoSuchCORSConfiguration => "NoSuchCORSConfiguration",
+			Error::NoSuchLifecycleConfiguration => "NoSuchLifecycleConfiguration",
 		}
 	}
 }
@@ -160,7 +170,10 @@ impl ApiError for Error {
 	fn http_status_code(&self) -> StatusCode {
 		match self {
 			Error::Common(c) => c.http_status_code(),
-			Error::NoSuchKey | Error::NoSuchUpload => StatusCode::NOT_FOUND,
+			Error::NoSuchKey
+			| Error::NoSuchUpload
+			| Error::NoSuchCORSConfiguration
+			| Error::NoSuchLifecycleConfiguration => StatusCode::NOT_FOUND,
 			Error::PreconditionFailed => StatusCode::PRECONDITION_FAILED,
 			Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE,
 			Error::NotImplemented(_) => StatusCode::NOT_IMPLEMENTED,

src/api/s3/lifecycle.rs

@@ -26,9 +26,7 @@ pub async fn handle_get_lifecycle(ctx: ReqCtx) -> Result<Response<ResBody>, Erro
 			.header(http::header::CONTENT_TYPE, "application/xml")
 			.body(string_body(xml))?)
 	} else {
-		Ok(Response::builder()
-			.status(StatusCode::NOT_FOUND)
-			.body(empty_body())?)
+		Err(Error::NoSuchLifecycleConfiguration)
 	}
 }
 

src/garage/tests/s3/website.rs

@@ -4,6 +4,7 @@ use crate::json_body;
 
 use assert_json_diff::assert_json_eq;
 use aws_sdk_s3::{
+	error::ProvideErrorMetadata,
 	primitives::ByteStream,
 	types::{
 		Condition, CorsConfiguration, CorsRule, ErrorDocument, IndexDocument, Protocol, Redirect,
@@ -381,15 +382,17 @@ async fn test_website_s3_api() {
 		.unwrap();
 
 	// Check CORS are deleted from the API
-	// @FIXME check what is the expected behavior when GetBucketCors is called on a bucket without
-	// any CORS.
-	assert!(ctx
-		.client
-		.get_bucket_cors()
-		.bucket(&bucket)
-		.send()
-		.await
-		.is_err());
+	assert_eq!(
+		ctx.client
+			.get_bucket_cors()
+			.bucket(&bucket)
+			.send()
+			.await
+			.unwrap_err()
+			.into_service_error()
+			.code(),
+		Some("NoSuchCORSConfiguration")
+	);
 
 	// Test CORS are not sent anymore on a previously allowed request
 	{