mirror of
https://github.com/rustfs/rustfs.git
synced 2026-09-02 02:08:41 +00:00
fix(tier): harden reference proof and audit output (#6807)
Validate lifecycle tier references through the tier reference proof path, preserve S3 list CommonPrefix XML compatibility, and make GetObject audit completion use real S3 error status codes. Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -3820,7 +3820,15 @@ impl DefaultObjectUsecase {
|
||||
Box::pin(self.execute_get_object_inner(req))
|
||||
}
|
||||
|
||||
fn complete_get_object_error<T>(helper: OperationHelper, err: S3Error) -> S3Result<S3Response<T>> {
|
||||
let result = Err(err);
|
||||
let _ = helper.complete(&result);
|
||||
result
|
||||
}
|
||||
|
||||
async fn execute_get_object_inner(&self, req: S3Request<GetObjectInput>) -> S3Result<S3Response<GetObjectOutput>> {
|
||||
let helper = OperationHelper::new(&req, EventName::ObjectAccessedGet, S3Operation::GetObject).suppress_event();
|
||||
|
||||
if let Some(context) = &self.context {
|
||||
let _ = context.object_store();
|
||||
}
|
||||
@@ -3840,7 +3848,10 @@ impl DefaultObjectUsecase {
|
||||
context.start_time.elapsed().as_secs_f64(),
|
||||
);
|
||||
}
|
||||
let bootstrap = self.init_get_object_bootstrap(&req.input.bucket, &req.input.key, &request_id)?;
|
||||
let bootstrap = match self.init_get_object_bootstrap(&req.input.bucket, &req.input.key, &request_id) {
|
||||
Ok(bootstrap) => bootstrap,
|
||||
Err(err) => return Self::complete_get_object_error(helper, err),
|
||||
};
|
||||
record_get_object_s3_handler_stage_duration(GET_OBJECT_STAGE_REQUEST_SHAPE, request_shape_start);
|
||||
let timeout_config = bootstrap.timeout_config;
|
||||
let wrapper = bootstrap.wrapper;
|
||||
@@ -3848,7 +3859,6 @@ impl DefaultObjectUsecase {
|
||||
let concurrent_requests = bootstrap.concurrent_requests;
|
||||
let mut lifecycle = GetObjectBodyLifecycle::tracked(bootstrap.request_guard);
|
||||
|
||||
let helper = OperationHelper::new(&req, EventName::ObjectAccessedGet, S3Operation::GetObject).suppress_event();
|
||||
// mc get 3
|
||||
|
||||
// Cheap request-shape validations run first so invalid requests keep
|
||||
@@ -3858,7 +3868,7 @@ impl DefaultObjectUsecase {
|
||||
Ok(validated) => validated,
|
||||
Err(err) => {
|
||||
lifecycle.finish_err();
|
||||
return Err(err);
|
||||
return Self::complete_get_object_error(helper, err);
|
||||
}
|
||||
};
|
||||
record_get_object_s3_handler_stage_duration(GET_OBJECT_STAGE_REQUEST_VALIDATION, request_validation_start);
|
||||
@@ -3875,7 +3885,10 @@ impl DefaultObjectUsecase {
|
||||
let store_lookup_start = stage_metrics_enabled.then(std::time::Instant::now);
|
||||
let Some(store) = self.object_store() else {
|
||||
lifecycle.finish_err();
|
||||
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
|
||||
return Self::complete_get_object_error(
|
||||
helper,
|
||||
S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()),
|
||||
);
|
||||
};
|
||||
if let Some(store_lookup_start) = store_lookup_start {
|
||||
rustfs_io_metrics::record_get_object_stage_duration(
|
||||
@@ -3887,7 +3900,7 @@ impl DefaultObjectUsecase {
|
||||
let bucket_validation_start = stage_metrics_enabled.then(std::time::Instant::now);
|
||||
if let Err(err) = validate_bucket_exists(&store, &req.input.bucket).await {
|
||||
lifecycle.finish_err();
|
||||
return Err(err);
|
||||
return Self::complete_get_object_error(helper, err);
|
||||
}
|
||||
record_get_object_s3_handler_stage_duration(GET_OBJECT_STAGE_BUCKET_VALIDATION, bucket_validation_start);
|
||||
|
||||
@@ -3896,7 +3909,7 @@ impl DefaultObjectUsecase {
|
||||
Ok(request_context) => request_context,
|
||||
Err(err) => {
|
||||
lifecycle.finish_err();
|
||||
return Err(err);
|
||||
return Self::complete_get_object_error(helper, err);
|
||||
}
|
||||
};
|
||||
if let Some(request_context_start) = request_context_start {
|
||||
@@ -3951,7 +3964,7 @@ impl DefaultObjectUsecase {
|
||||
return result;
|
||||
}
|
||||
lifecycle.finish_err();
|
||||
return Err(err);
|
||||
return Self::complete_get_object_error(helper.version_id(version_id_for_event), err);
|
||||
}
|
||||
};
|
||||
let GetObjectPreparedRead { io_planning, read_setup } = prepared_read;
|
||||
@@ -4040,7 +4053,7 @@ impl DefaultObjectUsecase {
|
||||
.await;
|
||||
let output_context = match output_context {
|
||||
Ok(output_context) => output_context,
|
||||
Err(err) => return Err(err),
|
||||
Err(err) => return Self::complete_get_object_error(helper.version_id(version_id_for_event), err),
|
||||
};
|
||||
if let Some(output_build_start) = output_build_start {
|
||||
rustfs_io_metrics::record_get_object_stage_duration(
|
||||
|
||||
@@ -23,7 +23,7 @@ use http::StatusCode;
|
||||
use metrics::counter;
|
||||
use rustfs_audit::{
|
||||
ObjectVersion,
|
||||
entity::{ApiDetails, ApiDetailsBuilder, AuditEntryBuilder},
|
||||
entity::{ApiDetailsBuilder, AuditEntryBuilder},
|
||||
global::AuditLogger,
|
||||
};
|
||||
use rustfs_io_metrics::record_s3_op;
|
||||
@@ -136,7 +136,7 @@ impl OperationHelper {
|
||||
|
||||
record_s3_op(op);
|
||||
|
||||
// Fast path: when both chains are disabled, avoid all request parsing/builder work.
|
||||
// Fast path: when both chains are disabled, avoid audit/notify builder work.
|
||||
if !audit_enabled && !notify_enabled {
|
||||
return Self::Disabled;
|
||||
}
|
||||
@@ -180,7 +180,7 @@ impl OperationHelper {
|
||||
|
||||
let audit_builder = if audit_enabled {
|
||||
Some(
|
||||
AuditEntryBuilder::new("1.0", event, trigger, ApiDetails::default())
|
||||
AuditEntryBuilder::new("1.0", event, trigger, api_builder.clone().build())
|
||||
.remote_host(remote_host)
|
||||
.user_agent(get_request_user_agent(&req.headers))
|
||||
.req_host(get_request_host(&req.headers))
|
||||
@@ -453,8 +453,8 @@ mod tests {
|
||||
use rustfs_s3_ops::S3Operation;
|
||||
use rustfs_s3_types::EventName;
|
||||
use rustfs_utils::http::headers::{AMZ_REQUEST_ID, REQUEST_ID_HEADER};
|
||||
use s3s::dto::{DeleteObjectTaggingInput, DeleteObjectTaggingOutput};
|
||||
use s3s::{S3Request, S3Response};
|
||||
use s3s::dto::{DeleteObjectTaggingInput, DeleteObjectTaggingOutput, GetObjectInput, GetObjectOutput};
|
||||
use s3s::{S3Error, S3ErrorCode, S3Request, S3Response};
|
||||
use std::sync::{Arc, Mutex};
|
||||
use temp_env::{async_with_vars, with_vars};
|
||||
|
||||
@@ -615,6 +615,71 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn operation_helper_initializes_audit_api_details_before_completion() {
|
||||
with_vars(
|
||||
[
|
||||
(rustfs_config::ENV_NOTIFY_ENABLE, Some("false")),
|
||||
(rustfs_config::ENV_AUDIT_ENABLE, Some("true")),
|
||||
],
|
||||
|| {
|
||||
refresh_notify_module_enabled();
|
||||
refresh_audit_module_enabled();
|
||||
|
||||
let input = GetObjectInput::builder()
|
||||
.bucket("audit-bucket".to_string())
|
||||
.key("missing/object.txt".to_string())
|
||||
.build()
|
||||
.expect("get object input should build");
|
||||
let req = build_request(input, Method::GET, Uri::from_static("/audit-bucket/missing/object.txt"));
|
||||
let mut helper = OperationHelper::new(&req, EventName::ObjectAccessedGet, S3Operation::GetObject);
|
||||
|
||||
let OperationHelper::Enabled(state) = &mut helper else {
|
||||
panic!("helper should be enabled when the audit switch is on");
|
||||
};
|
||||
let audit_entry = state.audit_builder.take().expect("audit builder should exist").build();
|
||||
|
||||
assert_eq!(audit_entry.api.name.as_deref(), Some("s3:GetObject"));
|
||||
assert_eq!(audit_entry.api.bucket.as_deref(), Some("audit-bucket"));
|
||||
assert_eq!(audit_entry.api.object.as_deref(), Some("missing/object.txt"));
|
||||
assert!(audit_entry.api.status_code.is_none());
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn operation_helper_complete_records_failed_status_code() {
|
||||
with_vars(
|
||||
[
|
||||
(rustfs_config::ENV_NOTIFY_ENABLE, Some("false")),
|
||||
(rustfs_config::ENV_AUDIT_ENABLE, Some("true")),
|
||||
],
|
||||
|| {
|
||||
refresh_notify_module_enabled();
|
||||
refresh_audit_module_enabled();
|
||||
|
||||
let input = GetObjectInput::builder()
|
||||
.bucket("audit-bucket".to_string())
|
||||
.key("missing/object.txt".to_string())
|
||||
.build()
|
||||
.expect("get object input should build");
|
||||
let req = build_request(input, Method::GET, Uri::from_static("/audit-bucket/missing/object.txt"));
|
||||
let result: Result<S3Response<GetObjectOutput>, S3Error> = Err(S3Error::new(S3ErrorCode::NoSuchKey));
|
||||
let mut helper =
|
||||
OperationHelper::new(&req, EventName::ObjectAccessedGet, S3Operation::GetObject).complete(&result);
|
||||
|
||||
let OperationHelper::Enabled(state) = &mut helper else {
|
||||
panic!("helper should be enabled when the audit switch is on");
|
||||
};
|
||||
let audit_entry = state.audit_builder.take().expect("audit builder should exist").build();
|
||||
|
||||
assert_eq!(audit_entry.api.name.as_deref(), Some("s3:GetObject"));
|
||||
assert_eq!(audit_entry.api.status.as_deref(), Some("failure"));
|
||||
assert_eq!(audit_entry.api.status_code, Some(404));
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn operation_helper_prioritizes_request_context_for_request_id() {
|
||||
with_vars(
|
||||
|
||||
Reference in New Issue
Block a user