From: kchheda3 Date: Wed, 3 Sep 2025 14:48:42 +0000 (-0400) Subject: rgw/account: bucket acls are not completely migrated once the user is migrated to... X-Git-Tag: testing/wip-jcollin-testing-20251107.050937-tentacle~16^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=dc4f7e5d7ebbe397560d7e465c6b7b4c1c3ee166;p=ceph-ci.git rgw/account: bucket acls are not completely migrated once the user is migrated to an account Signed-off-by: kchheda3 (cherry picked from commit 23dc3697cfd309b4d8736ec99490cd57db621cf7) --- diff --git a/src/rgw/driver/daos/rgw_sal_daos.cc b/src/rgw/driver/daos/rgw_sal_daos.cc index c90a8770514..6350b266936 100644 --- a/src/rgw/driver/daos/rgw_sal_daos.cc +++ b/src/rgw/driver/daos/rgw_sal_daos.cc @@ -509,7 +509,9 @@ int DaosBucket::check_bucket_shards(const DoutPrefixProvider* dpp) { return DAOS_NOT_IMPLEMENTED_LOG(dpp); } -int DaosBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_user, +int DaosBucket::chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_user, + const std::string& new_owner_name, optional_yield y) { return DAOS_NOT_IMPLEMENTED_LOG(dpp); } diff --git a/src/rgw/driver/daos/rgw_sal_daos.h b/src/rgw/driver/daos/rgw_sal_daos.h index 65ecbdcbb28..743d20d484e 100644 --- a/src/rgw/driver/daos/rgw_sal_daos.h +++ b/src/rgw/driver/daos/rgw_sal_daos.h @@ -311,7 +311,9 @@ class DaosBucket : public StoreBucket { virtual int sync_owner_stats(const DoutPrefixProvider* dpp, optional_yield y) override; virtual int check_bucket_shards(const DoutPrefixProvider* dpp) override; - virtual int chown(const DoutPrefixProvider* dpp, const rgw_owner& new_user, + virtual int chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_user, + const std::string& new_owner_name, optional_yield y) override; virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime) override; diff --git a/src/rgw/driver/posix/rgw_sal_posix.cc b/src/rgw/driver/posix/rgw_sal_posix.cc index 798a18bfb9e..9012714f987 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.cc +++ b/src/rgw/driver/posix/rgw_sal_posix.cc @@ -2533,8 +2533,10 @@ int POSIXBucket::check_bucket_shards(const DoutPrefixProvider* dpp, return 0; } -int POSIXBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y) -{ +int POSIXBucket::chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + optional_yield y) { /* TODO map user to UID/GID, and change it */ return 0; } diff --git a/src/rgw/driver/posix/rgw_sal_posix.h b/src/rgw/driver/posix/rgw_sal_posix.h index b31acb3b860..abad8a475cb 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.h +++ b/src/rgw/driver/posix/rgw_sal_posix.h @@ -527,9 +527,12 @@ public: RGWBucketEnt* ent) override; virtual int check_bucket_shards(const DoutPrefixProvider* dpp, uint64_t num_objs, optional_yield y) override; - virtual int chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y) override; + virtual int chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + optional_yield y) override; virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, - ceph::real_time mtime, optional_yield y) override; + ceph::real_time mtime, optional_yield y) override; virtual int check_empty(const DoutPrefixProvider* dpp, optional_yield y) override; virtual int check_quota(const DoutPrefixProvider *dpp, RGWQuota& quota, uint64_t obj_size, optional_yield y, bool check_size_only = false) override; virtual int try_refresh_info(const DoutPrefixProvider* dpp, ceph::real_time* pmtime, optional_yield y) override; diff --git a/src/rgw/driver/rados/rgw_bucket.cc b/src/rgw/driver/rados/rgw_bucket.cc index 7d4501f0970..9128e52af21 100644 --- a/src/rgw/driver/rados/rgw_bucket.cc +++ b/src/rgw/driver/rados/rgw_bucket.cc @@ -96,6 +96,7 @@ static void dump_multipart_index_results(std::list& objs, void check_bad_owner_bucket_mapping(rgw::sal::Driver* driver, const rgw_owner& owner, + const std::string& owner_name, const std::string& tenant, bool fix, optional_yield y, const DoutPrefixProvider *dpp) @@ -126,7 +127,7 @@ void check_bad_owner_bucket_mapping(rgw::sal::Driver* driver, << " got " << bucket << std::endl; if (fix) { cout << "fixing" << std::endl; - r = bucket->chown(dpp, owner, y); + r = bucket->chown(dpp, owner, owner_name, y); if (r < 0) { cerr << "failed to fix bucket: " << cpp_strerror(-r) << std::endl; } diff --git a/src/rgw/driver/rados/rgw_bucket.h b/src/rgw/driver/rados/rgw_bucket.h index 7455c682e2e..1edfa6a9e31 100644 --- a/src/rgw/driver/rados/rgw_bucket.h +++ b/src/rgw/driver/rados/rgw_bucket.h @@ -212,6 +212,7 @@ extern int rgw_object_get_attr(rgw::sal::Driver* driver, rgw::sal::Object* obj, void check_bad_owner_bucket_mapping(rgw::sal::Driver* driver, const rgw_owner& owner, + const std::string& owner_name, const std::string& tenant, bool fix, optional_yield y, const DoutPrefixProvider *dpp); diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index 37eebb39c67..0e332279a8d 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -709,8 +709,10 @@ int RadosBucket::unlink(const DoutPrefixProvider* dpp, const rgw_owner& owner, o y, dpp, update_entrypoint); } -int RadosBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y) -{ +int RadosBucket::chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + optional_yield y) { // unlink from the owner, but don't update the entrypoint until link() int r = this->unlink(dpp, info.owner, y, false); if (r < 0) { @@ -730,13 +732,26 @@ int RadosBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner try { auto p = i->second.cbegin(); - RGWAccessControlPolicy acl; - decode(acl, p); + RGWAccessControlPolicy policy; + decode(policy, p); + //Get the ACL from the policy + RGWAccessControlList& acl = policy.get_acl(); + ACLOwner& owner = policy.get_owner(); + + //Remove grant that is set to old owner + acl.remove_canon_user_grant(owner.id); + + //Create a grant and add grant + ACLGrant grant; + grant.set_canon(new_owner, new_owner_name, RGW_PERM_FULL_CONTROL); + acl.add_grant(grant); - acl.get_owner().id = new_owner; + //Update the ACL owner to the new user + owner.id = new_owner; + owner.display_name = new_owner_name; bufferlist bl; - encode(acl, bl); + encode(policy, bl); i->second = std::move(bl); } catch (const buffer::error&) { diff --git a/src/rgw/driver/rados/rgw_sal_rados.h b/src/rgw/driver/rados/rgw_sal_rados.h index ecfb4ba8c43..6b7f5a8e7ff 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.h +++ b/src/rgw/driver/rados/rgw_sal_rados.h @@ -745,7 +745,10 @@ class RadosBucket : public StoreBucket { RGWBucketEnt* ent) override; int check_bucket_shards(const DoutPrefixProvider* dpp, uint64_t num_objs, optional_yield y) override; - virtual int chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y) override; + virtual int chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + optional_yield y) override; virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime, optional_yield y) override; virtual int check_empty(const DoutPrefixProvider* dpp, optional_yield y) override; virtual int check_quota(const DoutPrefixProvider *dpp, RGWQuota& quota, uint64_t obj_size, optional_yield y, bool check_size_only = false) override; diff --git a/src/rgw/driver/rados/rgw_user.cc b/src/rgw/driver/rados/rgw_user.cc index cce593c6bd5..716dec70795 100644 --- a/src/rgw/driver/rados/rgw_user.cc +++ b/src/rgw/driver/rados/rgw_user.cc @@ -1688,8 +1688,8 @@ static int adopt_user_bucket(const DoutPrefixProvider* dpp, optional_yield y, rgw::sal::Driver* driver, const rgw_bucket& bucketid, - const rgw_owner& new_owner) -{ + const rgw_owner& new_owner, + const std::string& new_owner_name) { // retry in case of racing writes to the bucket instance metadata static constexpr auto max_retries = 10; int tries = 0; @@ -1706,7 +1706,7 @@ static int adopt_user_bucket(const DoutPrefixProvider* dpp, return r; } - r = bucket->chown(dpp, new_owner, y); + r = bucket->chown(dpp, new_owner, new_owner_name, y); if (r < 0) { ldpp_dout(dpp, 1) << "failed to chown bucket " << bucketid << ": " << cpp_strerror(r) << dendl; @@ -1719,8 +1719,8 @@ static int adopt_user_bucket(const DoutPrefixProvider* dpp, static int adopt_user_buckets(const DoutPrefixProvider* dpp, optional_yield y, rgw::sal::Driver* driver, const rgw_user& user, - const rgw_account_id& account_id) -{ + const rgw_account_id& account_id, + const std::string& account_name) { const size_t max_chunk = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; constexpr bool need_stats = false; @@ -1736,7 +1736,8 @@ static int adopt_user_buckets(const DoutPrefixProvider* dpp, optional_yield y, } for (const auto& ent : listing.buckets) { - r = adopt_user_bucket(dpp, y, driver, ent.bucket, account_id); + r = adopt_user_bucket(dpp, y, driver, ent.bucket, account_id, + account_name); if (r < 0 && r != -ENOENT) { return r; } @@ -2169,9 +2170,19 @@ int RGWUser::execute_modify(const DoutPrefixProvider *dpp, RGWUserAdminOpState& set_err_msg(err_msg, err); return ret; } + RGWAccountInfo account_info; + rgw::sal::Attrs attrs; + RGWObjVersionTracker objv; + int r = driver->load_account_by_id(dpp, y, op_state.account_id, + account_info, + attrs, objv); + if (r < 0) { + err = "Failed to load account by id"; + return r; + } // change account on user's buckets ret = adopt_user_buckets(dpp, y, driver, user_info.user_id, - user_info.account_id); + user_info.account_id, account_info.name); if (ret < 0) { set_err_msg(err_msg, "failed to change ownership of user's buckets"); return ret; diff --git a/src/rgw/radosgw-admin/radosgw-admin.cc b/src/rgw/radosgw-admin/radosgw-admin.cc index 2cf301f7e5b..f1f24d34c6c 100644 --- a/src/rgw/radosgw-admin/radosgw-admin.cc +++ b/src/rgw/radosgw-admin/radosgw-admin.cc @@ -9500,7 +9500,8 @@ next: } if (opt_cmd == OPT::USER_CHECK) { - check_bad_owner_bucket_mapping(driver, user->get_id(), user->get_tenant(), + check_bad_owner_bucket_mapping(driver, user->get_id(), + user->get_display_name(), user->get_tenant(), fix, null_yield, dpp()); } diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc index 93cd2ea7634..ea48dd9923f 100644 --- a/src/rgw/rgw_bucket.cc +++ b/src/rgw/rgw_bucket.cc @@ -137,7 +137,8 @@ int rgw_chown_bucket_and_objects(rgw::sal::Driver* driver, rgw::sal::Bucket* buc const DoutPrefixProvider *dpp, optional_yield y) { /* Chown on the bucket */ - int ret = bucket->chown(dpp, new_user->get_id(), y); + int ret = bucket->chown(dpp, new_user->get_id(), new_user->get_display_name(), + y); if (ret < 0) { set_err_msg(err_msg, "Failed to change object ownership: " + cpp_strerror(-ret)); } diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 780aa70d755..1991882d9d3 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -936,7 +936,10 @@ class Bucket { uint64_t num_objs, optional_yield y) = 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, const rgw_owner& new_owner, optional_yield y) = 0; + virtual int chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + 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, optional_yield y) = 0; /** Get the owner of this bucket */ diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index e1bbe2cdf89..61fb2eb646f 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -230,8 +230,10 @@ namespace rgw::sal { return 0; } - int DBBucket::chown(const DoutPrefixProvider *dpp, const rgw_owner& new_owner, optional_yield y) - { + int DBBucket::chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + optional_yield y) { int ret; ret = store->getDB()->update_bucket(dpp, "owner", info, false, &new_owner, nullptr, nullptr, nullptr); diff --git a/src/rgw/rgw_sal_dbstore.h b/src/rgw/rgw_sal_dbstore.h index caebc28dabe..158d4bce920 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -164,7 +164,10 @@ protected: RGWBucketEnt* ent) override; int check_bucket_shards(const DoutPrefixProvider *dpp, uint64_t num_objs, optional_yield y) override; - virtual int chown(const DoutPrefixProvider *dpp, const rgw_owner& new_owner, optional_yield y) override; + virtual int chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + optional_yield y) override; virtual int put_info(const DoutPrefixProvider *dpp, bool exclusive, ceph::real_time mtime, optional_yield y) override; virtual int check_empty(const DoutPrefixProvider *dpp, optional_yield y) override; virtual int check_quota(const DoutPrefixProvider *dpp, RGWQuota& quota, uint64_t obj_size, optional_yield y, bool check_size_only = false) override; diff --git a/src/rgw/rgw_sal_filter.cc b/src/rgw/rgw_sal_filter.cc index 25844f07748..51c32d1e176 100644 --- a/src/rgw/rgw_sal_filter.cc +++ b/src/rgw/rgw_sal_filter.cc @@ -885,9 +885,11 @@ int FilterBucket::check_bucket_shards(const DoutPrefixProvider* dpp, return next->check_bucket_shards(dpp, num_objs, y); } -int FilterBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y) -{ - return next->chown(dpp, new_owner, y); +int FilterBucket::chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + optional_yield y) { + return next->chown(dpp, new_owner, new_owner_name, y); } int FilterBucket::put_info(const DoutPrefixProvider* dpp, bool exclusive, diff --git a/src/rgw/rgw_sal_filter.h b/src/rgw/rgw_sal_filter.h index d58253a7de2..f0624bdcf92 100644 --- a/src/rgw/rgw_sal_filter.h +++ b/src/rgw/rgw_sal_filter.h @@ -599,8 +599,10 @@ public: RGWBucketEnt* ent) override; int check_bucket_shards(const DoutPrefixProvider* dpp, uint64_t num_objs, optional_yield y) override; - virtual int chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, - optional_yield y) override; + virtual int chown(const DoutPrefixProvider* dpp, + const rgw_owner& new_owner, + const std::string& new_owner_name, + optional_yield y) override; virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime, optional_yield y) override; virtual const rgw_owner& get_owner() const override;