From 348be1878bf18e857bbe227842dac1fe63531d32 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Thu, 5 Dec 2024 15:22:53 +0000 Subject: [PATCH] Remove old API migrations Fixes #4079 Remove deprecated migration steps from the `migrate_database` function in `api_app/api/routes/migrations.py`. * **Migration Steps Removal** - Remove multiple deprecated migration steps from the `migrate_database` function. - Update the `migrate_database` function to only include necessary migration steps. * **Version Update** - Update the version in `api_app/_version.py` from "0.19.3" to "0.20.0". * **Test Updates** - Remove tests related to deprecated migration steps in `api_app/tests_ma/test_api/test_routes/test_migrations.py`. - Update tests to only include necessary migration steps. * **File Deletions** - Delete `api_app/db/migrations/airlock.py`. - Delete `api_app/db/migrations/resources.py`. - Delete `api_app/db/migrations/shared_services.py`. - Delete `api_app/db/migrations/workspaces.py`. - Delete `api_app/tests_ma/test_db/test_migrations/test_workspace_migration.py`. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/microsoft/AzureTRE/issues/4079?shareId=XXXX-XXXX-XXXX-XXXX). --- api_app/_version.py | 2 +- api_app/api/routes/migrations.py | 53 +------------ api_app/db/migrations/airlock.py | 77 ------------------- api_app/db/migrations/resources.py | 54 ------------- api_app/db/migrations/shared_services.py | 48 ------------ api_app/db/migrations/workspaces.py | 48 ------------ .../test_api/test_routes/test_migrations.py | 33 +------- .../test_workspace_migration.py | 47 ----------- 8 files changed, 5 insertions(+), 357 deletions(-) delete mode 100644 api_app/db/migrations/airlock.py delete mode 100644 api_app/db/migrations/resources.py delete mode 100644 api_app/db/migrations/shared_services.py delete mode 100644 api_app/db/migrations/workspaces.py delete mode 100644 api_app/tests_ma/test_db/test_migrations/test_workspace_migration.py diff --git a/api_app/_version.py b/api_app/_version.py index 1a95d56216..5f4bb0b345 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.19.3" +__version__ = "0.20.0" diff --git a/api_app/api/routes/migrations.py b/api_app/api/routes/migrations.py index 692f664583..ca34d7877b 100644 --- a/api_app/api/routes/migrations.py +++ b/api_app/api/routes/migrations.py @@ -29,57 +29,8 @@ async def migrate_database(resources_repo=Depends(get_repository(ResourceReposit airlock_migration=Depends(get_repository(AirlockMigration)),): try: migrations = list() - logger.info("PR 1030") - await resources_repo.rename_field_name('resourceTemplateName', 'templateName') - await resources_repo.rename_field_name('resourceTemplateVersion', 'templateVersion') - await resources_repo.rename_field_name('resourceTemplateParameters', 'properties') - migrations.append(Migration(issueNumber="PR 1030", status="Executed")) - - logger.info("PR 1031") - await resources_repo.rename_field_name('workspaceType', 'templateName') - await resources_repo.rename_field_name('workspaceServiceType', 'templateName') - await resources_repo.rename_field_name('userResourceType', 'templateName') - migrations.append(Migration(issueNumber="PR 1031", status="Executed")) - - logger.info("PR 1717 - Shared services") - migration_status = "Executed" if await shared_services_migration.deleteDuplicatedSharedServices() else "Skipped" - migrations.append(Migration(issueNumber="PR 1717", status=migration_status)) - - logger.info("PR 1726 - Authentication needs to be in properties so we can update them") - migration_status = "Executed" if await workspace_migration.moveAuthInformationToProperties() else "Skipped" - migrations.append(Migration(issueNumber="PR 1726", status=migration_status)) - - logger.info("PR 1406 - Extra field to support UI") - num_rows = await resource_migration.add_deployment_status_field(operations_repo) - migrations.append(Migration(issueNumber="1406", status=f'Updated {num_rows} resource objects')) - - logger.info("PR 3066 - Archive resources history") - num_rows = await resource_migration.archive_history(resource_history_repo) - migrations.append(Migration(issueNumber="3066", status=f'Updated {num_rows} resource objects')) - - logger.info("PR 2371 - Validate min firewall version") - await shared_services_migration.checkMinFirewallVersion() - migrations.append(Migration(issueNumber="2371", status='Firewall version meets requirement')) - - logger.info("PR 2779 - Restructure Airlock requests & add createdBy field") - await airlock_migration.rename_field_name('requestType', 'type') - await airlock_migration.rename_field_name('requestTitle', 'title') - await airlock_migration.rename_field_name('user', 'updatedBy') - await airlock_migration.rename_field_name('creationTime', 'createdWhen') - num_updated = await airlock_migration.add_created_by_and_rename_in_history() - migrations.append(Migration(issueNumber="2779", status=f'Renamed fields & updated {num_updated} airlock requests with createdBy')) - - logger.info("PR 2883 - Support multiple reviewer VMs per Airlock request") - num_updated = await airlock_migration.change_review_resources_to_dict() - migrations.append(Migration(issueNumber="2883", status=f'Updated {num_updated} airlock requests with new reviewUserResources format')) - - logger.info("PR 3152 - Migrate reviewDecision of Airlock Reviews") - num_updated = await airlock_migration.update_review_decision_values() - migrations.append(Migration(issueNumber="3152", status=f'Updated {num_updated} airlock requests with new reviewDecision value')) - - logger.info("PR 3358 - Migrate OperationSteps of Operations") - num_updated = await resource_migration.migrate_step_id_of_operation_steps(operations_repo) - migrations.append(Migration(issueNumber="3358", status=f'Updated {num_updated} operations')) + + # ADD MIGRATIONS HERE return MigrationOutList(migrations=migrations) except Exception as e: diff --git a/api_app/db/migrations/airlock.py b/api_app/db/migrations/airlock.py deleted file mode 100644 index aa0ce011cd..0000000000 --- a/api_app/db/migrations/airlock.py +++ /dev/null @@ -1,77 +0,0 @@ -from resources import strings -from db.repositories.airlock_requests import AirlockRequestRepository - - -class AirlockMigration(AirlockRequestRepository): - @classmethod - async def create(cls): - cls = AirlockMigration() - resource_repo = await super().create() - cls._container = resource_repo._container - return cls - - async def add_created_by_and_rename_in_history(self) -> int: - num_updated = 0 - for request in await self.query('SELECT * FROM c'): - # Only migrate if createdBy isn't present - if 'createdBy' in request: - continue - - # For each request, check if it has history - if len(request['history']) > 0: - # createdBy value will be first user in history - request['createdBy'] = request['history'][0]['user'] - - # Also rename user to updatedBy in each history item - for item in request['history']: - if 'user' in item: - item['updatedBy'] = item['user'] - del item['user'] - else: - # If not, the createdBy user will be the same as the updatedBy value - request['createdBy'] = request['updatedBy'] - - await self.update_item_dict(request) - num_updated += 1 - - return num_updated - - async def change_review_resources_to_dict(self) -> int: - num_updated = 0 - for request in await self.query('SELECT * FROM c'): - # Only migrate if airlockReviewResources property present and is a list - if 'reviewUserResources' in request and isinstance(request['reviewUserResources'], list): - updated_review_resources = {} - for i, resource in enumerate(request['reviewUserResources']): - updated_review_resources['UNKNOWN' + str(i)] = resource - - request['reviewUserResources'] = updated_review_resources - await self.update_item_dict(request) - num_updated += 1 - - return num_updated - - async def update_review_decision_values(self) -> int: - num_updated = 0 - for request in await self.query('SELECT * FROM c WHERE ARRAY_LENGTH(c.reviews) > 0'): - request_changed = False - - for review in request['reviews']: - old_decision = review['reviewDecision'] - new_decision = old_decision - - if old_decision == 'approval_in_progress': - new_decision = strings.AIRLOCK_REVIEW_DECISION_APPROVED - - if old_decision == 'rejection_in_progress': - new_decision = strings.AIRLOCK_REVIEW_DECISION_REJECTED - - if new_decision != old_decision: - request_changed = True - review['reviewDecision'] = new_decision - - if request_changed: - await self.update_item_dict(request) - num_updated += 1 - - return num_updated diff --git a/api_app/db/migrations/resources.py b/api_app/db/migrations/resources.py deleted file mode 100644 index 4c99bff4cf..0000000000 --- a/api_app/db/migrations/resources.py +++ /dev/null @@ -1,54 +0,0 @@ -import uuid -from db.repositories.operations import OperationRepository -from db.repositories.resources import ResourceRepository -from db.repositories.resources_history import ResourceHistoryRepository - - -class ResourceMigration(ResourceRepository): - @classmethod - async def create(cls): - cls = ResourceMigration() - resource_repo = await super().create() - cls._container = resource_repo._container - return cls - - async def add_deployment_status_field(self, operations_repository: OperationRepository) -> int: - num_updated = 0 - for resource in await self.query("SELECT * from c WHERE NOT IS_DEFINED(c.deploymentStatus)"): - # For each resource, find the last operation, if it exists - id = resource['id'] - op = await operations_repository.query(f'SELECT * from c WHERE c.resourceId = "{id}" ORDER BY c._ts DESC OFFSET 0 LIMIT 1') - if op: - # Set the deploymentStatus of the resource to be the status fo its last operation - resource['deploymentStatus'] = op[0]['status'] - await self.update_item_dict(resource) - num_updated = num_updated + 1 - - return num_updated - - async def archive_history(self, resource_history_repository: ResourceHistoryRepository) -> int: - num_updated = 0 - for resource in await self.query("SELECT * from c WHERE IS_DEFINED(c.history)"): - for history_item in resource['history']: - history_item['id'] = str(uuid.uuid4()) - history_item['resourceId'] = resource['id'] - await resource_history_repository.update_item_dict(history_item) - # Remove the history item from the resource - del resource['history'] - await self.update_item_dict(resource) - num_updated = num_updated + 1 - - return num_updated - - async def migrate_step_id_of_operation_steps(self, operations_repository: OperationRepository) -> int: - num_updated = 0 - for operation in await operations_repository.query("SELECT * from c WHERE ARRAY_LENGTH(c.steps) > 0 AND IS_DEFINED(c.steps[0].stepId)"): - for operation_step in operation['steps']: - operation_step['templateStepId'] = operation_step['stepId'] - operation_step['id'] = str(uuid.uuid4()) - del operation_step['stepId'] - - await operations_repository.update_item_dict(operation) - num_updated = num_updated + 1 - - return num_updated diff --git a/api_app/db/migrations/shared_services.py b/api_app/db/migrations/shared_services.py deleted file mode 100644 index 621991314a..0000000000 --- a/api_app/db/migrations/shared_services.py +++ /dev/null @@ -1,48 +0,0 @@ -import semantic_version - -from db.repositories.shared_services import SharedServiceRepository -from db.repositories.resources import IS_ACTIVE_RESOURCE -from services.logging import logger - - -class SharedServiceMigration(SharedServiceRepository): - @classmethod - async def create(cls): - cls = SharedServiceMigration() - resource_repo = await super().create() - cls._container = resource_repo._container - return cls - - async def deleteDuplicatedSharedServices(self) -> bool: - template_names = ['tre-shared-service-firewall', 'tre-shared-service-sonatype-nexus', 'tre-shared-service-gitea'] - - migrated = False - for template_name in template_names: - for item in await self.query(query=f'SELECT * FROM c WHERE c.resourceType = "shared-service" \ - AND c.templateName = "{template_name}" AND {IS_ACTIVE_RESOURCE} \ - ORDER BY c.updatedWhen ASC OFFSET 1 LIMIT 10000'): - template_version = semantic_version.Version(item["templateVersion"]) - if (template_version < semantic_version.Version('0.3.0')): - logger.info(f'Deleting element {item["id"]}') - await self.delete_item(item["id"]) - migrated = True - - return migrated - - async def checkMinFirewallVersion(self) -> bool: - template_name = 'tre-shared-service-firewall' - min_template_version = semantic_version.Version('0.4.0') - - resources = await self.query(query=f'SELECT * FROM c WHERE c.resourceType = "shared-service" \ - AND c.templateName = "{template_name}" AND {IS_ACTIVE_RESOURCE}') - - if not resources: - raise ValueError(f"Expecting to have an instance of Firewall (template name {template_name}) deployed in a successful TRE deployment") - - template_version = semantic_version.Version(resources[0]["templateVersion"]) - - if (template_version < min_template_version): - raise ValueError(f"{template_name} deployed version ({template_version}) is below minimum ({min_template_version})!", - "Go to https://github.com/microsoft/AzureTRE/blob/main/CHANGELOG.md, and review release 0.5.0 for more info.") - - return True diff --git a/api_app/db/migrations/workspaces.py b/api_app/db/migrations/workspaces.py deleted file mode 100644 index e8a422080b..0000000000 --- a/api_app/db/migrations/workspaces.py +++ /dev/null @@ -1,48 +0,0 @@ -import semantic_version - -from db.repositories.workspaces import WorkspaceRepository -from services.logging import logger - - -class WorkspaceMigration(WorkspaceRepository): - @classmethod - async def create(cls): - cls = WorkspaceMigration() - resource_repo = await super().create() - cls._container = resource_repo._container - return cls - - async def moveAuthInformationToProperties(self) -> bool: - migrated = False - for item in await self.query(query=WorkspaceRepository.workspaces_query_string()): - template_version = semantic_version.Version(item["templateVersion"]) - updated = False - if (template_version < semantic_version.Version('0.2.7')): - - # Rename app_id to be client_id - if "app_id" in item["properties"]: - item["properties"]["client_id"] = item["properties"]["app_id"] - del item["properties"]["app_id"] - updated = True - - if "scope_id" not in item["properties"]: - item["properties"]["scope_id"] = f"api://{item['properties']['client_id']}" - updated = True - - if "authInformation" in item: - logger.info(f'Upgrading authInformation in workspace {item["id"]}') - - # Copy authInformation into properties - item["properties"]["sp_id"] = item["authInformation"]["sp_id"] - item["properties"]["app_role_id_workspace_researcher"] = item["authInformation"]["roles"]["WorkspaceResearcher"] - item["properties"]["app_role_id_workspace_owner"] = item["authInformation"]["roles"]["WorkspaceOwner"] - # cleanup - del item["authInformation"] - updated = True - - if updated: - await self.update_item_dict(item) - logger.info(f'Upgraded authentication info for workspace id {item["id"]}') - migrated = True - - return migrated diff --git a/api_app/tests_ma/test_api/test_routes/test_migrations.py b/api_app/tests_ma/test_api/test_routes/test_migrations.py index d49239d222..240ddb27a7 100644 --- a/api_app/tests_ma/test_api/test_routes/test_migrations.py +++ b/api_app/tests_ma/test_api/test_routes/test_migrations.py @@ -35,44 +35,15 @@ def _prepare(self, app, admin_user): # [POST] /migrations/ @ patch("api.routes.migrations.logger.info") @ patch("api.routes.migrations.OperationRepository") - @ patch("api.routes.migrations.ResourceMigration.archive_history") - @ patch("api.routes.migrations.ResourceMigration.add_deployment_status_field") - @ patch("api.routes.migrations.ResourceRepository.rename_field_name") - @ patch("api.routes.migrations.SharedServiceMigration.deleteDuplicatedSharedServices") - @ patch("api.routes.migrations.WorkspaceMigration.moveAuthInformationToProperties") - @ patch("api.routes.migrations.SharedServiceMigration.checkMinFirewallVersion") - @ patch("api.routes.migrations.AirlockMigration.add_created_by_and_rename_in_history") - @ patch("api.routes.migrations.AirlockMigration.rename_field_name") - @ patch("api.routes.migrations.AirlockMigration.change_review_resources_to_dict") - @ patch("api.routes.migrations.AirlockMigration.update_review_decision_values") - @ patch("api.routes.migrations.ResourceMigration.migrate_step_id_of_operation_steps") - async def test_post_migrations_returns_202_on_successful(self, migrate_step_id_of_operation_steps, update_review_decision_values, - change_review_resources_to_dict, airlock_rename_field, add_created_by_and_rename_in_history, - check_min_firewall_version, workspace_migration, shared_services_migration, - rename_field, add_deployment_field, archive_history, _, logging, client, app): + async def test_post_migrations_returns_202_on_successful(self, _, logging, client, app): response = await client.post(app.url_path_for(strings.API_MIGRATE_DATABASE)) - check_min_firewall_version.assert_called_once() - shared_services_migration.assert_called_once() - workspace_migration.assert_called_once() - rename_field.assert_called() - add_deployment_field.assert_called() - add_created_by_and_rename_in_history.assert_called_once() - airlock_rename_field.assert_called() - change_review_resources_to_dict.assert_called_once() - update_review_decision_values.assert_called_once() - migrate_step_id_of_operation_steps.assert_called_once() - archive_history.assert_called_once() logging.assert_called() assert response.status_code == status.HTTP_202_ACCEPTED # [POST] /migrations/ @ patch("api.routes.migrations.logger.info") @ patch("api.routes.migrations.ResourceRepository.rename_field_name", side_effect=ValueError) - @ patch("api.routes.migrations.SharedServiceMigration.deleteDuplicatedSharedServices") - @ patch("api.routes.migrations.WorkspaceMigration.moveAuthInformationToProperties") - @ patch("api.routes.migrations.AirlockMigration.add_created_by_and_rename_in_history") - async def test_post_migrations_returns_400_if_template_does_not_exist(self, workspace_migration, shared_services_migration, - resources_repo, airlock_migration, logging, client, app): + async def test_post_migrations_returns_400_if_template_does_not_exist(self, resources_repo, logging, client, app): response = await client.post(app.url_path_for(strings.API_MIGRATE_DATABASE)) assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/api_app/tests_ma/test_db/test_migrations/test_workspace_migration.py b/api_app/tests_ma/test_db/test_migrations/test_workspace_migration.py deleted file mode 100644 index 3da577094b..0000000000 --- a/api_app/tests_ma/test_db/test_migrations/test_workspace_migration.py +++ /dev/null @@ -1,47 +0,0 @@ -from unittest.mock import AsyncMock -import pytest -import pytest_asyncio -from mock import patch - -from models.domain.resource import ResourceType -from db.migrations.workspaces import WorkspaceMigration - -pytestmark = pytest.mark.asyncio - - -@pytest_asyncio.fixture -async def workspace_migrator(): - with patch('api.dependencies.database.Database.get_container_proxy', return_value=AsyncMock()): - workspace_migrator = await WorkspaceMigration.create() - yield workspace_migrator - - -def get_sample_old_workspace(workspace_id: str = "7ab18f7e-ee8f-4202-8d46-747818ec76f4", spec_workspace_id: str = "0001") -> dict: - return [{ - "id": workspace_id, - "templateName": "tre-workspace-base", - "templateVersion": "0.1.0", - "properties": { - "app_id": "03f18f7e-ee8f-4202-8d46-747818ec76f4", - "azure_location": "westeurope", - "workspace_id": spec_workspace_id, - "tre_id": "mytre-dev-1234", - "address_space_size": "small", - }, - "resourceType": ResourceType.Workspace, - "workspaceURL": "", - "authInformation": { - "sp_id": "f153f0f4-e89a-4456-b7ba-d0c46571d7c8", - "roles": { - "WorkspaceResearcher": "100358cf-5c65-4dfb-88b8-ed87fdc59db0", - "WorkspaceOwner": "682df69e-bf3c-4606-85ab-75d70c0d510f" - }, - "app_id": "03f18f7e-ee8f-4202-8d46-747818ec76f4" - }, - }] - - -async def test_workspace_migration_moves_fields(workspace_migrator): - workspace_migrator.query = AsyncMock(return_value=get_sample_old_workspace()) - - assert (await workspace_migrator.moveAuthInformationToProperties())