diff --git a/rbac/internal/views.py b/rbac/internal/views.py index 6ef163d8e..030aece66 100644 --- a/rbac/internal/views.py +++ b/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) diff --git a/rbac/management/management/commands/migrate_relations.py b/rbac/management/management/commands/migrate_relations.py index d3728ee0b..926f835c9 100644 --- a/rbac/management/management/commands/migrate_relations.py +++ b/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") diff --git a/rbac/migration_tool/migrate.py b/rbac/migration_tool/migrate.py index 15f7929a1..aa69b6826 100644 --- a/rbac/migration_tool/migrate.py +++ b/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 diff --git a/tests/internal/test_views.py b/tests/internal/test_views.py index 723ff4d2d..fb544c204 100644 --- a/tests/internal/test_views.py +++ b/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) diff --git a/tests/migration_tool/tests_migrate.py b/tests/migration_tool/tests_migrate.py index e527137fd..71f119c33 100644 --- a/tests/migration_tool/tests_migrate.py +++ b/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()