Support skipping migrating roles (RedHatInsights#1395)

rbac/internal/views.py

@@ -495,7 +495,12 @@ def get_param_list(request, param_name, default: list = []):
 def data_migration(request):
     """View method for running migrations from V1 to V2 spiceDB schema.
 
-    POST /_private/api/utils/data_migration/?exclude_apps=cost_management,rbac&orgs=id_1,id_2&write_relationships=True
+    POST /_private/api/utils/data_migration/
+    query params:
+        exclude_apps: e.g., cost_management,rbac
+        orgs: e.g., id_1,id_2
+        write_relationships: True, False, outbox
+        skip_roles: True or False
     """
     if request.method != "POST":
         return HttpResponse('Invalid method, only "POST" is allowed.', status=405)
@@ -505,6 +510,7 @@ def data_migration(request):
         "exclude_apps": get_param_list(request, "exclude_apps", default=settings.V2_MIGRATION_APP_EXCLUDE_LIST),
         "orgs": get_param_list(request, "orgs"),
         "write_relationships": request.GET.get("write_relationships", "False"),
+        "skip_roles": request.GET.get("skip_roles", "False").lower() == "true",
     }
     migrate_data_in_worker.delay(args)
     return HttpResponse("Data migration from V1 to V2 are running in a background worker.", status=202)

rbac/management/management/commands/migrate_relations.py

@@ -29,6 +29,12 @@ def add_arguments(self, parser):
             choices=["True", "False", "relations-api", "outbox"],
             help="Whether to replicate relationships and how. True is == 'relations-api' for compatibility.",
         )
+        parser.add_argument(
+            "--skip-roles",
+            default="False",
+            choices=["True", "False"],
+            help="Whether to skip migrate roles.",
+        )
 
     def handle(self, *args, **options):
         """Handle method for command."""
@@ -37,6 +43,7 @@ def handle(self, *args, **options):
             "exclude_apps": options["exclude_apps"],
             "orgs": options["org_list"],
             "write_relationships": options["write_relationships"],
+            "skip_roles": options["skip_roles"] == "True",
         }
         migrate_data(**kwargs)
         logger.info("*** Migration completed. ***\n")

rbac/migration_tool/migrate.py

@@ -119,14 +119,8 @@ def migrate_groups_for_tenant(tenant: Tenant, replicator: RelationReplicator):
                 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.")
-
-    migrate_groups_for_tenant(tenant, replicator)
-
-    logger.info("Finished migrating relations of group and user.")
-
+def migrate_roles_for_tenant(tenant, exclude_apps, replicator):
+    """Migrate all roles for a given tenant."""
     default_workspace = Workspace.objects.get(type=Workspace.Types.DEFAULT, tenant=tenant)
 
     roles = tenant.role_set.all()
@@ -158,6 +152,20 @@ def migrate_data_for_tenant(tenant: Tenant, exclude_apps: list, replicator: Rela
         logger.info(f"Migration completed for role: {role.name} with UUID {role.uuid}.")
     logger.info(f"Migrated {roles.count()} roles for tenant: {tenant.org_id}")
 
+
+def migrate_data_for_tenant(tenant: Tenant, exclude_apps: list, replicator: RelationReplicator, skip_roles: bool):
+    """Migrate all data for a given tenant."""
+    logger.info("Migrating relations of group and user.")
+
+    migrate_groups_for_tenant(tenant, replicator)
+
+    logger.info("Finished migrating relations of group and user.")
+
+    if skip_roles:
+        logger.info("Skipping migrating roles.")
+    else:
+        migrate_roles_for_tenant(tenant, exclude_apps, replicator)
+
     logger.info("Migrating relations of cross account requests.")
     migrate_cross_account_requests(tenant, replicator)
     logger.info("Finished relations of cross account requests.")
@@ -179,7 +187,9 @@ def migrate_cross_account_requests(tenant: Tenant, replicator: RelationReplicato
                 dual_write_handler.replicate()
 
 
-def migrate_data(exclude_apps: list = [], orgs: list = [], write_relationships: str = "False"):
+def migrate_data(
+    exclude_apps: list = [], orgs: list = [], write_relationships: str = "False", skip_roles: bool = False
+):
     """Migrate all data for all tenants."""
     # Only run this in maintanence mode or
     # if we don't write relationships (testing out the migration and clean up the created bindingmappings)
@@ -201,7 +211,7 @@ def migrate_data(exclude_apps: list = [], orgs: list = [], write_relationships:
             logger.info(f"Migrating data for tenant: {tenant.org_id}")
 
         try:
-            migrate_data_for_tenant(tenant, exclude_apps, replicator)
+            migrate_data_for_tenant(tenant, exclude_apps, replicator, skip_roles)
         except Exception as e:
             logger.error(f"Failed to migrate data for tenant: {tenant.org_id}. Error: {e}")
             raise e

tests/internal/test_views.py

@@ -499,6 +499,7 @@ def test_run_migrations_of_data(self, migration_mock):
                 "exclude_apps": ["rbac", "costmanagement"],
                 "orgs": ["acct00001", "acct00002"],
                 "write_relationships": "False",
+                "skip_roles": False,
             }
         )
         self.assertEqual(
@@ -514,7 +515,7 @@ def test_run_migrations_of_data(self, migration_mock):
                 **self.request.META,
             )
             migration_mock.assert_called_once_with(
-                {"exclude_apps": ["fooapp"], "orgs": [], "write_relationships": "False"}
+                {"exclude_apps": ["fooapp"], "orgs": [], "write_relationships": "False", "skip_roles": False}
             )
             self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED)
             self.assertEqual(
@@ -529,7 +530,9 @@ def test_run_migrations_of_data(self, migration_mock):
                 f"/_private/api/utils/data_migration/",
                 **self.request.META,
             )
-            migration_mock.assert_called_once_with({"exclude_apps": [], "orgs": [], "write_relationships": "False"})
+            migration_mock.assert_called_once_with(
+                {"exclude_apps": [], "orgs": [], "write_relationships": "False", "skip_roles": False}
+            )
             self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED)
             self.assertEqual(
                 response.content.decode(),
@@ -549,6 +552,7 @@ def test_run_migrations_of_data_outbox_replication(self, migration_mock):
                 "exclude_apps": ["rbac", "costmanagement"],
                 "orgs": ["acct00001", "acct00002"],
                 "write_relationships": "outbox",
+                "skip_roles": False,
             }
         )
         self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED)

tests/migration_tool/tests_migrate.py

@@ -281,3 +281,16 @@ def test_skips_orgs_without_org_ids(self):
             migrate_data(**kwargs)
         except Exception:
             self.fail("migrate_data raised an exception when migrating tenant without org_id")
+
+    @override_settings(REPLICATION_TO_RELATION_ENABLED=True, PRINCIPAL_USER_DOMAIN="redhat", READ_ONLY_API_MODE=True)
+    @patch("migration_tool.migrate.migrate_groups_for_tenant")
+    @patch("migration_tool.migrate.migrate_roles_for_tenant")
+    @patch("migration_tool.migrate.migrate_cross_account_requests")
+    def test_skips_roles_migration(self, group_migrator, role_migrator, car_migrator):
+        kwargs = {"orgs": ["1234567"], "skip_roles": True}
+
+        migrate_data(**kwargs)
+
+        group_migrator.assert_called_once()
+        role_migrator.assert_not_called()
+        car_migrator.assert_called_once()