Skip to content

Commit 637bdd7

Browse files
cbodleycbodley
committed
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 <Error> 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 <[email protected]>
1 parent 36583eb commit 637bdd7

File tree

8 files changed

+79
-45
lines changed

8 files changed

+79
-45
lines changed

‎src/rgw/driver/rados/rgw_bucket.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1476,8 +1476,9 @@ int RGWBucketAdminOp::remove_bucket(rgw::sal::Driver* driver, const rgw::SiteCon
14761476
env.set("REQUEST_URI", delpath);
14771477
env.set("QUERY_STRING", fmt::format("bucket={}&tenant={}", bucket->get_name(), bucket->get_tenant()));
14781478
req_info req(dpp->get_cct(), &env);
1479+
rgw_err err; // unused
14791480

1480-
ret = rgw_forward_request_to_master(dpp, site, bucket->get_owner(), nullptr, nullptr, req, y);
1481+
ret = rgw_forward_request_to_master(dpp, site, bucket->get_owner(), nullptr, nullptr, req, err, y);
14811482
if (ret < 0) {
14821483
ldpp_dout(dpp, 0) << "ERROR: failed to forward request to master zonegroup: "
14831484
<< ret << dendl;

‎src/rgw/driver/rados/rgw_rest_bucket.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ void RGWOp_Bucket_Link::execute(optional_yield y)
153153
op_state.set_new_bucket_name(new_bucket_name);
154154

155155
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(),
156-
nullptr, nullptr, s->info, y);
156+
nullptr, nullptr, s->info, s->err, y);
157157
if (op_ret < 0) {
158158
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
159159
return;
@@ -192,7 +192,7 @@ void RGWOp_Bucket_Unlink::execute(optional_yield y)
192192
op_state.set_bucket_name(bucket);
193193

194194
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(),
195-
nullptr, nullptr, s->info, y);
195+
nullptr, nullptr, s->info, s->err, y);
196196
if (op_ret < 0) {
197197
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
198198
return;

‎src/rgw/driver/rados/rgw_rest_user.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ int fetch_access_keys_from_master(const DoutPrefixProvider* dpp, req_state* s,
2828
bufferlist data;
2929
JSONParser jp;
3030
int ret = rgw_forward_request_to_master(dpp, *s->penv.site, s->user->get_id(),
31-
&data, &jp, s->info, y);
31+
&data, &jp, s->info, s->err, y);
3232
if (ret < 0) {
3333
ldpp_dout(dpp, 0) << "forward_request_to_master returned ret=" << ret << dendl;
3434
return ret;
@@ -480,7 +480,7 @@ void RGWOp_User_Remove::execute(optional_yield y)
480480
op_state.set_purge_data(purge_data);
481481

482482
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(),
483-
nullptr, nullptr, s->info, y);
483+
nullptr, nullptr, s->info, s->err, y);
484484
if (op_ret < 0) {
485485
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
486486
return;
@@ -555,7 +555,7 @@ void RGWOp_Subuser_Create::execute(optional_yield y)
555555
op_state.set_key_type(key_type);
556556

557557
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(),
558-
nullptr, nullptr, s->info, y);
558+
nullptr, nullptr, s->info, s->err, y);
559559
if (op_ret < 0) {
560560
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
561561
return;
@@ -622,7 +622,7 @@ void RGWOp_Subuser_Modify::execute(optional_yield y)
622622
op_state.set_key_type(key_type);
623623

624624
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(),
625-
nullptr, nullptr, s->info, y);
625+
nullptr, nullptr, s->info, s->err, y);
626626
if (op_ret < 0) {
627627
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
628628
return;
@@ -665,7 +665,7 @@ void RGWOp_Subuser_Remove::execute(optional_yield y)
665665
op_state.set_purge_keys();
666666

667667
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(),
668-
nullptr, nullptr, s->info, y);
668+
nullptr, nullptr, s->info, s->err, y);
669669
if (op_ret < 0) {
670670
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
671671
return;
@@ -812,7 +812,7 @@ void RGWOp_Caps_Add::execute(optional_yield y)
812812
op_state.set_caps(caps);
813813

814814
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(),
815-
nullptr, nullptr, s->info, y);
815+
nullptr, nullptr, s->info, s->err, y);
816816
if (op_ret < 0) {
817817
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
818818
return;
@@ -850,7 +850,7 @@ void RGWOp_Caps_Remove::execute(optional_yield y)
850850
op_state.set_caps(caps);
851851

852852
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(),
853-
nullptr, nullptr, s->info, y);
853+
nullptr, nullptr, s->info, s->err, y);
854854
if (op_ret < 0) {
855855
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
856856
return;

‎src/rgw/rgw_common.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,9 @@ void set_req_state_err(struct rgw_err& err, /* out */
337337
err_no = -err_no;
338338

339339
err.ret = -err_no;
340+
if (!err.err_code.empty()) { // request already set the error
341+
return;
342+
}
340343

341344
if (prot_flags & RGW_REST_SWIFT) {
342345
if (search_err(rgw_http_swift_errors, err_no, err.http_ret, err.err_code))

‎src/rgw/rgw_op.cc‎

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,35 @@ static constexpr auto S3_EXISTING_OBJTAG = "s3:ExistingObjectTag";
120120
static constexpr auto S3_RESOURCE_TAG = "s3:ResourceTag";
121121
static constexpr auto S3_RUNTIME_RESOURCE_VAL = "${s3:ResourceTag";
122122

123+
// try to parse the xml <Error> response body
124+
bool parse_aws_s3_error(const std::string& input, rgw_err& err)
125+
{
126+
RGWXMLParser parser;
127+
if (!parser.init()) {
128+
return false;
129+
}
130+
if (!parser.parse(input.c_str(), input.length(), 1)) {
131+
return false;
132+
}
133+
auto error = parser.find_first("Error");
134+
if (!error) {
135+
return false;
136+
}
137+
if (auto code = error->find_first("Code"); code) {
138+
err.err_code = code->get_data();
139+
}
140+
if (auto message = error->find_first("Message"); message) {
141+
err.message = message->get_data();
142+
}
143+
return true;
144+
}
145+
123146
int rgw_forward_request_to_master(const DoutPrefixProvider* dpp,
124147
const rgw::SiteConfig& site,
125148
const rgw_owner& effective_owner,
126149
bufferlist* indata, JSONParser* jp,
127-
req_info& req, optional_yield y)
150+
const req_info& req, rgw_err& err,
151+
optional_yield y)
128152
{
129153
const auto& period = site.get_period();
130154
if (!period) {
@@ -160,7 +184,11 @@ int rgw_forward_request_to_master(const DoutPrefixProvider* dpp,
160184
if (!result) {
161185
return result.error();
162186
}
163-
int ret = rgw_http_error_to_errno(*result);
187+
err.http_ret = *result;
188+
if (err.is_err() && outdata.length()) { // 4xx or 5xx
189+
std::ignore = parse_aws_s3_error(rgw_bl_str(outdata), err);
190+
}
191+
int ret = rgw_http_error_to_errno(err.http_ret);
164192
if (ret < 0) {
165193
return ret;
166194
}
@@ -1309,7 +1337,7 @@ void RGWPutBucketTags::execute(optional_yield y)
13091337
return;
13101338

13111339
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
1312-
&in_data, nullptr, s->info, y);
1340+
&in_data, nullptr, s->info, s->err, y);
13131341
if (op_ret < 0) {
13141342
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
13151343
return;
@@ -1344,7 +1372,7 @@ int RGWDeleteBucketTags::verify_permission(optional_yield y)
13441372
void RGWDeleteBucketTags::execute(optional_yield y)
13451373
{
13461374
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
1347-
nullptr, nullptr, s->info, y);
1375+
nullptr, nullptr, s->info, s->err, y);
13481376
if (op_ret < 0) {
13491377
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
13501378
return;
@@ -1405,7 +1433,7 @@ void RGWPutBucketReplication::execute(optional_yield y) {
14051433
return;
14061434

14071435
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
1408-
&in_data, nullptr, s->info, y);
1436+
&in_data, nullptr, s->info, s->err, y);
14091437
if (op_ret < 0) {
14101438
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
14111439
return;
@@ -1451,7 +1479,7 @@ int RGWDeleteBucketReplication::verify_permission(optional_yield y)
14511479
void RGWDeleteBucketReplication::execute(optional_yield y)
14521480
{
14531481
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
1454-
nullptr, nullptr, s->info, y);
1482+
nullptr, nullptr, s->info, s->err, y);
14551483
if (op_ret < 0) {
14561484
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
14571485
return;
@@ -2936,7 +2964,7 @@ void RGWSetBucketVersioning::execute(optional_yield y)
29362964
}
29372965

29382966
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
2939-
&in_data, nullptr, s->info, y);
2967+
&in_data, nullptr, s->info, s->err, y);
29402968
if (op_ret < 0) {
29412969
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
29422970
return;
@@ -3034,7 +3062,7 @@ void RGWSetBucketWebsite::execute(optional_yield y)
30343062
}
30353063

30363064
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
3037-
&in_data, nullptr, s->info, y);
3065+
&in_data, nullptr, s->info, s->err, y);
30383066
if (op_ret < 0) {
30393067
ldpp_dout(this, 0) << " forward_request_to_master returned ret=" << op_ret << dendl;
30403068
return;
@@ -3080,7 +3108,7 @@ void RGWDeleteBucketWebsite::execute(optional_yield y)
30803108
}
30813109

30823110
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
3083-
nullptr, nullptr, s->info, y);
3111+
nullptr, nullptr, s->info, s->err, y);
30843112
if (op_ret < 0) {
30853113
ldpp_dout(this, 0) << "NOTICE: forward_to_master failed on bucket=" << s->bucket->get_name()
30863114
<< "returned err=" << op_ret << dendl;
@@ -3734,7 +3762,7 @@ void RGWCreateBucket::execute(optional_yield y)
37343762
// apply bucket creation on the master zone first
37353763
JSONParser jp;
37363764
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
3737-
&in_data, &jp, s->info, y);
3765+
&in_data, &jp, s->info, s->err, y);
37383766
if (op_ret < 0) {
37393767
return;
37403768
}
@@ -3868,7 +3896,7 @@ void RGWDeleteBucket::execute(optional_yield y)
38683896
}
38693897

38703898
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
3871-
nullptr, nullptr, s->info, y);
3899+
nullptr, nullptr, s->info, s->err, y);
38723900
if (op_ret < 0) {
38733901
if (op_ret == -ENOENT) {
38743902
/* adjust error, we want to return with NoSuchBucket and not
@@ -6222,7 +6250,7 @@ void RGWPutACLs::execute(optional_yield y)
62226250
// forward bucket acl requests to meta master zone
62236251
if ((rgw::sal::Object::empty(s->object.get()))) {
62246252
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
6225-
&data, nullptr, s->info, y);
6253+
&data, nullptr, s->info, s->err, y);
62266254
if (op_ret < 0) {
62276255
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
62286256
return;
@@ -6355,7 +6383,7 @@ void RGWPutLC::execute(optional_yield y)
63556383
}
63566384

63576385
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
6358-
&data, nullptr, s->info, y);
6386+
&data, nullptr, s->info, s->err, y);
63596387
if (op_ret < 0) {
63606388
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
63616389
return;
@@ -6372,7 +6400,7 @@ void RGWPutLC::execute(optional_yield y)
63726400
void RGWDeleteLC::execute(optional_yield y)
63736401
{
63746402
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
6375-
nullptr, nullptr, s->info, y);
6403+
nullptr, nullptr, s->info, s->err, y);
63766404
if (op_ret < 0) {
63776405
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
63786406
return;
@@ -6434,7 +6462,7 @@ void RGWPutCORS::execute(optional_yield y)
64346462
return;
64356463

64366464
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
6437-
&in_data, nullptr, s->info, y);
6465+
&in_data, nullptr, s->info, s->err, y);
64386466
if (op_ret < 0) {
64396467
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
64406468
return;
@@ -6464,7 +6492,7 @@ int RGWDeleteCORS::verify_permission(optional_yield y)
64646492
void RGWDeleteCORS::execute(optional_yield y)
64656493
{
64666494
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
6467-
nullptr, nullptr, s->info, y);
6495+
nullptr, nullptr, s->info, s->err, y);
64686496
if (op_ret < 0) {
64696497
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
64706498
return;
@@ -6595,7 +6623,7 @@ void RGWSetRequestPayment::execute(optional_yield y)
65956623
return;
65966624

65976625
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
6598-
&in_data, nullptr, s->info, y);
6626+
&in_data, nullptr, s->info, s->err, y);
65996627
if (op_ret < 0) {
66006628
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
66016629
return;
@@ -7634,8 +7662,9 @@ bool RGWBulkDelete::Deleter::delete_single(const acct_path_t& path, optional_yie
76347662
req_info req = s->info;
76357663
forward_req_info(dpp, s->cct, req, path.bucket_name);
76367664

7665+
rgw_err err; // unused
76377666
ret = rgw_forward_request_to_master(dpp, *s->penv.site, s->owner.id,
7638-
nullptr, nullptr, req, y);
7667+
nullptr, nullptr, req, err, y);
76397668
if (ret < 0) {
76407669
goto delop_fail;
76417670
}
@@ -7891,7 +7920,7 @@ int RGWBulkUploadOp::handle_dir(const std::string_view path, optional_yield y)
78917920
forward_req_info(this, s->cct, req, bucket_name);
78927921

78937922
ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
7894-
&in_data, &jp, req, y);
7923+
&in_data, &jp, req, s->err, y);
78957924
if (ret < 0) {
78967925
return ret;
78977926
}
@@ -8568,7 +8597,7 @@ void RGWPutBucketPolicy::execute(optional_yield y)
85688597
}
85698598

85708599
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
8571-
&data, nullptr, s->info, y);
8600+
&data, nullptr, s->info, s->err, y);
85728601
if (op_ret < 0) {
85738602
ldpp_dout(this, 20) << "forward_request_to_master returned ret=" << op_ret << dendl;
85748603
return;
@@ -8695,7 +8724,7 @@ int RGWDeleteBucketPolicy::verify_permission(optional_yield y)
86958724
void RGWDeleteBucketPolicy::execute(optional_yield y)
86968725
{
86978726
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
8698-
nullptr, nullptr, s->info, y);
8727+
nullptr, nullptr, s->info, s->err, y);
86998728
if (op_ret < 0) {
87008729
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
87018730
return;
@@ -8768,7 +8797,7 @@ void RGWPutBucketObjectLock::execute(optional_yield y)
87688797
}
87698798

87708799
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
8771-
&data, nullptr, s->info, y);
8800+
&data, nullptr, s->info, s->err, y);
87728801
if (op_ret < 0) {
87738802
ldpp_dout(this, 20) << __func__ << "forward_request_to_master returned ret=" << op_ret << dendl;
87748803
return;
@@ -9146,7 +9175,7 @@ void RGWPutBucketPublicAccessBlock::execute(optional_yield y)
91469175
}
91479176

91489177
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
9149-
&data, nullptr, s->info, y);
9178+
&data, nullptr, s->info, s->err, y);
91509179
if (op_ret < 0) {
91519180
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
91529181
return;
@@ -9228,7 +9257,7 @@ int RGWDeleteBucketPublicAccessBlock::verify_permission(optional_yield y)
92289257
void RGWDeleteBucketPublicAccessBlock::execute(optional_yield y)
92299258
{
92309259
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
9231-
nullptr, nullptr, s->info, y);
9260+
nullptr, nullptr, s->info, s->err, y);
92329261
if (op_ret < 0) {
92339262
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
92349263
return;
@@ -9284,7 +9313,7 @@ void RGWPutBucketEncryption::execute(optional_yield y)
92849313
}
92859314

92869315
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
9287-
&data, nullptr, s->info, y);
9316+
&data, nullptr, s->info, s->err, y);
92889317
if (op_ret < 0) {
92899318
ldpp_dout(this, 20) << "forward_request_to_master returned ret=" << op_ret << dendl;
92909319
return;
@@ -9339,7 +9368,7 @@ int RGWDeleteBucketEncryption::verify_permission(optional_yield y)
93399368
void RGWDeleteBucketEncryption::execute(optional_yield y)
93409369
{
93419370
op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->owner.id,
9342-
nullptr, nullptr, s->info, y);
9371+
nullptr, nullptr, s->info, s->err, y);
93439372
if (op_ret < 0) {
93449373
ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl;
93459374
return;

‎src/rgw/rgw_op.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ int rgw_forward_request_to_master(const DoutPrefixProvider* dpp,
7474
const rgw::SiteConfig& site,
7575
const rgw_owner& effective_owner,
7676
bufferlist* indata, JSONParser* jp,
77-
req_info& req, optional_yield y);
77+
const req_info& req, rgw_err& err,
78+
optional_yield y);
7879

7980
int rgw_op_get_bucket_policy_from_attr(const DoutPrefixProvider *dpp,
8081
CephContext *cct,

0 commit comments

Comments
 (0)