]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
RGW - Zipper - Chown rework 49794/head
authorDaniel Gryniewicz <dang@redhat.com>
Thu, 27 Oct 2022 15:47:52 +0000 (11:47 -0400)
committerDaniel Gryniewicz <dang@redhat.com>
Thu, 19 Jan 2023 17:47:28 +0000 (12:47 -0500)
Chown was not unlinking, and was partially leaving the ownership wrong.
Fix this by reworking chown.  Bucket::chown now just changes ownership
of the bucket from it's previous owner to a new one.  An Object::chown
was added to change the ownership of an object.  The chown admin API was
modified to loop through all the objects in a bucket, changing the
ownership, so that a normal Bucket::chown doesn't need to do that
anymore.

Fixes https://tracker.ceph.com/issues/57936

Signed-off-by: Daniel Gryniewicz <dang@redhat.com>
(cherry picked from commit c466fd7bc30841c2eeef48ecccfba85d25bedb5c)

src/rgw/rgw_admin.cc
src/rgw/rgw_bucket.cc
src/rgw/rgw_bucket.h
src/rgw/rgw_sal.h
src/rgw/rgw_sal_dbstore.cc
src/rgw/rgw_sal_dbstore.h
src/rgw/rgw_sal_rados.cc
src/rgw/rgw_sal_rados.h
src/rgw/rgw_user.cc

index a18ac23d7807b541a1af4df271121d5d40487e15..5e89b9f2196cb4b54b217158cf8d5593323c776b 100644 (file)
@@ -8039,7 +8039,7 @@ next:
   }
 
   if (opt_cmd == OPT::USER_CHECK) {
-    check_bad_user_bucket_mapping(store, user.get(), fix, null_yield, dpp());
+    check_bad_user_bucket_mapping(store, *user.get(), fix, null_yield, dpp());
   }
 
   if (opt_cmd == OPT::USER_STATS) {
index 12772f1ea28bcc0a71556fec28c9d9e76c60a711..e7840cae03447d4dc328c007851f62cc4b613729 100644 (file)
@@ -44,6 +44,7 @@
 #include "rgw_reshard.h"
 #include "rgw_lc.h"
 #include "rgw_bucket_layout.h"
+#include "common/errno.h"
 
 // stolen from src/cls/version/cls_version.cc
 #define VERSION_ATTR "ceph.objclass.version"
@@ -252,7 +253,7 @@ static void dump_mulipart_index_results(list<rgw_obj_index_key>& objs_to_unlink,
   }
 }
 
-void check_bad_user_bucket_mapping(rgw::sal::Store* store, rgw::sal::User* user,
+void check_bad_user_bucket_mapping(rgw::sal::Store* store, rgw::sal::User& user,
                                   bool fix,
                                   optional_yield y,
                                    const DoutPrefixProvider *dpp)
@@ -265,7 +266,7 @@ void check_bad_user_bucket_mapping(rgw::sal::Store* store, rgw::sal::User* user,
   size_t max_entries = cct->_conf->rgw_list_buckets_max_chunk;
 
   do {
-    int ret = user->list_buckets(dpp, marker, string(), max_entries, false, user_buckets, y);
+    int ret = user.list_buckets(dpp, marker, string(), max_entries, false, user_buckets, y);
     if (ret < 0) {
       ldout(store->ctx(), 0) << "failed to read user buckets: "
                             << cpp_strerror(-ret) << dendl;
@@ -281,7 +282,7 @@ void check_bad_user_bucket_mapping(rgw::sal::Store* store, rgw::sal::User* user,
       auto& bucket = i->second;
 
       std::unique_ptr<rgw::sal::Bucket> actual_bucket;
-      int r = store->get_bucket(dpp, user, user->get_tenant(), bucket->get_name(), &actual_bucket, null_yield);
+      int r = store->get_bucket(dpp, &user, user.get_tenant(), bucket->get_name(), &actual_bucket, null_yield);
       if (r < 0) {
         ldout(store->ctx(), 0) << "could not get bucket info for bucket=" << bucket << dendl;
         continue;
@@ -294,7 +295,7 @@ void check_bad_user_bucket_mapping(rgw::sal::Store* store, rgw::sal::User* user,
         cout << "bucket info mismatch: expected " << actual_bucket << " got " << bucket << std::endl;
         if (fix) {
           cout << "fixing" << std::endl;
-         r = actual_bucket->chown(dpp, user, nullptr, null_yield);
+         r = actual_bucket->chown(dpp, user, null_yield);
           if (r < 0) {
             cerr << "failed to fix bucket: " << cpp_strerror(-r) << std::endl;
           }
@@ -419,12 +420,12 @@ bool rgw_find_bucket_by_id(const DoutPrefixProvider *dpp, CephContext *cct, rgw:
 int RGWBucket::chown(RGWBucketAdminOpState& op_state, const string& marker,
                      optional_yield y, const DoutPrefixProvider *dpp, std::string *err_msg)
 {
-  int ret = bucket->chown(dpp, user.get(), user.get(), y, &marker);
-  if (ret < 0) {
-    set_err_msg(err_msg, "Failed to change object ownership: " + cpp_strerror(-ret));
-  }
-  
-  return ret;
+  /* User passed in by rgw_admin is the new user; get the current user and set it in
+   * the bucket */
+  std::unique_ptr<rgw::sal::User> old_user = store->get_user(bucket->get_info().owner);
+  bucket->set_owner(old_user.get());
+
+  return rgw_chown_bucket_and_objects(store, bucket.get(), user.get(), marker, err_msg, dpp, y);
 }
 
 int RGWBucket::set_quota(RGWBucketAdminOpState& op_state, const DoutPrefixProvider *dpp, std::string *err_msg)
@@ -2888,97 +2889,6 @@ int RGWBucketCtl::do_unlink_bucket(RGWSI_Bucket_EP_Ctx& ctx,
   return svc.bucket->store_bucket_entrypoint_info(ctx, meta_key, ep, false, real_time(), &attrs, &ot, y, dpp);
 }
 
-// TODO: remove RGWRados dependency for bucket listing
-int RGWBucketCtl::chown(rgw::sal::Store* store, rgw::sal::Bucket* bucket,
-                        const rgw_user& user_id, const std::string& display_name,
-                        const std::string& marker, optional_yield y, const DoutPrefixProvider *dpp)
-{
-  map<string, bool> common_prefixes;
-
-  rgw::sal::Bucket::ListParams params;
-  rgw::sal::Bucket::ListResults results;
-
-  params.list_versions = true;
-  params.allow_unordered = true;
-  params.marker = marker;
-
-  int count = 0;
-  int max_entries = 1000;
-
-  //Loop through objects and update object acls to point to bucket owner
-
-  do {
-    RGWObjectCtx obj_ctx(store);
-    results.objs.clear();
-    int ret = bucket->list(dpp, params, max_entries, results, y);
-    if (ret < 0) {
-      ldpp_dout(dpp, 0) << "ERROR: list objects failed: " << cpp_strerror(-ret) << dendl;
-      return ret;
-    }
-
-    params.marker = results.next_marker;
-    count += results.objs.size();
-
-    for (const auto& obj : results.objs) {
-      std::unique_ptr<rgw::sal::Object> r_obj = bucket->get_object(obj.key);
-
-      ret = r_obj->get_obj_attrs(&obj_ctx, y, dpp);
-      if (ret < 0){
-        ldpp_dout(dpp, 0) << "ERROR: failed to read object " << obj.key.name << cpp_strerror(-ret) << dendl;
-        continue;
-      }
-      const auto& aiter = r_obj->get_attrs().find(RGW_ATTR_ACL);
-      if (aiter == r_obj->get_attrs().end()) {
-        ldpp_dout(dpp, 0) << "ERROR: no acls found for object " << obj.key.name << " .Continuing with next object." << dendl;
-        continue;
-      } else {
-        bufferlist& bl = aiter->second;
-        RGWAccessControlPolicy policy(store->ctx());
-        ACLOwner owner;
-        try {
-          decode(policy, bl);
-          owner = policy.get_owner();
-        } catch (buffer::error& err) {
-          ldpp_dout(dpp, 0) << "ERROR: decode policy failed" << err.what()
-                                << dendl;
-          return -EIO;
-        }
-
-        //Get the ACL from the policy
-        RGWAccessControlList& acl = policy.get_acl();
-
-        //Remove grant that is set to old owner
-        acl.remove_canon_user_grant(owner.get_id());
-
-        //Create a grant and add grant
-        ACLGrant grant;
-        grant.set_canon(user_id, display_name, RGW_PERM_FULL_CONTROL);
-        acl.add_grant(&grant);
-
-        //Update the ACL owner to the new user
-        owner.set_id(user_id);
-        owner.set_name(display_name);
-        policy.set_owner(owner);
-
-        bl.clear();
-        encode(policy, bl);
-
-       r_obj->set_atomic(&obj_ctx);
-       map<string, bufferlist> attrs;
-       attrs[RGW_ATTR_ACL] = bl;
-       ret = r_obj->set_obj_attrs(dpp, &obj_ctx, &attrs, nullptr, y);
-        if (ret < 0) {
-          ldpp_dout(dpp, 0) << "ERROR: modify attr failed " << cpp_strerror(-ret) << dendl;
-          return ret;
-        }
-      }
-    }
-    cerr << count << " objects processed in " << bucket
-        << ". Next marker " << params.marker.name << std::endl;
-  } while(results.is_truncated);
-  return 0;
-}
-
 int RGWBucketCtl::read_bucket_stats(const rgw_bucket& bucket,
                                     RGWBucketEnt *result,
                                     optional_yield y,
@@ -3119,3 +3029,56 @@ void RGWBucketEntryPoint::decode_json(JSONObj *obj) {
   }
 }
 
+int rgw_chown_bucket_and_objects(rgw::sal::Store* store, rgw::sal::Bucket* bucket,
+                                rgw::sal::User* new_user,
+                                const std::string& marker, std::string *err_msg,
+                                const DoutPrefixProvider *dpp, optional_yield y)
+{
+  /* Chown on the bucket */
+  int ret = bucket->chown(dpp, *new_user, y);
+  if (ret < 0) {
+    set_err_msg(err_msg, "Failed to change object ownership: " + cpp_strerror(-ret));
+  }
+
+  /* Now chown on all the objects in the bucket */
+  map<string, bool> common_prefixes;
+
+  rgw::sal::Bucket::ListParams params;
+  rgw::sal::Bucket::ListResults results;
+
+  params.list_versions = true;
+  params.allow_unordered = true;
+  params.marker = marker;
+
+  int count = 0;
+  int max_entries = 1000;
+
+  //Loop through objects and update object acls to point to bucket owner
+
+  do {
+    results.objs.clear();
+    ret = bucket->list(dpp, params, max_entries, results, y);
+    if (ret < 0) {
+      ldpp_dout(dpp, 0) << "ERROR: list objects failed: " << cpp_strerror(-ret) << dendl;
+      return ret;
+    }
+
+    params.marker = results.next_marker;
+    count += results.objs.size();
+
+    for (const auto& obj : results.objs) {
+      std::unique_ptr<rgw::sal::Object> r_obj = bucket->get_object(obj.key);
+
+      ret = r_obj->chown(*new_user, dpp, y);
+        if (ret < 0) {
+          ldpp_dout(dpp, 0) << "ERROR: chown failed on " << r_obj << " :" << cpp_strerror(-ret) << dendl;
+          return ret;
+        }
+      }
+    cerr << count << " objects processed in " << bucket
+        << ". Next marker " << params.marker.name << std::endl;
+  } while(results.is_truncated);
+
+  return ret;
+}
+
index 0bc24db7e40957c8571800b32fa62d38e031c72e..090effd2858f51243361cb253f57a27512ac3f1a 100644 (file)
@@ -51,6 +51,14 @@ extern void rgw_parse_url_bucket(const std::string& bucket,
                                  const std::string& auth_tenant,
                                  std::string &tenant_name, std::string &bucket_name);
 
+extern int rgw_chown_bucket_and_objects(rgw::sal::Store* store,
+                                       rgw::sal::Bucket* bucket,
+                                       rgw::sal::User* new_user,
+                                       const std::string& marker,
+                                       std::string *err_msg,
+                                       const DoutPrefixProvider *dpp,
+                                       optional_yield y);
+
 // this is used as a filter to RGWRados::cls_bucket_list_ordered; it
 // conforms to the type RGWBucketListNameFilter
 extern bool rgw_bucket_object_check_filter(const std::string& oid);
@@ -225,7 +233,7 @@ extern int rgw_object_get_attr(rgw::sal::Store* store, rgw::sal::Object* obj,
                               const char* attr_name, bufferlist& out_bl,
                               optional_yield y);
 
-extern void check_bad_user_bucket_mapping(rgw::sal::Store* store, rgw::sal::User* user, bool fix, optional_yield y, const DoutPrefixProvider *dpp);
+extern void check_bad_user_bucket_mapping(rgw::sal::Store* store, rgw::sal::User& user, bool fix, optional_yield y, const DoutPrefixProvider *dpp);
 
 struct RGWBucketAdminOpState {
   rgw_user uid;
@@ -688,10 +696,6 @@ public:
                     const DoutPrefixProvider *dpp,
                     bool update_entrypoint = true);
 
-  int chown(rgw::sal::Store* store, rgw::sal::Bucket* bucket,
-            const rgw_user& user_id, const std::string& display_name,
-            const std::string& marker, optional_yield y, const DoutPrefixProvider *dpp);
-
   int read_buckets_stats(std::map<std::string, RGWBucketEnt>& m,
                          optional_yield y,
                          const DoutPrefixProvider *dpp);
index 9d02cbbfdf501bc914e514f411b6580eb72423e7..3434c7d51c6885db5e8e52c79ca0370194d06853 100644 (file)
@@ -665,8 +665,9 @@ class Bucket {
     virtual int update_container_stats(const DoutPrefixProvider* dpp) = 0;
     /** Check if this bucket needs resharding, and schedule it if it does */
     virtual int check_bucket_shards(const DoutPrefixProvider* dpp) = 0;
-    /** Change the owner of this bucket in the backing store */
-    virtual int chown(const DoutPrefixProvider* dpp, User* new_user, User* old_user, optional_yield y, const std::string* marker = nullptr) = 0;
+    /** Change the owner of this bucket in the backing store.  Current owner must be set.  Does not
+     * change ownership of the objects in the bucket. */
+    virtual int chown(const DoutPrefixProvider* dpp, User& new_user, optional_yield y) = 0;
     /** Store the cached bucket info into the backing store */
     virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime) = 0;
     /** Check to see if the given user is the owner of this bucket */
@@ -1093,6 +1094,8 @@ class Object {
     /** Get a single OMAP value matching the given key */
     virtual int omap_set_val_by_key(const DoutPrefixProvider *dpp, const std::string& key, bufferlist& val,
                                    bool must_exist, optional_yield y) = 0;
+    /** Change the ownership of this object */
+    virtual int chown(User& new_user, const DoutPrefixProvider* dpp, optional_yield y) = 0;
 
     /** Check to see if the give object pointer is uninitialized */
     static bool empty(Object* o) { return (!o || o->empty()); }
index f646d94357dfa1aaa3170be62518108dadc6eb67..67ab7b86f78485c1ed3d645cda0c5f8a95f4d427 100644 (file)
@@ -311,13 +311,11 @@ namespace rgw::sal {
     return 0;
   }
 
-  int DBBucket::chown(const DoutPrefixProvider *dpp, User* new_user, User* old_user, optional_yield y, const std::string* marker)
+  int DBBucket::chown(const DoutPrefixProvider *dpp, User& new_user, optional_yield y)
   {
     int ret;
 
-    ret = store->getDB()->update_bucket(dpp, "owner", info, false, &(new_user->get_id()), nullptr, nullptr, nullptr);
-
-    /* XXX: Update policies of all the bucket->objects with new user */
+    ret = store->getDB()->update_bucket(dpp, "owner", info, false, &(new_user.get_id()), nullptr, nullptr, nullptr);
     return ret;
   }
 
@@ -684,6 +682,11 @@ namespace rgw::sal {
     return op_target.obj_omap_set_val_by_key(dpp, key, val, must_exist);
   }
 
+  int DBObject::chown(User& new_user, const DoutPrefixProvider* dpp, optional_yield y)
+  {
+    return 0;
+  }
+
   MPSerializer* DBObject::get_serializer(const DoutPrefixProvider *dpp, const std::string& lock_name)
   {
     return new MPDBSerializer(dpp, store, this, lock_name);
index 49c1bc4340bc24e8bcc5d2bb1507495d7b7a5ae8..0420ac1af221b00f08432767a137b1df9559d312 100644 (file)
@@ -196,7 +196,7 @@ protected:
       virtual int sync_user_stats(const DoutPrefixProvider *dpp, optional_yield y) override;
       virtual int update_container_stats(const DoutPrefixProvider *dpp) override;
       virtual int check_bucket_shards(const DoutPrefixProvider *dpp) override;
-      virtual int chown(const DoutPrefixProvider *dpp, User* new_user, User* old_user, optional_yield y, const std::string* marker = nullptr) override;
+      virtual int chown(const DoutPrefixProvider *dpp, User& new_user, optional_yield y) override;
       virtual int put_info(const DoutPrefixProvider *dpp, bool exclusive, ceph::real_time mtime) override;
       virtual bool is_owner(User* user) override;
       virtual int check_empty(const DoutPrefixProvider *dpp, optional_yield y) override;
@@ -555,6 +555,7 @@ protected:
           Attrs* vals) override;
       virtual int omap_set_val_by_key(const DoutPrefixProvider *dpp, const std::string& key, bufferlist& val,
           bool must_exist, optional_yield y) override;
+      virtual int chown(User& new_user, const DoutPrefixProvider* dpp, optional_yield y) override;
     private:
       int read_attrs(const DoutPrefixProvider* dpp, DB::Object::Read &read_op, optional_yield y, rgw_obj* target_obj = nullptr);
   };
index 8c9f9ef031b7da6dc24178d7f3b3e4a215593efa..f01518c0472e8f75cd6ef65ba060a98a8848e912 100644 (file)
@@ -26,6 +26,7 @@
 #include "rgw_sal_rados.h"
 #include "rgw_bucket.h"
 #include "rgw_multi.h"
+#include "rgw_acl.h"
 #include "rgw_acl_s3.h"
 #include "rgw_aio.h"
 #include "rgw_aio_throttle.h"
@@ -692,23 +693,22 @@ int RadosBucket::unlink(const DoutPrefixProvider* dpp, User* new_user, optional_
   return store->ctl()->bucket->unlink_bucket(new_user->get_id(), info.bucket, y, dpp, update_entrypoint);
 }
 
-int RadosBucket::chown(const DoutPrefixProvider* dpp, User* new_user, User* old_user, optional_yield y, const std::string* marker)
+int RadosBucket::chown(const DoutPrefixProvider* dpp, User& new_user, optional_yield y)
 {
   std::string obj_marker;
+  int r;
 
-  if (marker == nullptr)
-    marker = &obj_marker;
+  if (!owner) {
+      ldpp_dout(dpp, 0) << __func__ << " Cannot chown without an owner " << dendl;
+      return -EINVAL;
+  }
 
-  int r = this->link(dpp, new_user, y);
+  r = this->unlink(dpp, owner, y);
   if (r < 0) {
     return r;
   }
-  if (!old_user) {
-    return r;
-  }
 
-  return store->ctl()->bucket->chown(store, this, new_user->get_id(),
-                          old_user->get_display_name(), *marker, y, dpp);
+  return this->link(dpp, &new_user, y);
 }
 
 int RadosBucket::put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time _mtime)
@@ -1648,6 +1648,65 @@ int RadosObject::omap_set_val_by_key(const DoutPrefixProvider *dpp, const std::s
   return sysobj.omap().set_must_exist(must_exist).set(dpp, key, val, y);
 }
 
+int RadosObject::chown(User& new_user, const DoutPrefixProvider* dpp, optional_yield y)
+{
+  RGWObjectCtx obj_ctx(store);
+  int r = get_obj_attrs(&obj_ctx, y, dpp);
+  if (r < 0) {
+    ldpp_dout(dpp, 0) << "ERROR: failed to read object attrs " << get_name() << cpp_strerror(-r) << dendl;
+    return r;
+  }
+
+  const auto& aiter = get_attrs().find(RGW_ATTR_ACL);
+  if (aiter == get_attrs().end()) {
+    ldpp_dout(dpp, 0) << "ERROR: no acls found for object " << get_name() << dendl;
+    return -EINVAL;
+  }
+
+  bufferlist& bl = aiter->second;
+  RGWAccessControlPolicy policy(store->ctx());
+  ACLOwner owner;
+  auto bliter = bl.cbegin();
+  try {
+    policy.decode(bliter);
+    owner = policy.get_owner();
+  } catch (buffer::error& err) {
+    ldpp_dout(dpp, 0) << "ERROR: decode policy failed" << err.what()
+      << dendl;
+    return -EIO;
+  }
+
+  //Get the ACL from the policy
+  RGWAccessControlList& acl = policy.get_acl();
+
+  //Remove grant that is set to old owner
+  acl.remove_canon_user_grant(owner.get_id());
+
+  //Create a grant and add grant
+  ACLGrant grant;
+  grant.set_canon(new_user.get_id(), new_user.get_display_name(), RGW_PERM_FULL_CONTROL);
+  acl.add_grant(&grant);
+
+  //Update the ACL owner to the new user
+  owner.set_id(new_user.get_id());
+  owner.set_name(new_user.get_display_name());
+  policy.set_owner(owner);
+
+  bl.clear();
+  encode(policy, bl);
+
+  set_atomic(&obj_ctx);
+  map<string, bufferlist> attrs;
+  attrs[RGW_ATTR_ACL] = bl;
+  r = set_obj_attrs(dpp, &obj_ctx, &attrs, nullptr, y);
+  if (r < 0) {
+    ldpp_dout(dpp, 0) << "ERROR: modify attr failed " << cpp_strerror(-r) << dendl;
+    return r;
+  }
+
+  return 0;
+}
+
 MPSerializer* RadosObject::get_serializer(const DoutPrefixProvider *dpp, const std::string& lock_name)
 {
   return new MPRadosSerializer(dpp, store, this, lock_name);
index 4e4673c032806dce1026a9e29af5287defe541e5..498c51223c33d546beaa17842acc00ed1dcb0dc5 100644 (file)
@@ -213,6 +213,7 @@ class RadosObject : public Object {
                              Attrs* vals) override;
     virtual int omap_set_val_by_key(const DoutPrefixProvider *dpp, const std::string& key, bufferlist& val,
                                    bool must_exist, optional_yield y) override;
+    virtual int chown(User& new_user, const DoutPrefixProvider* dpp, optional_yield y) override;
 
     /* Internal to RadosStore */
     int get_max_chunk_size(const DoutPrefixProvider* dpp,
@@ -300,7 +301,7 @@ class RadosBucket : public Bucket {
     virtual int sync_user_stats(const DoutPrefixProvider *dpp, optional_yield y) override;
     virtual int update_container_stats(const DoutPrefixProvider* dpp) override;
     virtual int check_bucket_shards(const DoutPrefixProvider* dpp) override;
-    virtual int chown(const DoutPrefixProvider* dpp, User* new_user, User* old_user, optional_yield y, const std::string* marker = nullptr) override;
+    virtual int chown(const DoutPrefixProvider* dpp, User& new_user, optional_yield y) override;
     virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime) override;
     virtual bool is_owner(User* user) override;
     virtual int check_empty(const DoutPrefixProvider* dpp, optional_yield y) override;
index 3db58af7e9501fc6084ede03601060ad85f0da23..28eaed66bd0216a524960e27dd6c7c4aa464b3da 100644 (file)
@@ -1690,7 +1690,8 @@ int RGWUser::execute_rename(const DoutPrefixProvider *dpp, RGWUserAdminOpState&
         return ret;
       }
 
-      ret = bucket->chown(dpp, new_user.get(), old_user.get(), y);
+      ret = rgw_chown_bucket_and_objects(store, bucket.get(), new_user.get(),
+                                        std::string(), nullptr, dpp, y);
       if (ret < 0) {
         set_err_msg(err_msg, "failed to run bucket chown" + cpp_strerror(-ret));
         return ret;