From 450a6f0b419b95f45ec9297289d22caf027e57c9 Mon Sep 17 00:00:00 2001 From: Soumya Koduri Date: Tue, 3 May 2022 20:00:41 +0530 Subject: [PATCH] rgw/dbstore: Support user creation via `radosgw-admin` With the changes in https://github.com/ceph/ceph/pull/45987 , 'radosgw-admin' command can be used to execute few admin operations on other stores. This fix include changes to support user creation/remove via `radosgw-admin` command in dbstore. Also fixed an issue with updating objv_tracker in op_state.user Signed-off-by: Soumya Koduri --- src/rgw/rgw_sal.cc | 19 ++++++------------- src/rgw/rgw_sal_dbstore.cc | 6 +++--- src/rgw/rgw_user.cc | 9 +++++++++ src/rgw/rgw_user.h | 2 ++ src/rgw/store/dbstore/README.md | 8 +++++--- src/rgw/store/dbstore/common/dbstore.cc | 17 +++++++++++------ src/rgw/store/dbstore/sqlite/sqliteDB.cc | 6 +++--- src/rgw/store/dbstore/tests/dbstore_tests.cc | 5 ++--- 8 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/rgw/rgw_sal.cc b/src/rgw/rgw_sal.cc index 78f63156e78d0..f42254e67d13e 100644 --- a/src/rgw/rgw_sal.cc +++ b/src/rgw/rgw_sal.cc @@ -147,19 +147,6 @@ rgw::sal::Store* StoreManager::init_storage_provider(const DoutPrefixProvider* d return nullptr; } - /* XXX: temporary - create testid user */ - rgw_user testid_user("", "testid", ""); - std::unique_ptr user = store->get_user(testid_user); - user->get_info().display_name = "M. Tester"; - user->get_info().user_email = "tester@ceph.com"; - RGWAccessKey k1("0555b35654ad1656d804", "h7GhxuBLTrlhVUyxSPUKUV8r/2EI4ngqJxD7iBdBYLhwluN30JaT3Q=="); - user->get_info().access_keys["0555b35654ad1656d804"] = k1; - user->get_info().max_buckets = RGW_DEFAULT_MAX_BUCKETS; - - int r = user->store_user(dpp, null_yield, true); - if (r < 0) { - ldpp_dout(dpp, 0) << "ERROR: failed inserting testid user in dbstore error r=" << r << dendl; - } return store; } #endif @@ -238,6 +225,12 @@ rgw::sal::Store* StoreManager::init_raw_storage_provider(const DoutPrefixProvide if (svc.compare("dbstore") == 0) { #ifdef WITH_RADOSGW_DBSTORE store = newDBStore(cct); + + if ((*(rgw::sal::DBStore*)store).initialize(cct, dpp) < 0) { + delete store; + return nullptr; + } + #else store = nullptr; #endif diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index 32a9719a49a42..489aa6c7c0e5a 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -156,7 +156,7 @@ namespace rgw::sal { int DBUser::read_attrs(const DoutPrefixProvider* dpp, optional_yield y) { int ret; - ret = store->getDB()->get_user(dpp, string("user_id"), "", info, &attrs, + ret = store->getDB()->get_user(dpp, string("user_id"), get_id().id, info, &attrs, &objv_tracker); return ret; } @@ -196,7 +196,7 @@ namespace rgw::sal { { int ret = 0; - ret = store->getDB()->get_user(dpp, string("user_id"), "", info, &attrs, + ret = store->getDB()->get_user(dpp, string("user_id"), get_id().id, info, &attrs, &objv_tracker); return ret; @@ -1660,7 +1660,7 @@ namespace rgw::sal { int DBStore::get_user_by_swift(const DoutPrefixProvider *dpp, const std::string& user_str, optional_yield y, std::unique_ptr* user) { /* Swift keys and subusers are not supported for now */ - return 0; + return -ENOTSUP; } std::string DBStore::get_cluster_id(const DoutPrefixProvider* dpp, optional_yield y) diff --git a/src/rgw/rgw_user.cc b/src/rgw/rgw_user.cc index 580c429b5c120..c13dc280cc31d 100644 --- a/src/rgw/rgw_user.cc +++ b/src/rgw/rgw_user.cc @@ -421,6 +421,11 @@ void RGWUserAdminOpState::set_user_info(RGWUserInfo& user_info) user->get_info() = user_info; } +void RGWUserAdminOpState::set_user_version_tracker(RGWObjVersionTracker& objv_tracker) +{ + user->get_version_tracker() = objv_tracker; +} + const rgw_user& RGWUserAdminOpState::get_user_id() { return user->get_id(); @@ -1515,6 +1520,7 @@ int RGWUser::init(const DoutPrefixProvider *dpp, RGWUserAdminOpState& op_state, op_state.set_user_info(user->get_info()); op_state.set_populated(); op_state.objv = user->get_version_tracker(); + op_state.set_user_version_tracker(user->get_version_tracker()); old_info = user->get_info(); set_populated(); @@ -1568,6 +1574,8 @@ int RGWUser::update(const DoutPrefixProvider *dpp, RGWUserAdminOpState& op_state ret = user->store_user(dpp, y, false, pold_info); op_state.objv = user->get_version_tracker(); + op_state.set_user_version_tracker(user->get_version_tracker()); + if (ret < 0) { set_err_msg(err_msg, "unable to store user info"); return ret; @@ -1717,6 +1725,7 @@ int RGWUser::execute_rename(const DoutPrefixProvider *dpp, RGWUserAdminOpState& RGWUserInfo& user_info = op_state.get_user_info(); user_info.user_id = new_user->get_id(); op_state.objv = user->get_version_tracker(); + op_state.set_user_version_tracker(user->get_version_tracker()); rename_swift_keys(new_user->get_id(), user_info.swift_keys); diff --git a/src/rgw/rgw_user.h b/src/rgw/rgw_user.h index b9f25d323aec9..732a7827f37ff 100644 --- a/src/rgw/rgw_user.h +++ b/src/rgw/rgw_user.h @@ -299,6 +299,8 @@ struct RGWUserAdminOpState { void set_user_info(RGWUserInfo& user_info); + void set_user_version_tracker(RGWObjVersionTracker& objv_tracker); + void set_max_buckets(int32_t mb) { max_buckets = mb; max_buckets_specified = true; diff --git a/src/rgw/store/dbstore/README.md b/src/rgw/store/dbstore/README.md index 32c4b6c41ef2b..dedf8416accdf 100644 --- a/src/rgw/store/dbstore/README.md +++ b/src/rgw/store/dbstore/README.md @@ -20,11 +20,13 @@ Edit ceph.conf to add below option [client] rgw backend store = dbstore -Restart vstart cluster or just RGW server +Start vstart cluster - [..] RGW=1 ../src/vstart.sh -d + [..] RGW=1 ../src/vstart.sh -o rgw_backend_store=dbstore -n -d -The above configuration brings up RGW server on dbstore and creates testid user to be used for s3 operations. +The above vstart command brings up RGW server on dbstore and creates few default users (eg., testid) to be used for s3 operations. + +`radosgw-admin` can be used to create and remove other users. By default, dbstore creates .db file *'/var/run/ceph/dbstore-default_ns.db'* to store the data. This can be configured using below options in ceph.conf diff --git a/src/rgw/store/dbstore/common/dbstore.cc b/src/rgw/store/dbstore/common/dbstore.cc index d215df6ab9558..692d954b78139 100644 --- a/src/rgw/store/dbstore/common/dbstore.cc +++ b/src/rgw/store/dbstore/common/dbstore.cc @@ -268,9 +268,8 @@ int DB::get_user(const DoutPrefixProvider *dpp, RGWObjVersionTracker *pobjv_tracker) { int ret = 0; - if (query_str.empty()) { - // not checking for query_str_val as the query can be to fetch - // entries with null values + if (query_str.empty() || query_str_val.empty()) { + ldpp_dout(dpp, 0)<<"In GetUser - Invalid query(" << query_str <<"), query_str_val(" << query_str_val <<")" << dendl; return -1; } @@ -302,7 +301,8 @@ int DB::get_user(const DoutPrefixProvider *dpp, goto out; /* Verify if its a valid user */ - if (params.op.user.uinfo.access_keys.empty()) { + if (params.op.user.uinfo.access_keys.empty() || + params.op.user.uinfo.user_id.id.empty()) { ldpp_dout(dpp, 0)<<"In GetUser - No user with query(" <read_version = obj_ver; @@ -395,7 +396,11 @@ int DB::remove_user(const DoutPrefixProvider *dpp, RGWObjVersionTracker objv_tracker = {}; orig_info.user_id = uinfo.user_id; - ret = get_user(dpp, string("user_id"), "", orig_info, nullptr, &objv_tracker); + ret = get_user(dpp, string("user_id"), uinfo.user_id.id, orig_info, nullptr, &objv_tracker); + + if (ret) { + return ret; + } if (!ret && objv_tracker.read_version.ver) { /* already exists. */ diff --git a/src/rgw/store/dbstore/sqlite/sqliteDB.cc b/src/rgw/store/dbstore/sqlite/sqliteDB.cc index 6668173414a9f..efbad31121d8d 100644 --- a/src/rgw/store/dbstore/sqlite/sqliteDB.cc +++ b/src/rgw/store/dbstore/sqlite/sqliteDB.cc @@ -369,9 +369,9 @@ static int list_user(const DoutPrefixProvider *dpp, DBOpInfo &op, sqlite3_stmt * op.user.uinfo.display_name = (const char*)sqlite3_column_text(stmt, DisplayName); // user_name op.user.uinfo.user_email = (const char*)sqlite3_column_text(stmt, UserEmail); - SQL_DECODE_BLOB_PARAM(dpp, stmt, AccessKeys, op.user.uinfo.access_keys, sdb); SQL_DECODE_BLOB_PARAM(dpp, stmt, SwiftKeys, op.user.uinfo.swift_keys, sdb); SQL_DECODE_BLOB_PARAM(dpp, stmt, SubUsers, op.user.uinfo.subusers, sdb); + SQL_DECODE_BLOB_PARAM(dpp, stmt, AccessKeys, op.user.uinfo.access_keys, sdb); op.user.uinfo.suspended = sqlite3_column_int(stmt, Suspended); op.user.uinfo.max_buckets = sqlite3_column_int(stmt, MaxBuckets); @@ -1144,9 +1144,9 @@ int SQLInsertUser::Bind(const DoutPrefixProvider *dpp, struct DBOpParams *params SQL_BIND_INDEX(dpp, stmt, index, p_params.op.user.access_keys_secret, sdb); SQL_BIND_TEXT(dpp, stmt, index, key.c_str(), sdb); - SQL_BIND_INDEX(dpp, stmt, index, p_params.op.user.access_keys, sdb); - SQL_ENCODE_BLOB_PARAM(dpp, stmt, index, params->op.user.uinfo.access_keys, sdb); } + SQL_BIND_INDEX(dpp, stmt, index, p_params.op.user.access_keys, sdb); + SQL_ENCODE_BLOB_PARAM(dpp, stmt, index, params->op.user.uinfo.access_keys, sdb); SQL_BIND_INDEX(dpp, stmt, index, p_params.op.user.swift_keys, sdb); SQL_ENCODE_BLOB_PARAM(dpp, stmt, index, params->op.user.uinfo.swift_keys, sdb); diff --git a/src/rgw/store/dbstore/tests/dbstore_tests.cc b/src/rgw/store/dbstore/tests/dbstore_tests.cc index 10f7274984810..22c6144b50b03 100644 --- a/src/rgw/store/dbstore/tests/dbstore_tests.cc +++ b/src/rgw/store/dbstore/tests/dbstore_tests.cc @@ -268,7 +268,7 @@ TEST_F(DBStoreTest, StoreUser) { uinfo.access_keys["id2"] = k2; /* non exclusive create..should create new one */ - ret = db->store_user(dpp, uinfo, true, &attrs, &objv_tracker, &old_uinfo); + ret = db->store_user(dpp, uinfo, false, &attrs, &objv_tracker, &old_uinfo); ASSERT_EQ(ret, 0); ASSERT_EQ(old_uinfo.user_email, ""); ASSERT_EQ(objv_tracker.read_version.ver, 1); @@ -305,7 +305,7 @@ TEST_F(DBStoreTest, GetUserQueryByUserID) { uinfo.user_id.tenant = "tenant"; uinfo.user_id.id = "user_id2"; - ret = db->get_user(dpp, "user_id", "", uinfo, &attrs, &objv); + ret = db->get_user(dpp, "user_id", "user_id2", uinfo, &attrs, &objv); ASSERT_EQ(ret, 0); ASSERT_EQ(uinfo.user_id.tenant, "tenant"); ASSERT_EQ(uinfo.user_email, "user2_new@dbstore.com"); @@ -629,7 +629,6 @@ TEST_F(DBStoreTest, RemoveUserAPI) { ret = db->remove_user(dpp, uinfo, &objv); ASSERT_EQ(ret, -125); - /* invalid version number...should fail */ objv.read_version.ver = 2; ret = db->remove_user(dpp, uinfo, &objv); ASSERT_EQ(ret, 0); -- 2.39.5