[RHCLOUD-35855] System roles assigments in migrator

rbac/management/group/relation_api_dual_write_group_handler.py

@@ -93,13 +93,17 @@ def _generate_member_relations(self):
 
         return relations
 
+    def generate_relations_to_add_principals(self, principals: list[Principal]):
+        """Generate relations to add principals."""
+        logger.info("[Dual Write] Generate new relations from Group(%s): '%s'", self.group.uuid, self.group.name)
+        self.principals = principals
+        self.group_relations_to_add = self._generate_member_relations()
+
     def replicate_new_principals(self, principals: list[Principal]):
         """Replicate new principals into group."""
         if not self.replication_enabled():
             return
-        logger.info("[Dual Write] Generate new relations from Group(%s): '%s'", self.group.uuid, self.group.name)
-        self.principals = principals
-        self.group_relations_to_add = self._generate_member_relations()
+        self.generate_relations_to_add_principals(principals)
         self._replicate()
 
     def replicate_removed_principals(self, principals: list[Principal]):

rbac/management/relation_replicator/relation_replicator.py

@@ -52,6 +52,7 @@ class ReplicationEventType(str, Enum):
     MIGRATE_CUSTOM_ROLE = "migrate_custom_role"
     MIGRATE_TENANT_GROUPS = "migrate_tenant_groups"
     CUSTOMIZE_DEFAULT_GROUP = "customize_default_group"
+    MIGRATE_SYSTEM_ROLE_ASSIGMENT = "migrate_system_role_assignment"
 
 
 class ReplicationEvent:

rbac/migration_tool/migrate.py

@@ -18,7 +18,10 @@
 import logging
 from typing import Iterable
 
+from django.conf import settings
+from django.db import transaction
 from kessel.relations.v1beta1 import common_pb2
+from management.group.relation_api_dual_write_group_handler import RelationApiDualWriteGroupHandler
 from management.models import Workspace
 from management.principal.model import Principal
 from management.relation_replicator.logging_replicator import LoggingReplicator
@@ -94,30 +97,33 @@ def migrate_role(
     return relationships, v2_role_bindings
 
 
-def migrate_users_for_groups(tenant: Tenant) -> list[common_pb2.Relationship]:
-    """Write users relationship to groups."""
-    relationships: list[common_pb2.Relationship] = []
-    for group in tenant.group_set.exclude(platform_default=True):
-        user_set: Iterable[Principal] = group.principals.all()
-        for user in user_set:
-            if (relationship := group.relationship_to_principal(user)) is not None:
-                relationships.append(relationship)
-    return relationships
+def migrate_groups_for_tenant(tenant: Tenant, replicator: RelationReplicator):
+    """Generate user relationships and system role assignments for groups in a tenant."""
+    groups = tenant.group_set.all()
+    for group in groups:
+        principals: list[Principal] = []
+        system_roles: list[Role] = []
+        if not group.platform_default:
+            principals = group.principals.all()
+        if group.system is False and group.admin_default is False:
+            system_roles = group.roles().filter(system=True)
+        if any(True for _ in system_roles) or any(True for _ in principals):
+            # The migrator does not generally deal with concurrency control,
+            # but we require an atomic block due to use of select_for_update in the dual write handler.
+            with transaction.atomic():
+                dual_write_handler = RelationApiDualWriteGroupHandler(
+                    group, ReplicationEventType.MIGRATE_TENANT_GROUPS, replicator=replicator
+                )
+                dual_write_handler.generate_relations_to_add_principals(principals)
+                dual_write_handler.generate_relations_to_add_roles(system_roles)
+                dual_write_handler.replicate()
 
 
 def migrate_data_for_tenant(tenant: Tenant, exclude_apps: list, replicator: RelationReplicator):
     """Migrate all data for a given tenant."""
     logger.info("Migrating relations of group and user.")
 
-    tuples = migrate_users_for_groups(tenant)
-    replicator.replicate(
-        ReplicationEvent(
-            event_type=ReplicationEventType.MIGRATE_TENANT_GROUPS,
-            info={"tenant": tenant.org_id},
-            partition_key=PartitionKey.byEnvironment(),
-            add=tuples,
-        )
-    )
+    migrate_groups_for_tenant(tenant, replicator)
 
     logger.info("Finished migrating relations of group and user.")
 
@@ -155,6 +161,10 @@ def migrate_data_for_tenant(tenant: Tenant, exclude_apps: list, replicator: Rela
 
 def migrate_data(exclude_apps: list = [], orgs: list = [], write_relationships: str = "False"):
     """Migrate all data for all tenants."""
+    if not settings.READ_ONLY_API_MODE:
+        logger.fatal("Read-only API mode is required. READ_ONLY_API_MODE must be set to true.")
+        return
+
     count = 0
     tenants = Tenant.objects.exclude(tenant_name="public")
     replicator = _get_replicator(write_relationships)

tests/migration_tool/tests_migrate.py

@@ -15,13 +15,17 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 #
 """Test the utils module."""
+from platform import system
 from unittest.mock import Mock, call, patch
 
-from django.test import TestCase
+from uuid import uuid4
+
+from django.test import TestCase, override_settings
 
 from api.models import Tenant
 from management.models import *
 from migration_tool.migrate import migrate_data
+from management.group.definer import seed_group, clone_default_group_in_public_schema
 
 
 class MigrateTests(TestCase):
@@ -30,25 +34,42 @@ class MigrateTests(TestCase):
     def setUp(self):
         """Set up the utils tests."""
         super().setUp()
+        # public tenant
         public_tenant = Tenant.objects.get(tenant_name="public")
-        Group.objects.create(name="default", tenant=public_tenant, platform_default=True)
+
+        # system roles
+        self.system_role_1 = Role.objects.create(
+            name="System Role 1", platform_default=True, tenant=public_tenant, system=True
+        )
+        self.system_role_2 = Role.objects.create(
+            name="System Role 2", platform_default=True, system=True, tenant=public_tenant
+        )
+        # default group
+        default_group, _ = seed_group()
+        # permissions
         # This would be skipped
         permission1 = Permission.objects.create(permission="app1:hosts:read", tenant=public_tenant)
         permission2 = Permission.objects.create(permission="inventory:hosts:write", tenant=public_tenant)
-        # Two organization
+        Access.objects.bulk_create(
+            [
+                Access(permission=permission1, role=self.system_role_2, tenant=public_tenant),
+                Access(permission=permission2, role=self.system_role_2, tenant=public_tenant),
+            ]
+        )
+
+        # two organizations
+        # tenant 1 - org_id=1234567
         self.tenant = Tenant.objects.create(org_id="1234567", tenant_name="tenant")
         self.root_workspace = Workspace.objects.create(
             type=Workspace.Types.ROOT, tenant=self.tenant, name="Root Workspace"
         )
         self.default_workspace = Workspace.objects.create(
             type=Workspace.Types.DEFAULT, tenant=self.tenant, name="Default Workspace", parent=self.root_workspace
         )
-
-        another_tenant = Tenant.objects.create(org_id="7654321")
-
         # setup data for organization 1234567
         self.workspace_id_1 = "123456"
         self.workspace_id_2 = "654321"
+
         # This role will be skipped because it contains permission with skipping application
         self.role_a1 = Role.objects.create(name="role_a1", tenant=self.tenant)
         self.access_a11 = Access.objects.create(permission=permission1, role=self.role_a1, tenant=self.tenant)
@@ -82,19 +103,21 @@ def setUp(self):
         self.group_a2.principals.add(self.principal1, self.principal2)
         self.policy_a2 = Policy.objects.create(name="System Policy_a2", group=self.group_a2, tenant=self.tenant)
         self.policy_a2.roles.add(self.role_a2)
-        self.policy_a2.save()
+
+        # tenant 2 - org_id=7654321
+        another_tenant = Tenant.objects.create(org_id="7654321")
 
         # setup data for another tenant 7654321
         self.role_b = Role.objects.create(name="role_b", tenant=another_tenant)
         self.access_b = Access.objects.create(permission=permission2, role=self.role_b, tenant=another_tenant)
-        self.system_role = Role.objects.create(name="system_role", system=True, tenant=public_tenant)
-        Access.objects.bulk_create(
-            [
-                Access(permission=permission1, role=self.system_role, tenant=public_tenant),
-                Access(permission=permission2, role=self.system_role, tenant=public_tenant),
-            ]
-        )
 
+        self.policy_a2.roles.add(self.system_role_1)
+        self.policy_a2.save()
+
+        # create custom default group
+        self.custom_default_group = clone_default_group_in_public_schema(default_group, self.tenant)
+
+    @override_settings(REPLICATION_TO_RELATION_ENABLED=True, PRINCIPAL_USER_DOMAIN="redhat", READ_ONLY_API_MODE=True)
     @patch("management.relation_replicator.logging_replicator.logger")
     def test_migration_of_data(self, logger_mock):
         """Test that we get the correct access for a principal."""
@@ -134,6 +157,30 @@ def test_migration_of_data(self, logger_mock):
         ):
             workspace_1, workspace_2 = workspace_2, workspace_1
 
+        binding_mapping_system_role_1 = BindingMapping.objects.filter(
+            role=self.system_role_1,
+            resource_type_name="workspace",
+            resource_type_namespace="rbac",
+            resource_id=self.default_workspace.id,
+        ).get()
+
+        self.assertEqual(
+            binding_mapping_system_role_1.mappings["groups"],
+            [str(self.custom_default_group.uuid), str(self.group_a2.uuid)],
+        )
+
+        role_binding_system_role_1_uuid = binding_mapping_system_role_1.mappings["id"]
+
+        binding_mapping_system_role_2 = BindingMapping.objects.filter(
+            role=self.system_role_2,
+            resource_type_name="workspace",
+            resource_type_namespace="rbac",
+            resource_id=self.default_workspace.id,
+        ).get()
+
+        self.assertEqual(binding_mapping_system_role_2.mappings["groups"][0], str(self.custom_default_group.uuid))
+
+        role_binding_system_role_2_uuid = binding_mapping_system_role_2.mappings["id"]
         tuples = [
             # Org relationships of self.tenant
             # the other org is not included since it is not specified in the orgs parameter
@@ -155,5 +202,13 @@ def test_migration_of_data(self, logger_mock):
             call(f"role:{v2_role_a32}#inventory_hosts_write@principal:*"),
             call(f"workspace:{workspace_2}#parent@workspace:{default_workspace_id}"),
             call(f"workspace:{workspace_2}#binding@role_binding:{rolebinding_a32}"),
+            ## System role 1 assigment to custom group
+            call(f"workspace:{self.default_workspace.id}#binding@role_binding:{role_binding_system_role_1_uuid}"),
+            call(f"role_binding:{role_binding_system_role_1_uuid}#subject@group:{self.group_a2.uuid}"),
+            call(f"role_binding:{role_binding_system_role_1_uuid}#role@role:{self.system_role_1.uuid}"),
+            ## System role 2 assigment to custom default group
+            call(f"workspace:{self.default_workspace.id}#binding@role_binding:{role_binding_system_role_2_uuid}"),
+            call(f"role_binding:{role_binding_system_role_2_uuid}#subject@group:{self.custom_default_group.uuid}"),
+            call(f"role_binding:{role_binding_system_role_2_uuid}#role@role:{self.system_role_2.uuid}"),
         ]
         logger_mock.info.assert_has_calls(tuples, any_order=True)