From: Casey Bodley Date: Sat, 26 Apr 2025 02:46:12 +0000 (-0400) Subject: rgw/multisite: rgw_forward_request_to_master() preserves Error responses X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=637bdd7ca5ffadf2e2f9823e5687329cb3d68b5d;p=ceph.git rgw/multisite: rgw_forward_request_to_master() preserves Error responses when a forwarded request fails on the master zone, the local zone should return that same error response back to the client. this means reproducing both the http error and the aws xml response rgw_forward_request_to_master() stores these errors in s->err, and set_req_state_err() now avoids overwriting existing an error Fixes: https://tracker.ceph.com/issues/71098 Signed-off-by: Casey Bodley --- diff --git a/src/rgw/driver/rados/rgw_bucket.cc b/src/rgw/driver/rados/rgw_bucket.cc index 42c5aea82db0f..b760ac18b8b30 100644 --- a/src/rgw/driver/rados/rgw_bucket.cc +++ b/src/rgw/driver/rados/rgw_bucket.cc @@ -1476,8 +1476,9 @@ int RGWBucketAdminOp::remove_bucket(rgw::sal::Driver* driver, const rgw::SiteCon env.set("REQUEST_URI", delpath); env.set("QUERY_STRING", fmt::format("bucket={}&tenant={}", bucket->get_name(), bucket->get_tenant())); req_info req(dpp->get_cct(), &env); + rgw_err err; // unused - ret = rgw_forward_request_to_master(dpp, site, bucket->get_owner(), nullptr, nullptr, req, y); + ret = rgw_forward_request_to_master(dpp, site, bucket->get_owner(), nullptr, nullptr, req, err, y); if (ret < 0) { ldpp_dout(dpp, 0) << "ERROR: failed to forward request to master zonegroup: " << ret << dendl; diff --git a/src/rgw/driver/rados/rgw_rest_bucket.cc b/src/rgw/driver/rados/rgw_rest_bucket.cc index bdbafe2d1ebed..d38c081070251 100644 --- a/src/rgw/driver/rados/rgw_rest_bucket.cc +++ b/src/rgw/driver/rados/rgw_rest_bucket.cc @@ -153,7 +153,7 @@ void RGWOp_Bucket_Link::execute(optional_yield y) op_state.set_new_bucket_name(new_bucket_name); op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -192,7 +192,7 @@ void RGWOp_Bucket_Unlink::execute(optional_yield y) op_state.set_bucket_name(bucket); op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; diff --git a/src/rgw/driver/rados/rgw_rest_user.cc b/src/rgw/driver/rados/rgw_rest_user.cc index 71bb8c4c0390c..ba539e4d16de7 100644 --- a/src/rgw/driver/rados/rgw_rest_user.cc +++ b/src/rgw/driver/rados/rgw_rest_user.cc @@ -28,7 +28,7 @@ int fetch_access_keys_from_master(const DoutPrefixProvider* dpp, req_state* s, bufferlist data; JSONParser jp; int ret = rgw_forward_request_to_master(dpp, *s->penv.site, s->user->get_id(), - &data, &jp, s->info, y); + &data, &jp, s->info, s->err, y); if (ret < 0) { ldpp_dout(dpp, 0) << "forward_request_to_master returned ret=" << ret << dendl; return ret; @@ -480,7 +480,7 @@ void RGWOp_User_Remove::execute(optional_yield y) op_state.set_purge_data(purge_data); op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -555,7 +555,7 @@ void RGWOp_Subuser_Create::execute(optional_yield y) op_state.set_key_type(key_type); op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -622,7 +622,7 @@ void RGWOp_Subuser_Modify::execute(optional_yield y) op_state.set_key_type(key_type); op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -665,7 +665,7 @@ void RGWOp_Subuser_Remove::execute(optional_yield y) op_state.set_purge_keys(); op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -812,7 +812,7 @@ void RGWOp_Caps_Add::execute(optional_yield y) op_state.set_caps(caps); op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -850,7 +850,7 @@ void RGWOp_Caps_Remove::execute(optional_yield y) op_state.set_caps(caps); op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index 32e474250b44b..b23010d67b77b 100644 --- a/src/rgw/rgw_common.cc +++ b/src/rgw/rgw_common.cc @@ -337,6 +337,9 @@ void set_req_state_err(struct rgw_err& err, /* out */ err_no = -err_no; err.ret = -err_no; + if (!err.err_code.empty()) { // request already set the error + return; + } if (prot_flags & RGW_REST_SWIFT) { if (search_err(rgw_http_swift_errors, err_no, err.http_ret, err.err_code)) diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 925a85e6691d8..793263e4a5109 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -120,11 +120,35 @@ static constexpr auto S3_EXISTING_OBJTAG = "s3:ExistingObjectTag"; static constexpr auto S3_RESOURCE_TAG = "s3:ResourceTag"; static constexpr auto S3_RUNTIME_RESOURCE_VAL = "${s3:ResourceTag"; +// try to parse the xml response body +bool parse_aws_s3_error(const std::string& input, rgw_err& err) +{ + RGWXMLParser parser; + if (!parser.init()) { + return false; + } + if (!parser.parse(input.c_str(), input.length(), 1)) { + return false; + } + auto error = parser.find_first("Error"); + if (!error) { + return false; + } + if (auto code = error->find_first("Code"); code) { + err.err_code = code->get_data(); + } + if (auto message = error->find_first("Message"); message) { + err.message = message->get_data(); + } + return true; +} + int rgw_forward_request_to_master(const DoutPrefixProvider* dpp, const rgw::SiteConfig& site, const rgw_owner& effective_owner, bufferlist* indata, JSONParser* jp, - req_info& req, optional_yield y) + const req_info& req, rgw_err& err, + optional_yield y) { const auto& period = site.get_period(); if (!period) { @@ -160,7 +184,11 @@ int rgw_forward_request_to_master(const DoutPrefixProvider* dpp, if (!result) { return result.error(); } - int ret = rgw_http_error_to_errno(*result); + err.http_ret = *result; + if (err.is_err() && outdata.length()) { // 4xx or 5xx + std::ignore = parse_aws_s3_error(rgw_bl_str(outdata), err); + } + int ret = rgw_http_error_to_errno(err.http_ret); if (ret < 0) { return ret; } @@ -1309,7 +1337,7 @@ void RGWPutBucketTags::execute(optional_yield y) return; op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &in_data, nullptr, s->info, y); + &in_data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -1344,7 +1372,7 @@ int RGWDeleteBucketTags::verify_permission(optional_yield y) void RGWDeleteBucketTags::execute(optional_yield y) { op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -1405,7 +1433,7 @@ void RGWPutBucketReplication::execute(optional_yield y) { return; op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &in_data, nullptr, s->info, y); + &in_data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -1451,7 +1479,7 @@ int RGWDeleteBucketReplication::verify_permission(optional_yield y) void RGWDeleteBucketReplication::execute(optional_yield y) { op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -2936,7 +2964,7 @@ void RGWSetBucketVersioning::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &in_data, nullptr, s->info, y); + &in_data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -3034,7 +3062,7 @@ void RGWSetBucketWebsite::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &in_data, nullptr, s->info, y); + &in_data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << " forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -3080,7 +3108,7 @@ void RGWDeleteBucketWebsite::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "NOTICE: forward_to_master failed on bucket=" << s->bucket->get_name() << "returned err=" << op_ret << dendl; @@ -3734,7 +3762,7 @@ void RGWCreateBucket::execute(optional_yield y) // apply bucket creation on the master zone first JSONParser jp; op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &in_data, &jp, s->info, y); + &in_data, &jp, s->info, s->err, y); if (op_ret < 0) { return; } @@ -3868,7 +3896,7 @@ void RGWDeleteBucket::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { if (op_ret == -ENOENT) { /* adjust error, we want to return with NoSuchBucket and not @@ -6222,7 +6250,7 @@ void RGWPutACLs::execute(optional_yield y) // forward bucket acl requests to meta master zone if ((rgw::sal::Object::empty(s->object.get()))) { op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &data, nullptr, s->info, y); + &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -6355,7 +6383,7 @@ void RGWPutLC::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &data, nullptr, s->info, y); + &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -6372,7 +6400,7 @@ void RGWPutLC::execute(optional_yield y) void RGWDeleteLC::execute(optional_yield y) { op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -6434,7 +6462,7 @@ void RGWPutCORS::execute(optional_yield y) return; op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &in_data, nullptr, s->info, y); + &in_data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -6464,7 +6492,7 @@ int RGWDeleteCORS::verify_permission(optional_yield y) void RGWDeleteCORS::execute(optional_yield y) { op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -6595,7 +6623,7 @@ void RGWSetRequestPayment::execute(optional_yield y) return; op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &in_data, nullptr, s->info, y); + &in_data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -7634,8 +7662,9 @@ bool RGWBulkDelete::Deleter::delete_single(const acct_path_t& path, optional_yie req_info req = s->info; forward_req_info(dpp, s->cct, req, path.bucket_name); + rgw_err err; // unused ret = rgw_forward_request_to_master(dpp, *s->penv.site, s->owner.id, - nullptr, nullptr, req, y); + nullptr, nullptr, req, err, y); if (ret < 0) { goto delop_fail; } @@ -7891,7 +7920,7 @@ int RGWBulkUploadOp::handle_dir(const std::string_view path, optional_yield y) forward_req_info(this, s->cct, req, bucket_name); ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &in_data, &jp, req, y); + &in_data, &jp, req, s->err, y); if (ret < 0) { return ret; } @@ -8568,7 +8597,7 @@ void RGWPutBucketPolicy::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &data, nullptr, s->info, y); + &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 20) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -8695,7 +8724,7 @@ int RGWDeleteBucketPolicy::verify_permission(optional_yield y) void RGWDeleteBucketPolicy::execute(optional_yield y) { op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -8768,7 +8797,7 @@ void RGWPutBucketObjectLock::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &data, nullptr, s->info, y); + &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 20) << __func__ << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -9146,7 +9175,7 @@ void RGWPutBucketPublicAccessBlock::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &data, nullptr, s->info, y); + &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -9228,7 +9257,7 @@ int RGWDeleteBucketPublicAccessBlock::verify_permission(optional_yield y) void RGWDeleteBucketPublicAccessBlock::execute(optional_yield y) { op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -9284,7 +9313,7 @@ void RGWPutBucketEncryption::execute(optional_yield y) } op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - &data, nullptr, s->info, y); + &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 20) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -9339,7 +9368,7 @@ int RGWDeleteBucketEncryption::verify_permission(optional_yield y) void RGWDeleteBucketEncryption::execute(optional_yield y) { op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id, - nullptr, nullptr, s->info, y); + nullptr, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index 6e01911b83c2b..87d2dfccfb8e5 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -74,7 +74,8 @@ int rgw_forward_request_to_master(const DoutPrefixProvider* dpp, const rgw::SiteConfig& site, const rgw_owner& effective_owner, bufferlist* indata, JSONParser* jp, - req_info& req, optional_yield y); + const req_info& req, rgw_err& err, + optional_yield y); int rgw_op_get_bucket_policy_from_attr(const DoutPrefixProvider *dpp, CephContext *cct, diff --git a/src/rgw/rgw_rest_account.cc b/src/rgw/rgw_rest_account.cc index 86328450ab93f..6a63adfbd7024 100644 --- a/src/rgw/rgw_rest_account.cc +++ b/src/rgw/rgw_rest_account.cc @@ -75,7 +75,7 @@ void RGWOp_Account_Create::execute(optional_yield y) bufferlist data; JSONParser parser; op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - &data, &parser, s->info, y); + &data, &parser, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -116,7 +116,7 @@ void RGWOp_Account_Modify::execute(optional_yield y) { bufferlist data; op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - &data, nullptr, s->info, y); + &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; @@ -205,7 +205,7 @@ void RGWOp_Account_Delete::execute(optional_yield y) { bufferlist data; op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), - &data, nullptr, s->info, y); + &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; return; diff --git a/src/rgw/rgw_rest_pubsub.cc b/src/rgw/rgw_rest_pubsub.cc index a78351d4e832c..102bf7f33e6b7 100644 --- a/src/rgw/rgw_rest_pubsub.cc +++ b/src/rgw/rgw_rest_pubsub.cc @@ -404,7 +404,7 @@ void RGWPSCreateTopicOp::execute(optional_yield y) { // master request will replicate the topic creation. if (should_forward_request_to_master(s, driver)) { op_ret = rgw_forward_request_to_master( - this, *s->penv.site, s->owner.id, &bl_post_body, nullptr, s->info, y); + this, *s->penv.site, s->owner.id, &bl_post_body, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 4) << "CreateTopic forward_request_to_master returned ret = " << op_ret @@ -871,7 +871,7 @@ class RGWPSSetTopicAttributesOp : public RGWOp { void RGWPSSetTopicAttributesOp::execute(optional_yield y) { if (should_forward_request_to_master(s, driver)) { op_ret = rgw_forward_request_to_master( - this, *s->penv.site, s->owner.id, &bl_post_body, nullptr, s->info, y); + this, *s->penv.site, s->owner.id, &bl_post_body, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 4) << "SetTopicAttributes forward_request_to_master returned ret = " @@ -1015,7 +1015,7 @@ class RGWPSDeleteTopicOp : public RGWOp { void RGWPSDeleteTopicOp::execute(optional_yield y) { if (should_forward_request_to_master(s, driver)) { op_ret = rgw_forward_request_to_master( - this, *s->penv.site, s->owner.id, &bl_post_body, nullptr, s->info, y); + this, *s->penv.site, s->owner.id, &bl_post_body, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 1) @@ -1268,7 +1268,7 @@ int RGWPSCreateNotifOp::verify_permission(optional_yield y) { void RGWPSCreateNotifOp::execute(optional_yield y) { if (should_forward_request_to_master(s, driver)) { op_ret = rgw_forward_request_to_master( - this, *s->penv.site, s->owner.id, &data, nullptr, s->info, y); + this, *s->penv.site, s->owner.id, &data, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 4) << "CreateBucketNotification " "forward_request_to_master returned ret = " @@ -1471,7 +1471,7 @@ void RGWPSDeleteNotifOp::execute(optional_yield y) { if (should_forward_request_to_master(s, driver)) { bufferlist indata; op_ret = rgw_forward_request_to_master( - this, *s->penv.site, s->owner.id, &indata, nullptr, s->info, y); + this, *s->penv.site, s->owner.id, &indata, nullptr, s->info, s->err, y); if (op_ret < 0) { ldpp_dout(this, 4) << "DeleteBucketNotification " "forward_request_to_master returned error ret= "