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 <skoduri@redhat.com>

diff --git a/src/rgw/rgw_sal.cc b/src/rgw/rgw_sal.cc
index 78f63156e78d0209c4dd812ae1eba3b8507b4b99..f42254e67d13ed7241a003e885954c6fc3ac4b20 100644 (file)
--- 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<rgw::sal::User> 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 32a9719a49a422350135e3d79ebf82202342e2c6..489aa6c7c0e5a2dc2c99f08b3a6115bf8e4feae9 100644 (file)
--- 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>* 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 580c429b5c1209af95585f0be39828ce802cde73..c13dc280cc31dce965a3b6044519b65ed2a13880 100644 (file)
--- 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 b9f25d323aec990735a9367b1cd1fb86406e6ded..732a7827f37ff8b733abd8488629d3dc09f3ed8e 100644 (file)
--- 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 32c4b6c41ef2bbd61a690ac16a113e94e0b94dc3..dedf8416accdf1e510fa9aa2f3468bbd9077d2f1 100644 (file)
--- 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 d215df6ab9558f8572de031bbea7d4d8b128c05c..692d954b7813902b7c7c460e5895fa01474d1260 100644 (file)
--- 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(" <<query_str.c_str()<<"), user_id(" << uinfo.user_id <<") found" << dendl;
     return -ENOENT;
   }
@@ -335,7 +335,7 @@ int DB::store_user(const DoutPrefixProvider *dpp,
   obj_version& obj_ver = objv_tracker.read_version;
 
   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 && obj_ver.ver) {
     /* already exists. */
@@ -374,6 +374,7 @@ int DB::store_user(const DoutPrefixProvider *dpp,
     ldpp_dout(dpp, 0)<<"store_user failed with err:(" <<ret<<") " << dendl;
     goto out;
   }
+  ldpp_dout(dpp, 20)<<"User creation successful - userid:(" <<uinfo.user_id<<") " << dendl;
 
   if (pobjv) {
     pobjv->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 6668173414a9f433aa9e09954a777a948ca7bf3e..efbad31121d8dedc06ab882f46f741be31c2eded 100644 (file)
--- 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 10f7274984810e9992bfe5d026d34487ed7df6e3..22c6144b50b03ca46cb30fcd3d75114cce541c93 100644 (file)
--- 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);