From: Prasad Krishnan <prasad.krishnan@flipkart.com>
Date: Wed, 18 Mar 2020 18:03:15 +0000 (+0000)
Subject: RGW:Multisite: Rename rgw_copy_verify_object to rgw_sync_obj_integrity
X-Git-Tag: v16.1.0~2110^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=31e944fced60e47139973361cbb753aeaeb3c863;p=ceph.git

RGW:Multisite: Rename rgw_copy_verify_object to rgw_sync_obj_integrity

This patch renames the option rgw_copy_verify_object to
rgw_sync_obj_integrity and incorporates more changes suggested through
code-review comments.

Signed-off-by: Prasad Krishnan <prasad.krishnan@flipkart.com>
---

diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index f2747371c7b4..ac269580ed71 100644
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -1434,7 +1434,7 @@ OPTION(rgw_curl_low_speed_limit, OPT_INT) // low speed limit for certain curl ca
 OPTION(rgw_curl_low_speed_time, OPT_INT) // low speed time for certain curl calls
 OPTION(rgw_copy_obj_progress, OPT_BOOL) // should dump progress during long copy operations?
 OPTION(rgw_copy_obj_progress_every_bytes, OPT_INT) // min bytes between copy progress output
-OPTION(rgw_copy_verify_object, OPT_BOOL) // verify if the copied object is identical to source
+OPTION(rgw_sync_obj_integrity, OPT_BOOL) // verify if the copied object from remote is identical to source
 OPTION(rgw_obj_tombstone_cache_size, OPT_INT) // how many objects in tombstone cache, which is used in multi-zone sync to keep
                                                     // track of removed objects' mtime
 
diff --git a/src/common/options.cc b/src/common/options.cc
index 97a492a32e48..be4a61f04dd4 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -6287,12 +6287,14 @@ std::vector<Option> get_rgw_options() {
     .set_default(1_M)
     .set_description("Send copy-object progress info after these many bytes"),
 
-    Option("rgw_copy_verify_object", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
+    Option("rgw_sync_obj_integrity", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
     .set_default(false)
-    .set_description("Verify if the object copied is identical to its source")
+    .set_description("Verify if the object copied from remote is identical to its source")
     .set_long_description(
-        "If true, this option computes the MD5 checksum of the data which is written at the"
-	"destination and checks if it is identical to the ETAG stored in the source."),
+        "If true, this option computes the MD5 checksum of the data which is written at the "
+	"destination and checks if it is identical to the ETAG stored in the source. "
+        "It ensures integrity of the objects fetched from a remote server over HTTP including "
+        "multisite sync."),
 
     Option("rgw_obj_tombstone_cache_size", Option::TYPE_INT, Option::LEVEL_ADVANCED)
     .set_default(1000)
diff --git a/src/rgw/rgw_etag_verifier.cc b/src/rgw/rgw_etag_verifier.cc
index 2b79254c147b..4db3ca03c05d 100644
--- a/src/rgw/rgw_etag_verifier.cc
+++ b/src/rgw/rgw_etag_verifier.cc
@@ -25,12 +25,12 @@ void RGWPutObj_ETagVerifier_Atomic::calculate_etag()
 
   hash.Final(m);
   buf_to_hex(m, CEPH_CRYPTO_MD5_DIGESTSIZE, calc_md5);
+  calculated_etag = calc_md5;
   ldout(cct, 20) << "Single part object: " << " etag:" << calculated_etag
           << dendl;
-  calculated_etag = calc_md5;
 }
 
-void RGWPutObj_ETagVerifier_MPU::process_end_of_MPU_part(bufferlist in)
+void RGWPutObj_ETagVerifier_MPU::process_end_of_MPU_part()
 {
   unsigned char m[CEPH_CRYPTO_MD5_DIGESTSIZE];
   char calc_md5_part[CEPH_CRYPTO_MD5_DIGESTSIZE * 2 + 1];
@@ -40,11 +40,11 @@ void RGWPutObj_ETagVerifier_MPU::process_end_of_MPU_part(bufferlist in)
   mpu_etag_hash.Update((const unsigned char *)m, sizeof(m));
   hash.Restart();
 
-  /* Debugging begin */
-  buf_to_hex(m, CEPH_CRYPTO_MD5_DIGESTSIZE, calc_md5_part);
-  calculated_etag_part = calc_md5_part;
-  ldout(cct, 20) << "Part etag: " << calculated_etag_part << dendl;
-  /* Debugging end */
+  if (cct->_conf->subsys.should_gather(dout_subsys, 20)) {
+    buf_to_hex(m, CEPH_CRYPTO_MD5_DIGESTSIZE, calc_md5_part);
+    calculated_etag_part = calc_md5_part;
+    ldout(cct, 20) << "Part etag: " << calculated_etag_part << dendl;
+  }
 
   cur_part_index++;
   next_part_index++;
@@ -65,7 +65,7 @@ int RGWPutObj_ETagVerifier_MPU::process(bufferlist&& in, uint64_t logical_offset
 
     uint64_t part_one_len = part_ofs[next_part_index] - logical_offset;
     hash.Update((const unsigned char *)in.c_str(), part_one_len);
-    process_end_of_MPU_part(in);
+    process_end_of_MPU_part();
 
     hash.Update((const unsigned char *)in.c_str() + part_one_len,
       bl_end - part_ofs[cur_part_index]);
@@ -81,7 +81,7 @@ int RGWPutObj_ETagVerifier_MPU::process(bufferlist&& in, uint64_t logical_offset
 
   /* Update the MPU Etag if the current part has ended */
   if (logical_offset + in.length() + 1 == part_ofs[next_part_index])
-    process_end_of_MPU_part(in);
+    process_end_of_MPU_part();
 
 done:
   return Pipe::process(std::move(in), logical_offset);
@@ -106,6 +106,6 @@ void RGWPutObj_ETagVerifier_MPU::calculate_etag()
            sizeof(final_etag_str) - CEPH_CRYPTO_MD5_DIGESTSIZE * 2,
            "-%lld", (long long)(part_ofs.size()));
 
-  ldout(cct, 20) << "MPU calculated ETag:" << calculated_etag << dendl;
   calculated_etag = final_etag_str;
+  ldout(cct, 20) << "MPU calculated ETag:" << calculated_etag << dendl;
 }
diff --git a/src/rgw/rgw_etag_verifier.h b/src/rgw/rgw_etag_verifier.h
index 5c777a8932ee..b55f277ed5b2 100644
--- a/src/rgw/rgw_etag_verifier.h
+++ b/src/rgw/rgw_etag_verifier.h
@@ -31,7 +31,6 @@ public:
 
   virtual void calculate_etag() = 0;
   string get_calculated_etag() { return calculated_etag;}
-  virtual void append_part_ofs(uint64_t ofs) = 0;
 
 }; /* RGWPutObj_ETagVerifier */
 
@@ -43,7 +42,6 @@ public:
 
   int process(bufferlist&& data, uint64_t logical_offset) override;
   void calculate_etag() override;
-  void append_part_ofs(uint64_t ofs) override {}
 
 }; /* RGWPutObj_ETagVerifier_Atomic */
 
@@ -53,7 +51,7 @@ class RGWPutObj_ETagVerifier_MPU : public RGWPutObj_ETagVerifier
   int cur_part_index{0}, next_part_index{1};
   MD5 mpu_etag_hash;
  
-  void process_end_of_MPU_part(bufferlist in);
+  void process_end_of_MPU_part();
 
 public:
   RGWPutObj_ETagVerifier_MPU(CephContext* cct_, rgw::putobj::DataProcessor *next)
@@ -61,7 +59,7 @@ public:
 
   int process(bufferlist&& data, uint64_t logical_offset) override;
   void calculate_etag() override;
-  void append_part_ofs(uint64_t ofs) override { part_ofs.emplace_back(ofs); }
+  void append_part_ofs(uint64_t ofs) { part_ofs.emplace_back(ofs); }
 
 }; /* RGWPutObj_ETagVerifier_MPU */
 
diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc
index b092f3f885d7..78f266d5aa14 100644
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -3294,10 +3294,14 @@ public:
 
       src_attrs.erase(RGW_ATTR_COMPRESSION);
       /* We need the manifest to recompute the ETag for verification */
-      manifest_bl = src_attrs[RGW_ATTR_MANIFEST];
+      auto iter = src_attrs.find(RGW_ATTR_MANIFEST);
+      if (iter != src_attrs.end()) {
+        manifest_bl = std::move(iter->second);
+        src_attrs.erase(iter);
+      }
 
       // filter out olh attributes
-      auto iter = src_attrs.lower_bound(RGW_ATTR_OLH_PREFIX);
+      iter = src_attrs.lower_bound(RGW_ATTR_OLH_PREFIX);
       while (iter != src_attrs.end()) {
         if (!boost::algorithm::starts_with(iter->first, RGW_ATTR_OLH_PREFIX)) {
           break;
@@ -3323,11 +3327,11 @@ public:
     }
 
     /*
-     * Presently we don't support ETag based verification if compression or
-     * encryption is requested. We can enable simultaneous support once we have
-     * a mechanism to know the sequence in which the filters must be applied.
+     * Presently we don't support ETag based verification if encryption is
+     * requested. We can enable simultaneous support once we have a mechanism
+     * to know the sequence in which the filters must be applied.
      */
-    if (cct->_conf->rgw_copy_verify_object && !plugin &&
+    if (cct->_conf->rgw_sync_obj_integrity &&
         src_attrs.find(RGW_ATTR_CRYPT_MODE) == src_attrs.end()) {
 
       RGWObjManifest manifest;
@@ -3354,8 +3358,6 @@ public:
       } else {
         is_mpu_obj = true;
         etag_verifier_mpu = boost::in_place(cct, filter);
-
-        RGWObjManifest::obj_iterator mi;
         uint64_t cur_part_ofs = UINT64_MAX;
 
         /*
@@ -3363,7 +3365,7 @@ public:
          * MPU part. These part ETags then become the input for the MPU object
          * Etag.
          */
-        for (mi = manifest.obj_begin(); mi != manifest.obj_end(); ++mi) {
+        for (auto mi = manifest.obj_begin(); mi != manifest.obj_end(); ++mi) {
           if (cur_part_ofs == mi.get_part_ofs())
             continue;
           cur_part_ofs = mi.get_part_ofs();
@@ -3442,6 +3444,9 @@ public:
   }
 
   string get_calculated_etag() {
+    if (!cct->_conf->rgw_sync_obj_integrity)
+      return "";
+
     if (is_mpu_obj) {
       etag_verifier_mpu->calculate_etag();
       return etag_verifier_mpu->get_calculated_etag();
@@ -4003,6 +4008,21 @@ int RGWRados::fetch_remote_obj(RGWObjectCtx& obj_ctx,
     set_mtime_weight.init(set_mtime, svc.zone->get_zone_short_id(), pg_ver);
   }
 
+  if (cct->_conf->rgw_sync_obj_integrity) {
+    string trimmed_etag = etag;
+
+    /* Remove the leading and trailing double quotes from etag */
+    trimmed_etag.erase(std::remove(trimmed_etag.begin(), trimmed_etag.end(),'\"'),
+      trimmed_etag.end());
+
+    if (cb.get_calculated_etag().compare(trimmed_etag)) {
+      ret = -EIO;
+      ldout(cct, 0) << "ERROR: source and destination objects don't match. Expected etag:"
+        << trimmed_etag << " Computed etag:" << cb.get_calculated_etag() << dendl;
+      goto set_err_state;
+    }
+  }
+
 #define MAX_COMPLETE_RETRY 100
   for (i = 0; i < MAX_COMPLETE_RETRY; i++) {
     bool canceled = false;
@@ -4013,21 +4033,6 @@ int RGWRados::fetch_remote_obj(RGWObjectCtx& obj_ctx,
       goto set_err_state;
     }
 
-    if (cct->_conf->rgw_copy_verify_object) {
-      string trimmed_etag = etag;
-
-      /* Remove the leading and trailing double quotes from etag */
-      trimmed_etag.erase(std::remove(trimmed_etag.begin(), trimmed_etag.end(),'\"'),
-        trimmed_etag.end());
-
-      if (cb.get_calculated_etag().compare(trimmed_etag)) {
-        ret = -EIO;
-        ldout(cct, 0) << "ERROR: source and destination objects don't match. Expected etag:"
-          << trimmed_etag << " Computed etag:" << cb.get_calculated_etag() << dendl;
-        goto set_err_state;
-     }
-   }
-
     if (copy_if_newer && canceled) {
       ldout(cct, 20) << "raced with another write of obj: " << dest_obj << dendl;
       obj_ctx.invalidate(dest_obj); /* object was overwritten */