Skip to content

Commit 5917c06

Browse files
adamemersonadamemerson
authored
Merge pull request ceph#65370 from kchheda3/wip-fixed-acl-account-migration
rgw/account: bucket acls are not completely migrated once the user is migrated to an account Reviewed-by: Casey Bodley <[email protected]>
2 parents 9bebe32 + 23dc369 commit 5917c06

16 files changed

+86
-32
lines changed

‎src/rgw/driver/daos/rgw_sal_daos.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,9 @@ int DaosBucket::check_bucket_shards(const DoutPrefixProvider* dpp) {
509509
return DAOS_NOT_IMPLEMENTED_LOG(dpp);
510510
}
511511

512-
int DaosBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_user,
512+
int DaosBucket::chown(const DoutPrefixProvider* dpp,
513+
const rgw_owner& new_user,
514+
const std::string& new_owner_name,
513515
optional_yield y) {
514516
return DAOS_NOT_IMPLEMENTED_LOG(dpp);
515517
}

‎src/rgw/driver/daos/rgw_sal_daos.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,9 @@ class DaosBucket : public StoreBucket {
311311
virtual int sync_owner_stats(const DoutPrefixProvider* dpp,
312312
optional_yield y) override;
313313
virtual int check_bucket_shards(const DoutPrefixProvider* dpp) override;
314-
virtual int chown(const DoutPrefixProvider* dpp, const rgw_owner& new_user,
314+
virtual int chown(const DoutPrefixProvider* dpp,
315+
const rgw_owner& new_user,
316+
const std::string& new_owner_name,
315317
optional_yield y) override;
316318
virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive,
317319
ceph::real_time mtime) override;

‎src/rgw/driver/posix/rgw_sal_posix.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2709,8 +2709,10 @@ int POSIXBucket::check_bucket_shards(const DoutPrefixProvider* dpp,
27092709
return 0;
27102710
}
27112711

2712-
int POSIXBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y)
2713-
{
2712+
int POSIXBucket::chown(const DoutPrefixProvider* dpp,
2713+
const rgw_owner& new_owner,
2714+
const std::string& new_owner_name,
2715+
optional_yield y) {
27142716
/* TODO map user to UID/GID, and change it */
27152717
return 0;
27162718
}

‎src/rgw/driver/posix/rgw_sal_posix.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -905,9 +905,12 @@ class POSIXBucket : public StoreBucket {
905905
RGWBucketEnt* ent) override;
906906
virtual int check_bucket_shards(const DoutPrefixProvider* dpp,
907907
uint64_t num_objs, optional_yield y) override;
908-
virtual int chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y) override;
908+
virtual int chown(const DoutPrefixProvider* dpp,
909+
const rgw_owner& new_owner,
910+
const std::string& new_owner_name,
911+
optional_yield y) override;
909912
virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive,
910-
ceph::real_time mtime, optional_yield y) override;
913+
ceph::real_time mtime, optional_yield y) override;
911914
virtual int check_empty(const DoutPrefixProvider* dpp, optional_yield y) override;
912915
virtual int check_quota(const DoutPrefixProvider *dpp, RGWQuota& quota, uint64_t obj_size, optional_yield y, bool check_size_only = false) override;
913916
virtual int try_refresh_info(const DoutPrefixProvider* dpp, ceph::real_time* pmtime, optional_yield y) override;

‎src/rgw/driver/rados/rgw_bucket.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ static void dump_multipart_index_results(std::list<rgw_obj_index_key>& objs,
9797

9898
void check_bad_owner_bucket_mapping(rgw::sal::Driver* driver,
9999
const rgw_owner& owner,
100+
const std::string& owner_name,
100101
const std::string& tenant,
101102
bool fix, optional_yield y,
102103
const DoutPrefixProvider *dpp)
@@ -127,7 +128,7 @@ void check_bad_owner_bucket_mapping(rgw::sal::Driver* driver,
127128
<< " got " << bucket << std::endl;
128129
if (fix) {
129130
cout << "fixing" << std::endl;
130-
r = bucket->chown(dpp, owner, y);
131+
r = bucket->chown(dpp, owner, owner_name, y);
131132
if (r < 0) {
132133
cerr << "failed to fix bucket: " << cpp_strerror(-r) << std::endl;
133134
}

‎src/rgw/driver/rados/rgw_bucket.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ extern int rgw_object_get_attr(rgw::sal::Driver* driver, rgw::sal::Object* obj,
212212

213213
void check_bad_owner_bucket_mapping(rgw::sal::Driver* driver,
214214
const rgw_owner& owner,
215+
const std::string& owner_name,
215216
const std::string& tenant,
216217
bool fix, optional_yield y,
217218
const DoutPrefixProvider *dpp);

‎src/rgw/driver/rados/rgw_sal_rados.cc‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -709,8 +709,10 @@ int RadosBucket::unlink(const DoutPrefixProvider* dpp, const rgw_owner& owner, o
709709
y, dpp, update_entrypoint);
710710
}
711711

712-
int RadosBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y)
713-
{
712+
int RadosBucket::chown(const DoutPrefixProvider* dpp,
713+
const rgw_owner& new_owner,
714+
const std::string& new_owner_name,
715+
optional_yield y) {
714716
// unlink from the owner, but don't update the entrypoint until link()
715717
int r = this->unlink(dpp, info.owner, y, false);
716718
if (r < 0) {
@@ -730,13 +732,26 @@ int RadosBucket::chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner
730732
try {
731733
auto p = i->second.cbegin();
732734

733-
RGWAccessControlPolicy acl;
734-
decode(acl, p);
735+
RGWAccessControlPolicy policy;
736+
decode(policy, p);
737+
//Get the ACL from the policy
738+
RGWAccessControlList& acl = policy.get_acl();
739+
ACLOwner& owner = policy.get_owner();
740+
741+
//Remove grant that is set to old owner
742+
acl.remove_canon_user_grant(owner.id);
743+
744+
//Create a grant and add grant
745+
ACLGrant grant;
746+
grant.set_canon(new_owner, new_owner_name, RGW_PERM_FULL_CONTROL);
747+
acl.add_grant(grant);
735748

736-
acl.get_owner().id = new_owner;
749+
//Update the ACL owner to the new user
750+
owner.id = new_owner;
751+
owner.display_name = new_owner_name;
737752

738753
bufferlist bl;
739-
encode(acl, bl);
754+
encode(policy, bl);
740755

741756
i->second = std::move(bl);
742757
} catch (const buffer::error&) {

‎src/rgw/driver/rados/rgw_sal_rados.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,10 @@ class RadosBucket : public StoreBucket {
745745
RGWBucketEnt* ent) override;
746746
int check_bucket_shards(const DoutPrefixProvider* dpp, uint64_t num_objs,
747747
optional_yield y) override;
748-
virtual int chown(const DoutPrefixProvider* dpp, const rgw_owner& new_owner, optional_yield y) override;
748+
virtual int chown(const DoutPrefixProvider* dpp,
749+
const rgw_owner& new_owner,
750+
const std::string& new_owner_name,
751+
optional_yield y) override;
749752
virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime, optional_yield y) override;
750753
virtual int check_empty(const DoutPrefixProvider* dpp, optional_yield y) override;
751754
virtual int check_quota(const DoutPrefixProvider *dpp, RGWQuota& quota, uint64_t obj_size, optional_yield y, bool check_size_only = false) override;

‎src/rgw/driver/rados/rgw_user.cc‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,8 +1626,8 @@ static int adopt_user_bucket(const DoutPrefixProvider* dpp,
16261626
optional_yield y,
16271627
rgw::sal::Driver* driver,
16281628
const rgw_bucket& bucketid,
1629-
const rgw_owner& new_owner)
1630-
{
1629+
const rgw_owner& new_owner,
1630+
const std::string& new_owner_name) {
16311631
// retry in case of racing writes to the bucket instance metadata
16321632
static constexpr auto max_retries = 10;
16331633
int tries = 0;
@@ -1644,7 +1644,7 @@ static int adopt_user_bucket(const DoutPrefixProvider* dpp,
16441644
return r;
16451645
}
16461646

1647-
r = bucket->chown(dpp, new_owner, y);
1647+
r = bucket->chown(dpp, new_owner, new_owner_name, y);
16481648
if (r < 0) {
16491649
ldpp_dout(dpp, 1) << "failed to chown bucket " << bucketid
16501650
<< ": " << cpp_strerror(r) << dendl;
@@ -1657,8 +1657,8 @@ static int adopt_user_bucket(const DoutPrefixProvider* dpp,
16571657

16581658
static int adopt_user_buckets(const DoutPrefixProvider* dpp, optional_yield y,
16591659
rgw::sal::Driver* driver, const rgw_user& user,
1660-
const rgw_account_id& account_id)
1661-
{
1660+
const rgw_account_id& account_id,
1661+
const std::string& account_name) {
16621662
const size_t max_chunk = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk;
16631663
constexpr bool need_stats = false;
16641664

@@ -1674,7 +1674,8 @@ static int adopt_user_buckets(const DoutPrefixProvider* dpp, optional_yield y,
16741674
}
16751675

16761676
for (const auto& ent : listing.buckets) {
1677-
r = adopt_user_bucket(dpp, y, driver, ent.bucket, account_id);
1677+
r = adopt_user_bucket(dpp, y, driver, ent.bucket, account_id,
1678+
account_name);
16781679
if (r < 0 && r != -ENOENT) {
16791680
return r;
16801681
}
@@ -2107,9 +2108,19 @@ int RGWUser::execute_modify(const DoutPrefixProvider *dpp, RGWUserAdminOpState&
21072108
set_err_msg(err_msg, err);
21082109
return ret;
21092110
}
2111+
RGWAccountInfo account_info;
2112+
rgw::sal::Attrs attrs;
2113+
RGWObjVersionTracker objv;
2114+
int r = driver->load_account_by_id(dpp, y, op_state.account_id,
2115+
account_info,
2116+
attrs, objv);
2117+
if (r < 0) {
2118+
err = "Failed to load account by id";
2119+
return r;
2120+
}
21102121
// change account on user's buckets
21112122
ret = adopt_user_buckets(dpp, y, driver, user_info.user_id,
2112-
user_info.account_id);
2123+
user_info.account_id, account_info.name);
21132124
if (ret < 0) {
21142125
set_err_msg(err_msg, "failed to change ownership of user's buckets");
21152126
return ret;

‎src/rgw/radosgw-admin/radosgw-admin.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9504,7 +9504,8 @@ int main(int argc, const char **argv)
95049504
}
95059505

95069506
if (opt_cmd == OPT::USER_CHECK) {
9507-
check_bad_owner_bucket_mapping(driver, user->get_id(), user->get_tenant(),
9507+
check_bad_owner_bucket_mapping(driver, user->get_id(),
9508+
user->get_display_name(), user->get_tenant(),
95089509
fix, null_yield, dpp());
95099510
}
95109511

0 commit comments

Comments
 (0)