From 8080bd28e5a9082e9a68272e4455f7fbd1a42531 Mon Sep 17 00:00:00 2001 From: Daniel Gryniewicz Date: Thu, 25 Mar 2021 13:36:14 -0400 Subject: [PATCH] RGW Zipper - move attrs into User Signed-off-by: Daniel Gryniewicz --- src/rgw/rgw_auth.cc | 6 ++--- src/rgw/rgw_file.h | 3 +-- src/rgw/rgw_op.cc | 24 ++++++++--------- src/rgw/rgw_op.h | 3 --- src/rgw/rgw_rest.cc | 5 ++-- src/rgw/rgw_rest_swift.cc | 7 ++--- src/rgw/rgw_rest_user_policy.cc | 47 ++++++++++++--------------------- src/rgw/rgw_sal.h | 7 +++-- src/rgw/rgw_sal_rados.cc | 12 ++++++--- src/rgw/rgw_sal_rados.h | 4 +-- src/rgw/rgw_sts.cc | 4 +-- src/rgw/rgw_user.cc | 12 +++------ src/test/rgw/test_rgw_lua.cc | 2 +- 13 files changed, 58 insertions(+), 78 deletions(-) diff --git a/src/rgw/rgw_auth.cc b/src/rgw/rgw_auth.cc index 75e29bb724c..7b0f68f6a64 100644 --- a/src/rgw/rgw_auth.cc +++ b/src/rgw/rgw_auth.cc @@ -376,8 +376,7 @@ void rgw::auth::WebIdentityApplier::create_account(const DoutPrefixProvider* dpp rgw_apply_default_bucket_quota(user->get_info().bucket_quota, cct->_conf); rgw_apply_default_user_quota(user->get_info().user_quota, cct->_conf); - int ret = user->store_info(dpp, null_yield, - RGWUserCtl::PutParams().set_exclusive(true)); + int ret = user->store_info(dpp, null_yield, true); if (ret < 0) { ldpp_dout(dpp, 0) << "ERROR: failed to store new user info: user=" << user << " ret=" << ret << dendl; @@ -603,8 +602,7 @@ void rgw::auth::RemoteApplier::create_account(const DoutPrefixProvider* dpp, rgw_apply_default_user_quota(user->get_info().user_quota, cct->_conf); user_info = user->get_info(); - int ret = user->store_info(dpp, null_yield, - RGWUserCtl::PutParams().set_exclusive(true)); + int ret = user->store_info(dpp, null_yield, true); if (ret < 0) { ldpp_dout(dpp, 0) << "ERROR: failed to store new user info: user=" << user << " ret=" << ret << dendl; diff --git a/src/rgw/rgw_file.h b/src/rgw/rgw_file.h index f58b3bf6e4e..b4d6b65496e 100644 --- a/src/rgw/rgw_file.h +++ b/src/rgw/rgw_file.h @@ -1005,8 +1005,7 @@ namespace rgw { if (token.valid() && (ldh->auth(token.id, token.key) == 0)) { /* try to store user if it doesn't already exist */ if (user->load_by_id(dpp, null_yield) < 0) { - int ret = user->store_info(dpp, null_yield, RGWUserCtl::PutParams() - .set_exclusive(true)); + int ret = user->store_info(dpp, null_yield, true); if (ret < 0) { lsubdout(get_context(), rgw, 10) << "NOTICE: failed to store new user's info: ret=" << ret diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index e66a5b85719..cc493a2c16c 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -640,12 +640,11 @@ int rgw_build_bucket_policies(const DoutPrefixProvider *dpp, rgw::sal::Store* st /* handle user ACL only for those APIs which support it */ if (s->user_acl) { - rgw::sal::Attrs uattrs; std::unique_ptr acl_user = store->get_user(acct_acl_user.uid); - ret = acl_user->read_attrs(dpp, y, &uattrs); + ret = acl_user->read_attrs(dpp, y); if (!ret) { - ret = get_user_policy_from_attr(s->cct, uattrs, *s->user_acl); + ret = get_user_policy_from_attr(s->cct, acl_user->get_attrs(), *s->user_acl); } if (-ENOENT == ret) { /* In already existing clusters users won't have ACL. In such case @@ -668,10 +667,11 @@ int rgw_build_bucket_policies(const DoutPrefixProvider *dpp, rgw::sal::Store* st // hence the check for user type if (! s->user->get_id().empty() && s->auth.identity->get_identity_type() != TYPE_ROLE) { try { - rgw::sal::Attrs uattrs; - ret = s->user->read_attrs(dpp, y, &uattrs); + ret = s->user->read_attrs(dpp, y); if (ret == 0) { - auto user_policies = get_iam_user_policy_from_attr(s->cct, uattrs, s->user->get_tenant()); + auto user_policies = get_iam_user_policy_from_attr(s->cct, + s->user->get_attrs(), + s->user->get_tenant()); s->iam_user_policies.insert(s->iam_user_policies.end(), std::make_move_iterator(user_policies.begin()), std::make_move_iterator(user_policies.end())); @@ -2288,7 +2288,7 @@ void RGWListBuckets::execute(optional_yield y) } if (supports_account_metadata()) { - op_ret = s->user->read_attrs(this, s->yield, &attrs); + op_ret = s->user->read_attrs(this, s->yield); if (op_ret < 0) { goto send_end; } @@ -4347,10 +4347,11 @@ int RGWPutMetadataAccount::init_processing(optional_yield y) return op_ret; } - op_ret = s->user->read_attrs(this, y, &orig_attrs, &acct_op_tracker); + op_ret = s->user->read_attrs(this, y); if (op_ret < 0) { return op_ret; } + orig_attrs = s->user->get_attrs(); if (has_policy) { bufferlist acl_bl; @@ -4411,7 +4412,6 @@ void RGWPutMetadataAccount::execute(optional_yield y) if (op_ret < 0) { return; } - acct_op_tracker = s->user->get_version_tracker(); /* Handle the TempURL-related stuff. */ if (!temp_url_keys.empty()) { @@ -4427,10 +4427,8 @@ void RGWPutMetadataAccount::execute(optional_yield y) /* We are passing here the current (old) user info to allow the function * optimize-out some operations. */ - op_ret = s->user->store_info(this, y, RGWUserCtl::PutParams() - .set_old_info(&s->user->get_info()) - .set_objv_tracker(&acct_op_tracker) - .set_attrs(&attrs)); + s->user->set_attrs(attrs); + op_ret = s->user->store_info(this, y, false, &s->user->get_info()); } int RGWPutMetadataBucket::verify_permission(optional_yield y) diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index 3de1b5d3b34..229c1f72b33 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -802,7 +802,6 @@ protected: std::string end_marker; int64_t limit; uint64_t limit_max; - rgw::sal::Attrs attrs; bool is_truncated; RGWUsageStats global_stats; @@ -1348,8 +1347,6 @@ protected: RGWQuotaInfo new_quota; bool new_quota_extracted; - RGWObjVersionTracker acct_op_tracker; - RGWAccessControlPolicy policy; bool has_policy; diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index d12f7f4d6a5..2f579f4d2a3 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -1849,9 +1849,8 @@ int RGWHandler_REST::init_permissions(RGWOp* op, optional_yield y) // We don't need user policies in case of STS token returned by AssumeRole, hence the check for user type if (! s->user->get_id().empty() && s->auth.identity->get_identity_type() != TYPE_ROLE) { try { - rgw::sal::Attrs uattrs; - if (auto ret = s->user->read_attrs(s, y, &uattrs); ! ret) { - auto user_policies = get_iam_user_policy_from_attr(s->cct, uattrs, s->user->get_tenant()); + if (auto ret = s->user->read_attrs(s, y); ! ret) { + auto user_policies = get_iam_user_policy_from_attr(s->cct, s->user->get_attrs(), s->user->get_tenant()); s->iam_user_policies.insert(s->iam_user_policies.end(), std::make_move_iterator(user_policies.begin()), std::make_move_iterator(user_policies.end())); diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index d5f3a93d8cc..478695082d4 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -175,7 +175,7 @@ void RGWListBuckets_ObjStore_SWIFT::send_response_begin(bool has_buckets) dump_account_metadata(s, global_stats, policies_stats, - attrs, + s->user->get_attrs(), s->user->get_info().user_quota, static_cast(*s->user_acl)); dump_errno(s); @@ -281,7 +281,7 @@ void RGWListBuckets_ObjStore_SWIFT::send_response_end() dump_account_metadata(s, global_stats, policies_stats, - attrs, + s->user->get_attrs(), s->user->get_info().user_quota, static_cast(*s->user_acl)); dump_errno(s); @@ -545,7 +545,8 @@ static void dump_container_metadata(struct req_state *s, void RGWStatAccount_ObjStore_SWIFT::execute(optional_yield y) { RGWStatAccount_ObjStore::execute(y); - op_ret = s->user->read_attrs(s, s->yield, &attrs); + op_ret = s->user->read_attrs(s, s->yield); + attrs = s->user->get_attrs(); } void RGWStatAccount_ObjStore_SWIFT::send_response() diff --git a/src/rgw/rgw_rest_user_policy.cc b/src/rgw/rgw_rest_user_policy.cc index a56ac49cbe1..eb55313da73 100644 --- a/src/rgw/rgw_rest_user_policy.cc +++ b/src/rgw/rgw_rest_user_policy.cc @@ -126,8 +126,7 @@ void RGWPutUserPolicy::execute(optional_yield y) return; } - rgw::sal::Attrs uattrs; - op_ret = user->read_attrs(s, s->yield, &uattrs); + op_ret = user->read_attrs(s, s->yield); if (op_ret == -ENOENT) { op_ret = -ERR_NO_SUCH_ENTITY; return; @@ -143,20 +142,16 @@ void RGWPutUserPolicy::execute(optional_yield y) try { const Policy p(s->cct, s->user->get_tenant(), bl); map policies; - if (auto it = uattrs.find(RGW_ATTR_USER_POLICY); it != uattrs.end()) { - bufferlist out_bl = uattrs[RGW_ATTR_USER_POLICY]; + if (auto it = user->get_attrs().find(RGW_ATTR_USER_POLICY); it != user->get_attrs().end()) { + bufferlist out_bl = it->second; decode(policies, out_bl); } bufferlist in_bl; policies[policy_name] = policy; encode(policies, in_bl); - uattrs[RGW_ATTR_USER_POLICY] = in_bl; + user->get_attrs()[RGW_ATTR_USER_POLICY] = in_bl; - RGWObjVersionTracker objv_tracker; - op_ret = user->store_info(s, s->yield, - RGWUserCtl::PutParams() - .set_objv_tracker(&objv_tracker) - .set_attrs(&uattrs)); + op_ret = user->store_info(s, s->yield, false); if (op_ret < 0) { op_ret = -ERR_INTERNAL_ERROR; } @@ -201,8 +196,7 @@ void RGWGetUserPolicy::execute(optional_yield y) } std::unique_ptr user = store->get_user(rgw_user(user_name)); - rgw::sal::Attrs uattrs; - op_ret = user->read_attrs(s, s->yield, &uattrs); + op_ret = user->read_attrs(s, s->yield); if (op_ret == -ENOENT) { ldpp_dout(this, 0) << "ERROR: attrs not found for user" << user_name << dendl; op_ret = -ERR_NO_SUCH_ENTITY; @@ -216,8 +210,8 @@ void RGWGetUserPolicy::execute(optional_yield y) s->formatter->close_section(); s->formatter->open_object_section("GetUserPolicyResult"); map policies; - if (auto it = uattrs.find(RGW_ATTR_USER_POLICY); it != uattrs.end()) { - bufferlist bl = uattrs[RGW_ATTR_USER_POLICY]; + if (auto it = user->get_attrs().find(RGW_ATTR_USER_POLICY); it != user->get_attrs().end()) { + bufferlist bl = it->second; decode(policies, bl); if (auto it = policies.find(policy_name); it != policies.end()) { policy = policies[policy_name]; @@ -265,8 +259,7 @@ void RGWListUserPolicies::execute(optional_yield y) } std::unique_ptr user = store->get_user(rgw_user(user_name)); - rgw::sal::Attrs uattrs; - op_ret = user->read_attrs(s, s->yield, &uattrs); + op_ret = user->read_attrs(s, s->yield); if (op_ret == -ENOENT) { ldpp_dout(this, 0) << "ERROR: attrs not found for user" << user_name << dendl; op_ret = -ERR_NO_SUCH_ENTITY; @@ -275,13 +268,13 @@ void RGWListUserPolicies::execute(optional_yield y) if (op_ret == 0) { map policies; - if (auto it = uattrs.find(RGW_ATTR_USER_POLICY); it != uattrs.end()) { + if (auto it = user->get_attrs().find(RGW_ATTR_USER_POLICY); it != user->get_attrs().end()) { s->formatter->open_object_section("ListUserPoliciesResponse"); s->formatter->open_object_section("ResponseMetadata"); s->formatter->dump_string("RequestId", s->trans_id); s->formatter->close_section(); s->formatter->open_object_section("ListUserPoliciesResult"); - bufferlist bl = uattrs[RGW_ATTR_USER_POLICY]; + bufferlist bl = it->second; decode(policies, bl); for (const auto& p : policies) { s->formatter->open_object_section("PolicyNames"); @@ -333,8 +326,7 @@ void RGWDeleteUserPolicy::execute(optional_yield y) return; } - rgw::sal::Attrs uattrs; - op_ret = user->read_attrs(this, s->yield, &uattrs); + op_ret = user->read_attrs(this, s->yield); if (op_ret == -ENOENT) { op_ret = -ERR_NO_SUCH_ENTITY; return; @@ -353,22 +345,17 @@ void RGWDeleteUserPolicy::execute(optional_yield y) } map policies; - if (auto it = uattrs.find(RGW_ATTR_USER_POLICY); it != uattrs.end()) { - bufferlist out_bl = uattrs[RGW_ATTR_USER_POLICY]; + if (auto it = user->get_attrs().find(RGW_ATTR_USER_POLICY); it != user->get_attrs().end()) { + bufferlist out_bl = it->second; decode(policies, out_bl); if (auto p = policies.find(policy_name); p != policies.end()) { bufferlist in_bl; policies.erase(p); encode(policies, in_bl); - uattrs[RGW_ATTR_USER_POLICY] = in_bl; - - RGWObjVersionTracker objv_tracker; - op_ret = user->store_info(s, s->yield, - RGWUserCtl::PutParams() - .set_old_info(&user->get_info()) - .set_objv_tracker(&objv_tracker) - .set_attrs(&uattrs)); + user->get_attrs()[RGW_ATTR_USER_POLICY] = in_bl; + + op_ret = user->store_info(s, s->yield, false); if (op_ret < 0) { op_ret = -ERR_INTERNAL_ERROR; } diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 1a00e407ffb..1bc47c84ad3 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -242,6 +242,7 @@ class User { protected: RGWUserInfo info; RGWObjVersionTracker objv_tracker; + Attrs attrs; public: User() : info() {} @@ -270,7 +271,7 @@ class User { const RGWUserCaps& get_caps() const { return info.caps; } static bool empty(User* u) { return (!u || u->info.user_id.id.empty()); } static bool empty(std::unique_ptr& u) { return (!u || u->info.user_id.id.empty()); } - virtual int read_attrs(const DoutPrefixProvider* dpp, optional_yield y, Attrs* uattrs, RGWObjVersionTracker* tracker = nullptr) = 0; + virtual int read_attrs(const DoutPrefixProvider* dpp, optional_yield y) = 0; virtual int read_stats(optional_yield y, RGWStorageStats* stats, ceph::real_time* last_stats_sync = nullptr, ceph::real_time* last_stats_update = nullptr) = 0; @@ -281,10 +282,12 @@ class User { map& usage) = 0; virtual int trim_usage(uint64_t start_epoch, uint64_t end_epoch) = 0; virtual RGWObjVersionTracker& get_version_tracker() { return objv_tracker; } + virtual Attrs& get_attrs() { return attrs; } + virtual void set_attrs(Attrs& _attrs) { attrs = _attrs; } /* Placeholders */ virtual int load_by_id(const DoutPrefixProvider* dpp, optional_yield y) = 0; - virtual int store_info(const DoutPrefixProvider* dpp, optional_yield y, const RGWUserCtl::PutParams& params = {}) = 0; + virtual int store_info(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive, RGWUserInfo* old_info = nullptr) = 0; virtual int remove_info(const DoutPrefixProvider* dpp, optional_yield y, const RGWUserCtl::RemoveParams& params = {}) = 0; /* dang temporary; will be removed when User is complete */ diff --git a/src/rgw/rgw_sal_rados.cc b/src/rgw/rgw_sal_rados.cc index 486b875782b..a8d0c15cc76 100644 --- a/src/rgw/rgw_sal_rados.cc +++ b/src/rgw/rgw_sal_rados.cc @@ -148,9 +148,9 @@ Bucket* RadosUser::create_bucket(rgw_bucket& bucket, return NULL; } -int RadosUser::read_attrs(const DoutPrefixProvider* dpp, optional_yield y, Attrs* uattrs, RGWObjVersionTracker* tracker) +int RadosUser::read_attrs(const DoutPrefixProvider* dpp, optional_yield y) { - return store->ctl()->user->get_attrs_by_uid(dpp, get_id(), uattrs, y, tracker); + return store->ctl()->user->get_attrs_by_uid(dpp, get_id(), &attrs, y, &objv_tracker); } int RadosUser::read_stats(optional_yield y, RGWStorageStats* stats, @@ -193,9 +193,13 @@ int RadosUser::load_by_id(const DoutPrefixProvider* dpp, optional_yield y) return store->ctl()->user->get_info_by_uid(dpp, info.user_id, &info, y, RGWUserCtl::GetParams().set_objv_tracker(&objv_tracker)); } -int RadosUser::store_info(const DoutPrefixProvider* dpp, optional_yield y, const RGWUserCtl::PutParams& params) +int RadosUser::store_info(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive, RGWUserInfo* old_info) { - return store->ctl()->user->store_info(dpp, info, y, params); + return store->ctl()->user->store_info(dpp, info, y, + RGWUserCtl::PutParams().set_objv_tracker(&objv_tracker) + .set_exclusive(exclusive) + .set_attrs(&attrs) + .set_old_info(old_info)); } int RadosUser::remove_info(const DoutPrefixProvider* dpp, optional_yield y, const RGWUserCtl::RemoveParams& params) diff --git a/src/rgw/rgw_sal_rados.h b/src/rgw/rgw_sal_rados.h index 7b5e0c5e4bd..5f8f9219a0a 100644 --- a/src/rgw/rgw_sal_rados.h +++ b/src/rgw/rgw_sal_rados.h @@ -52,7 +52,7 @@ class RadosUser : public User { uint64_t max, bool need_stats, BucketList& buckets, optional_yield y) override; virtual Bucket* create_bucket(rgw_bucket& bucket, ceph::real_time creation_time) override; - virtual int read_attrs(const DoutPrefixProvider* dpp, optional_yield y, Attrs* uattrs, RGWObjVersionTracker* tracker) override; + virtual int read_attrs(const DoutPrefixProvider* dpp, optional_yield y) override; virtual int read_stats(optional_yield y, RGWStorageStats* stats, ceph::real_time* last_stats_sync = nullptr, ceph::real_time* last_stats_update = nullptr) override; @@ -65,7 +65,7 @@ class RadosUser : public User { /* Placeholders */ virtual int load_by_id(const DoutPrefixProvider* dpp, optional_yield y) override; - virtual int store_info(const DoutPrefixProvider* dpp, optional_yield y, const RGWUserCtl::PutParams& params = {}) override; + virtual int store_info(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive, RGWUserInfo* old_info = nullptr) override; virtual int remove_info(const DoutPrefixProvider* dpp, optional_yield y, const RGWUserCtl::RemoveParams& params = {}) override; friend class RadosBucket; diff --git a/src/rgw/rgw_sts.cc b/src/rgw/rgw_sts.cc index c7863c57e71..bd0487724d7 100644 --- a/src/rgw/rgw_sts.cc +++ b/src/rgw/rgw_sts.cc @@ -322,9 +322,7 @@ int STSService::storeARN(const DoutPrefixProvider *dpp, string& arn, optional_yi user->get_info().assumed_role_arn = arn; - ret = user->store_info(dpp, y, RGWUserCtl::PutParams() - .set_old_info(&user->get_info()) - .set_exclusive(false)); + ret = user->store_info(dpp, y, false, &user->get_info()); if (ret < 0) { return -ERR_INTERNAL_ERROR; } diff --git a/src/rgw/rgw_user.cc b/src/rgw/rgw_user.cc index 0c46d9fb836..de71570f503 100644 --- a/src/rgw/rgw_user.cc +++ b/src/rgw/rgw_user.cc @@ -1565,9 +1565,8 @@ int RGWUser::update(const DoutPrefixProvider *dpp, RGWUserAdminOpState& op_state RGWUserInfo *pold_info = (is_populated() ? &old_info : nullptr); - ret = user->store_info(dpp, y, RGWUserCtl::PutParams() - .set_old_info(pold_info) - .set_objv_tracker(&op_state.objv)); + ret = user->store_info(dpp, y, false, pold_info); + op_state.objv = user->get_version_tracker(); if (ret < 0) { set_err_msg(err_msg, "unable to store user info"); return ret; @@ -1663,12 +1662,9 @@ int RGWUser::execute_rename(const DoutPrefixProvider *dpp, RGWUserAdminOpState& std::unique_ptr user; user = store->get_user(new_user->get_id()); - RGWObjVersionTracker objv; const bool exclusive = !op_state.get_overwrite_new_user(); // overwrite if requested - ret = user->store_info(dpp, y, RGWUserCtl::PutParams() - .set_objv_tracker(&objv) - .set_exclusive(exclusive)); + ret = user->store_info(dpp, y, exclusive); if (ret == -EEXIST) { set_err_msg(err_msg, "user name given by --new-uid already exists"); return ret; @@ -1731,7 +1727,7 @@ int RGWUser::execute_rename(const DoutPrefixProvider *dpp, RGWUserAdminOpState& // associated index objects RGWUserInfo& user_info = op_state.get_user_info(); user_info.user_id = new_user->get_id(); - op_state.objv = objv; + op_state.objv = user->get_version_tracker(); rename_swift_keys(new_user->get_id(), user_info.swift_keys); diff --git a/src/test/rgw/test_rgw_lua.cc b/src/test/rgw/test_rgw_lua.cc index 88dc52594b7..ad7c0eb8e04 100644 --- a/src/test/rgw/test_rgw_lua.cc +++ b/src/test/rgw/test_rgw_lua.cc @@ -34,7 +34,7 @@ public: return nullptr; } - virtual int read_attrs(const DoutPrefixProvider *dpp, optional_yield y, sal::Attrs* uattrs, RGWObjVersionTracker* tracker) override { + virtual int read_attrs(const DoutPrefixProvider *dpp, optional_yield y) override { return 0; } -- 2.39.5