From 634d7edbcb2dfb609a6bdd2c67eb4ff5c2482dc6 Mon Sep 17 00:00:00 2001 From: kchheda3 Date: Wed, 3 Sep 2025 10:48:42 -0400 Subject: [PATCH] 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) --- src/rgw/driver/daos/rgw_sal_daos.cc | 4 +++- src/rgw/driver/daos/rgw_sal_daos.h | 4 +++- src/rgw/driver/posix/rgw_sal_posix.cc | 6 ++++-- src/rgw/driver/posix/rgw_sal_posix.h | 7 +++++-- src/rgw/driver/rados/rgw_bucket.cc | 3 ++- src/rgw/driver/rados/rgw_bucket.h | 1 + src/rgw/driver/rados/rgw_sal_rados.cc | 27 +++++++++++++++++++++------ src/rgw/driver/rados/rgw_sal_rados.h | 5 ++++- src/rgw/driver/rados/rgw_user.cc | 25 ++++++++++++++++++------- src/rgw/rgw_admin.cc | 3 ++- src/rgw/rgw_bucket.cc | 3 ++- src/rgw/rgw_sal.h | 5 ++++- src/rgw/rgw_sal_dbstore.cc | 6 ++++-- src/rgw/rgw_sal_dbstore.h | 5 ++++- src/rgw/rgw_sal_filter.cc | 8 +++++--- src/rgw/rgw_sal_filter.h | 6 ++++-- 16 files changed, 86 insertions(+), 32 deletions(-) diff --git a/src/rgw/driver/daos/rgw_sal_daos.cc b/src/rgw/driver/daos/rgw_sal_daos.cc index 57e64057c240d..6f38aeebb678c 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 6cb186ab2998a..c3d5363c301cb 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 b3673a4143c47..ba10cef08f688 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.cc +++ b/src/rgw/driver/posix/rgw_sal_posix.cc @@ -980,8 +980,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 4206fe488455b..f98ed17964591 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.h +++ b/src/rgw/driver/posix/rgw_sal_posix.h @@ -201,9 +201,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 6fca78c2656c5..894b6996bea0a 100644 --- a/src/rgw/driver/rados/rgw_bucket.cc +++ b/src/rgw/driver/rados/rgw_bucket.cc @@ -95,6 +95,7 @@ static void dump_mulipart_index_results(list& objs_to_unlink, 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) @@ -125,7 +126,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 07ba76a3209f0..ccb616f39a0f6 100644 --- a/src/rgw/driver/rados/rgw_bucket.h +++ b/src/rgw/driver/rados/rgw_bucket.h @@ -220,6 +220,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 65ca6b05d6996..4c4069d6da07a 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -692,8 +692,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) { @@ -713,13 +715,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 d63de60895e2c..ca65ff145a384 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.h +++ b/src/rgw/driver/rados/rgw_sal_rados.h @@ -717,7 +717,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 1b58753d21e6c..b1dd743b06ad0 100644 --- a/src/rgw/driver/rados/rgw_user.cc +++ b/src/rgw/driver/rados/rgw_user.cc @@ -1682,8 +1682,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; @@ -1700,7 +1700,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; @@ -1713,8 +1713,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; @@ -1730,7 +1730,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; } @@ -2163,9 +2164,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/rgw_admin.cc b/src/rgw/rgw_admin.cc index a2727bba16794..805eee51a8e1e 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -9037,7 +9037,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 93cd2ea763493..ea48dd9923f03 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 97d894163cfeb..9fd50ff07285b 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -940,7 +940,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 9c58e8246c67a..3ca5a9b15ed97 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -228,8 +228,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 b732b98b6c047..e369bf43fb9c9 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -156,7 +156,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 30216d4701ceb..d6e6e3055eb0c 100644 --- a/src/rgw/rgw_sal_filter.cc +++ b/src/rgw/rgw_sal_filter.cc @@ -869,9 +869,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 29ef9e8c27da5..36ac94da50247 100644 --- a/src/rgw/rgw_sal_filter.h +++ b/src/rgw/rgw_sal_filter.h @@ -591,8 +591,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; -- 2.47.3