rgw/rest: RGWRESTConn::forward() prefers to return http errors

callers need to distinguish between transport errors (a failure to
forward the request) and http errors (successfully forwarded and got a
response). forward() was losing this information by mapping any http
errors to errnos

use tl::expected to differentiate between transport errors and http
errors, with the latter being the successful/expected case

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 36583eb9bdf0692790b0b6046a00e5b060da6c93)

diff --git a/src/rgw/radosgw-admin/radosgw-admin.cc b/src/rgw/radosgw-admin/radosgw-admin.cc
index 035ed66d2d8a741f2f5192cac87372247006f597..c207a271b6811cd8e6976d8ed7e79c410bfc0125 100644 (file)
--- a/src/rgw/radosgw-admin/radosgw-admin.cc
+++ b/src/rgw/radosgw-admin/radosgw-admin.cc
@@ -1902,14 +1902,21 @@ static int send_to_remote_gateway(RGWRESTConn* conn, req_info& info,
 
   ceph::bufferlist response;
   rgw_user user;
-  int ret = conn->forward(dpp(), user, info, MAX_REST_RESPONSE, &in_data, &response, null_yield);
+  auto result = conn->forward(dpp(), user, info, MAX_REST_RESPONSE, &in_data, &response, null_yield);
+  if (!result) {
+    return result.error();
+  }
+  int ret = rgw_http_error_to_errno(*result);
+  if (ret < 0) {
+    return ret;
+  }
 
-  int parse_ret = parser.parse(response.c_str(), response.length());
-  if (parse_ret < 0) {
+  ret = parser.parse(response.c_str(), response.length());
+  if (ret < 0) {
     cerr << "failed to parse response" << std::endl;
-    return parse_ret;
+    return ret;
   }
-  return ret;
+  return 0;
 }
 
 static int send_to_url(const string& url,
@@ -1930,14 +1937,21 @@ static int send_to_url(const string& url,
   RGWRESTSimpleRequest req(g_ceph_context, info.method, url, NULL, &params, opt_region);
 
   bufferlist response;
-  int ret = req.forward_request(dpp(), key, info, MAX_REST_RESPONSE, &in_data, &response, null_yield);
+  auto result = req.forward_request(dpp(), key, info, MAX_REST_RESPONSE, &in_data, &response, null_yield);
+  if (!result) {
+    return result.error();
+  }
+  int ret = rgw_http_error_to_errno(*result);
+  if (ret < 0) {
+    return ret;
+  }
 
-  int parse_ret = parser.parse(response.c_str(), response.length());
-  if (parse_ret < 0) {
+  ret = parser.parse(response.c_str(), response.length());
+  if (ret < 0) {
     cout << "failed to parse response" << std::endl;
-    return parse_ret;
+    return ret;
   }
-  return ret;
+  return 0;
 }
 
 static int send_to_remote_or_url(RGWRESTConn *conn, const string& url,

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 673a7fcf32517358d938df844c1be5edf9e3ccf1..3529dd077933cde6bb7a12f4a5a9d93fd921197a 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -155,8 +155,12 @@ int rgw_forward_request_to_master(const DoutPrefixProvider* dpp,
                           creds, site.get_zonegroup().id, zg->second.api_name};
   bufferlist outdata;
   constexpr size_t max_response_size = 128 * 1024; // we expect a very small response
-  int ret = conn.forward(dpp, effective_owner, req,
-                         max_response_size, indata, &outdata, y);
+  auto result = conn.forward(dpp, effective_owner, req,
+                             max_response_size, indata, &outdata, y);
+  if (!result) {
+    return result.error();
+  }
+  int ret = rgw_http_error_to_errno(*result);
   if (ret < 0) {
     return ret;
   }

diff --git a/src/rgw/rgw_period_puller.cc b/src/rgw/rgw_period_puller.cc
index 5b6d218f69cd6331b361760bb63eff7d8432e090..54137a0b0d3842b636d56899d58d228b24b08e8f 100644 (file)
--- a/src/rgw/rgw_period_puller.cc
+++ b/src/rgw/rgw_period_puller.cc
@@ -4,6 +4,7 @@
 #include "rgw_rados.h"
 #include "rgw_zone.h"
 #include "rgw_rest_conn.h"
+#include "rgw_http_errors.h"
 #include "common/ceph_json.h"
 #include "common/errno.h"
 
@@ -40,7 +41,11 @@ int pull_period(const DoutPrefixProvider *dpp, RGWRESTConn* conn, const std::str
 
   bufferlist data;
 #define MAX_REST_RESPONSE (128 * 1024)
-  int r = conn->forward(dpp, user, info, MAX_REST_RESPONSE, nullptr, &data, y);
+  auto result = conn->forward(dpp, user, info, MAX_REST_RESPONSE, nullptr, &data, y);
+  if (!result) {
+    return result.error();
+  }
+  int r = rgw_http_error_to_errno(*result);
   if (r < 0) {
     return r;
   }

diff --git a/src/rgw/rgw_rest_client.cc b/src/rgw/rgw_rest_client.cc
index c3fdd1b687d399e9944545af31b42ead89fccd38..f7116c7bae7e2295c982a87e07d11a2af1e7029f 100644 (file)
--- a/src/rgw/rgw_rest_client.cc
+++ b/src/rgw/rgw_rest_client.cc
@@ -370,7 +370,8 @@ static void scope_from_api_name(const DoutPrefixProvider *dpp,
   }
 }
 
-int RGWRESTSimpleRequest::forward_request(const DoutPrefixProvider *dpp, const RGWAccessKey& key, const req_info& info, size_t max_response, bufferlist *inbl, bufferlist *outbl, optional_yield y, std::string service)
+auto RGWRESTSimpleRequest::forward_request(const DoutPrefixProvider *dpp, const RGWAccessKey& key, const req_info& info, size_t max_response, bufferlist *inbl, bufferlist *outbl, optional_yield y, std::string service)
+  -> tl::expected<int, int>
 {
 
   string date_str;
@@ -416,7 +417,7 @@ int RGWRESTSimpleRequest::forward_request(const DoutPrefixProvider *dpp, const R
   int ret = sign_request(dpp, key, region, s, new_env, new_info, nullptr);
   if (ret < 0) {
     ldpp_dout(dpp, 0) << "ERROR: failed to sign request" << dendl;
-    return ret;
+    return tl::unexpected(ret);
   }
 
   if (s == "iam") {
@@ -458,13 +459,11 @@ int RGWRESTSimpleRequest::forward_request(const DoutPrefixProvider *dpp, const R
   method = new_info.method;
   url = new_url;
 
-  int r = process(dpp, y);
-  if (r < 0) {
-    if (http_status == 0) {
-      // no http status, generally means the service is not available
-      r = -ERR_SERVICE_UNAVAILABLE;
-    }
-    return r;
+  std::ignore = process(dpp, y);
+
+  if (http_status == 0) {
+    // no http status, generally means the service is not available
+    return tl::unexpected(-ERR_SERVICE_UNAVAILABLE);
   }
 
   response.append((char)0); /* NULL terminate response */
@@ -473,7 +472,7 @@ int RGWRESTSimpleRequest::forward_request(const DoutPrefixProvider *dpp, const R
     *outbl = std::move(response);
   }
 
-  return status;
+  return http_status;
 }
 
 class RGWRESTStreamOutCB : public RGWGetDataCB {

diff --git a/src/rgw/rgw_rest_client.h b/src/rgw/rgw_rest_client.h
index 16db7630a40ab1eb8bc52d086e0a8e3764b19e8d..c619e65fee4e5db2dcfdbc628f67854aa8b553fe 100644 (file)
--- a/src/rgw/rgw_rest_client.h
+++ b/src/rgw/rgw_rest_client.h
@@ -3,6 +3,7 @@
 
 #pragma once
 
+#include "include/expected.hpp"
 #include "rgw_http_client.h"
 
 class RGWGetDataCB;
@@ -66,7 +67,9 @@ public:
                        param_vec_t *_headers, param_vec_t *_params,
                        std::optional<std::string> _api_name) : RGWHTTPSimpleRequest(_cct, _method, _url, _headers, _params), api_name(_api_name) {}
 
-  int forward_request(const DoutPrefixProvider *dpp, const RGWAccessKey& key, const req_info& info, size_t max_response, bufferlist *inbl, bufferlist *outbl, optional_yield y, std::string service="");
+  // return the http status of the response or an error code from the transport
+  auto forward_request(const DoutPrefixProvider *dpp, const RGWAccessKey& key, const req_info& info, size_t max_response, bufferlist *inbl, bufferlist *outbl, optional_yield y, std::string service="")
+    -> tl::expected<int, int>;
 };
 
 class RGWWriteDrainCB {

diff --git a/src/rgw/rgw_rest_conn.cc b/src/rgw/rgw_rest_conn.cc
index f6b939303e0a3772c1d7885f4382fbb74089405f..56e69f3a84b0b075a6f602573e6139eab8992b78 100644 (file)
--- a/src/rgw/rgw_rest_conn.cc
+++ b/src/rgw/rgw_rest_conn.cc
@@ -3,6 +3,7 @@
 
 #include "rgw_zone.h"
 #include "rgw_rest_conn.h"
+#include "rgw_http_errors.h"
 #include "rgw_sal.h"
 #include "rgw_rados.h"
 
@@ -153,57 +154,63 @@ void RGWRESTConn::populate_params(param_vec_t& params, const rgw_owner* uid, con
   populate_zonegroup(params, zonegroup);
 }
 
-int RGWRESTConn::forward(const DoutPrefixProvider *dpp, const rgw_owner& uid, const req_info& info, size_t max_response, bufferlist *inbl, bufferlist *outbl, optional_yield y)
+auto RGWRESTConn::forward(const DoutPrefixProvider *dpp, const rgw_owner& uid,
+                          const req_info& info, size_t max_response,
+                          bufferlist *inbl, bufferlist *outbl, optional_yield y)
+  -> tl::expected<int, int>
 {
-  int ret = 0;
-
   static constexpr int NUM_ENPOINT_IOERROR_RETRIES = 20;
   for (int tries = 0; tries < NUM_ENPOINT_IOERROR_RETRIES; tries++) {
     string url;
-    ret = get_url(url);
-    if (ret < 0)
-      return ret;
+    int ret = get_url(url);
+    if (ret < 0) {
+      return tl::unexpected(ret);
+    }
     param_vec_t params;
     populate_params(params, &uid, self_zone_group);
     RGWRESTSimpleRequest req(cct, info.method, url, NULL, &params, api_name);
-    ret = req.forward_request(dpp, key, info, max_response, inbl, outbl, y);
-    if (ret == -EIO) {
-      set_url_unconnectable(url);
-      if (tries < NUM_ENPOINT_IOERROR_RETRIES - 1) {
-        ldpp_dout(dpp, 20) << __func__  << "(): failed to forward request. retries=" << tries << dendl;
-        continue;
-      }
+    auto result = req.forward_request(dpp, key, info, max_response, inbl, outbl, y);
+    if (result) {
+      return result;
+    } else if (result.error() != -EIO) {
+      return result;
+    }
+    set_url_unconnectable(url);
+    if (tries < NUM_ENPOINT_IOERROR_RETRIES - 1) {
+      ldpp_dout(dpp, 20) << __func__  << "(): failed to forward request. retries=" << tries << dendl;
     }
-    break;
   }
-  return ret;
+  return tl::unexpected(-EIO);
 }
 
-int RGWRESTConn::forward_iam_request(const DoutPrefixProvider *dpp, const req_info& info, size_t max_response, bufferlist *inbl, bufferlist *outbl, optional_yield y)
+auto RGWRESTConn::forward_iam(const DoutPrefixProvider *dpp, const req_info& info,
+                              size_t max_response, bufferlist *inbl,
+                              bufferlist *outbl, optional_yield y)
+  -> tl::expected<int, int>
 {
-  int ret = 0;
-
   static constexpr int NUM_ENPOINT_IOERROR_RETRIES = 20;
   for (int tries = 0; tries < NUM_ENPOINT_IOERROR_RETRIES; tries++) {
     string url;
-    ret = get_url(url);
-    if (ret < 0)
-      return ret;
+    int ret = get_url(url);
+    if (ret < 0) {
+      return tl::unexpected(ret);
+    }
     param_vec_t params;
     std::string service = "iam";
     RGWRESTSimpleRequest req(cct, info.method, url, NULL, &params, api_name);
     // coverity[uninit_use_in_call:SUPPRESS]
-    ret = req.forward_request(dpp, key, info, max_response, inbl, outbl, y, service);
-    if (ret == -EIO) {
-      set_url_unconnectable(url);
-      if (tries < NUM_ENPOINT_IOERROR_RETRIES - 1) {
-        ldpp_dout(dpp, 20) << __func__  << "(): failed to forward request. retries=" << tries << dendl;
-        continue;
-      }
+    auto result = req.forward_request(dpp, key, info, max_response, inbl, outbl, y, service);
+    if (result) {
+      return result;
+    } else if (result.error() != -EIO) {
+      return result;
+    }
+    set_url_unconnectable(url);
+    if (tries < NUM_ENPOINT_IOERROR_RETRIES - 1) {
+      ldpp_dout(dpp, 20) << __func__  << "(): failed to forward request. retries=" << tries << dendl;
     }
-    break;
   }
-  return ret;
+  return tl::unexpected(-EIO);
 }
 
 int RGWRESTConn::put_obj_send_init(const rgw_obj& obj, const rgw_http_param_pair *extra_params, RGWRESTStreamS3PutObj **req)

diff --git a/src/rgw/rgw_rest_conn.h b/src/rgw/rgw_rest_conn.h
index c310f017c0dc08f13c031322787e3a9bc0652605..1a4fa98a879c3f0bcc3ce68500a5c515bf64d383 100644 (file)
--- a/src/rgw/rgw_rest_conn.h
+++ b/src/rgw/rgw_rest_conn.h
@@ -130,11 +130,16 @@ public:
   virtual void populate_params(param_vec_t& params, const rgw_owner* uid, const std::string& zonegroup);
 
   /* sync request */
-  int forward(const DoutPrefixProvider *dpp, const rgw_owner& uid, const req_info& info, size_t max_response, bufferlist *inbl, bufferlist *outbl, optional_yield y);
+  auto forward(const DoutPrefixProvider *dpp, const rgw_owner& uid,
+               const req_info& info, size_t max_response,
+               bufferlist *inbl, bufferlist *outbl, optional_yield y)
+    -> tl::expected<int, int>;
 
   /* sync request */
-  int forward_iam_request(const DoutPrefixProvider *dpp, const req_info& info, size_t max_response, bufferlist *inbl, bufferlist *outbl, optional_yield y);
-
+  auto forward_iam(const DoutPrefixProvider *dpp, const req_info& info,
+                   size_t max_response, bufferlist *inbl,
+                   bufferlist *outbl, optional_yield y)
+    -> tl::expected<int, int>;
 
   /* async requests */
   int put_obj_send_init(const rgw_obj& obj, const rgw_http_param_pair *extra_params, RGWRESTStreamS3PutObj **req);

diff --git a/src/rgw/rgw_rest_iam.cc b/src/rgw/rgw_rest_iam.cc
index 32ff7ac0a2282da3a78bd8f6c17188a5e130f6e9..5f9ae9c3a866d432f49aca0c254bf06d08d1af8d 100644 (file)
--- a/src/rgw/rgw_rest_iam.cc
+++ b/src/rgw/rgw_rest_iam.cc
@@ -304,8 +304,12 @@ int forward_iam_request_to_master(const DoutPrefixProvider* dpp,
                           std::move(creds), zg->second.id, zg->second.api_name};
   bufferlist outdata;
   constexpr size_t max_response_size = 128 * 1024; // we expect a very small response
-  int ret = conn.forward_iam_request(dpp, req, max_response_size,
-                                     &indata, &outdata, y);
+  auto result = conn.forward_iam(dpp, req, max_response_size,
+                                 &indata, &outdata, y);
+  if (!result) {
+    return result.error();
+  }
+  int ret = rgw_http_error_to_errno(*result);
   if (ret < 0) {
     return ret;
   }