From: Alex Wojno <awojno@bloomberg.net>
Date: Wed, 6 Dec 2023 16:10:17 +0000 (-0500)
Subject: rgw/iam: fix role deletion replication
X-Git-Tag: testing/wip-batrick-testing-20240411.154038~679^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=037d7aab1de970a998111c07d82464d569800c1f;p=ceph-ci.git

rgw/iam: fix role deletion replication

Signed-off-by: Alex Wojno <awojno@bloomberg.net>
---

diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc
index 5ede8d44fa9..e550efb8684 100644
--- a/src/rgw/driver/rados/rgw_sal_rados.cc
+++ b/src/rgw/driver/rados/rgw_sal_rados.cc
@@ -3677,16 +3677,18 @@ int RadosRole::delete_obj(const DoutPrefixProvider *dpp, optional_yield y)
     return -ERR_DELETE_CONFLICT;
   }
 
-  // Delete id
-  std::string oid = get_info_oid_prefix() + info.id;
-  ret = rgw_delete_system_obj(dpp, store->svc()->sysobj, pool, oid, nullptr, y);
+  // Delete id & insert MD Log
+  RGWSI_MBSObj_RemoveParams params;
+  std::unique_ptr<RGWSI_MetaBackend::Context> ctx(store->svc()->role->svc.meta_be->alloc_ctx());
+  ctx->init(store->svc()->role->get_be_handler());
+  ret = store->svc()->role->svc.meta_be->remove(ctx.get(), info.id, params, &info.objv_tracker, y, dpp);
   if (ret < 0) {
-    ldpp_dout(dpp, 0) << "ERROR: deleting role id from Role pool: "
-                  << info.id << ": " << cpp_strerror(-ret) << dendl;
+    ldpp_dout(dpp, 0) << "ERROR: deleting role id: " << info.id << " failed with code: " << cpp_strerror(-ret) << dendl;
+    return ret;
   }
 
   // Delete name
-  oid = info.tenant + get_names_oid_prefix() + info.name;
+  std::string oid = info.tenant + get_names_oid_prefix() + info.name;
   ret = rgw_delete_system_obj(dpp, store->svc()->sysobj, pool, oid, nullptr, y);
   if (ret < 0) {
     ldpp_dout(dpp, 0) << "ERROR: deleting role name from Role pool: "
diff --git a/src/test/rgw/rgw_multi/tests.py b/src/test/rgw/rgw_multi/tests.py
index 156fac12e7f..c720423e923 100644
--- a/src/test/rgw/rgw_multi/tests.py
+++ b/src/test/rgw/rgw_multi/tests.py
@@ -1705,6 +1705,30 @@ def test_role_sync():
 
             check_role_eq(source_conn, target_conn, role)
 
+def test_role_delete_sync():
+    zonegroup = realm.master_zonegroup()
+    zonegroup_conns = ZonegroupConns(zonegroup)
+    role_name = gen_role_name()
+    log.info('create role zone=%s name=%s', zonegroup_conns.master_zone.name, role_name)
+    zonegroup_conns.master_zone.create_role("", role_name, None, "")
+
+    zonegroup_meta_checkpoint(zonegroup)
+
+    for zone in zonegroup_conns.zones:
+        log.info(f'checking if zone: {zone.name} has role: {role_name}')
+        assert(zone.has_role(role_name))
+        log.info(f'success, zone: {zone.name} has role: {role_name}')
+
+    log.info(f"deleting role: {role_name}")
+    zonegroup_conns.master_zone.delete_role(role_name)
+    zonegroup_meta_checkpoint(zonegroup)
+
+    for zone in zonegroup_conns.zones:
+        log.info(f'checking if zone: {zone.name} does not have role: {role_name}')
+        assert(not zone.has_role(role_name))
+        log.info(f'success, zone: {zone.name} does not have role: {role_name}')
+
+
 @attr('data_sync_init')
 def test_bucket_full_sync_after_data_sync_init():
     zonegroup = realm.master_zonegroup()
diff --git a/src/test/rgw/rgw_multi/zone_cloud.py b/src/test/rgw/rgw_multi/zone_cloud.py
index dd5640cf271..7c94aaa8a60 100644
--- a/src/test/rgw/rgw_multi/zone_cloud.py
+++ b/src/test/rgw/rgw_multi/zone_cloud.py
@@ -304,6 +304,12 @@ class CloudZone(Zone):
         def create_role(self, path, rolename, policy_document, tag_list):
             assert False
 
+        def delete_role(self, role_name):
+            assert False
+
+        def has_role(self, role_name):
+            assert False
+
     def get_conn(self, credentials):
         return self.Conn(self, credentials)
 
diff --git a/src/test/rgw/rgw_multi/zone_es.py b/src/test/rgw/rgw_multi/zone_es.py
index e98b3fdd8fa..84628b775d1 100644
--- a/src/test/rgw/rgw_multi/zone_es.py
+++ b/src/test/rgw/rgw_multi/zone_es.py
@@ -246,6 +246,12 @@ class ESZone(Zone):
         def create_role(self, path, rolename, policy_document, tag_list):
             assert False
 
+        def delete_role(self, role_name):
+            assert False
+
+        def has_role(self, role_name):
+            assert False
+
     def get_conn(self, credentials):
         return self.Conn(self, credentials)
 
diff --git a/src/test/rgw/rgw_multi/zone_rados.py b/src/test/rgw/rgw_multi/zone_rados.py
index ac4edd004d6..7b7fe5228cb 100644
--- a/src/test/rgw/rgw_multi/zone_rados.py
+++ b/src/test/rgw/rgw_multi/zone_rados.py
@@ -1,5 +1,6 @@
 import logging
 from boto.s3.deletemarker import DeleteMarker
+from boto.exception import BotoServerError
 
 from itertools import zip_longest  # type: ignore
 
@@ -127,8 +128,20 @@ class RadosZone(Zone):
             return True
 
         def create_role(self, path, rolename, policy_document, tag_list):
+            if policy_document is None:
+                policy_document = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"AWS\":[\"arn:aws:iam:::user/testuser\"]},\"Action\":[\"sts:AssumeRole\"]}]}"
             return self.iam_conn.create_role(rolename, policy_document, path)
 
+        def delete_role(self, role_name):
+            return self.iam_conn.delete_role(role_name)
+
+        def has_role(self, role_name):
+            try:
+                self.get_role(role_name)
+            except BotoServerError:
+                return False
+            return True
+
     def get_conn(self, credentials):
         return self.Conn(self, credentials)