From c7da5d3bb553fd02d72cd07662e67815f86bfb05 Mon Sep 17 00:00:00 2001 From: Guy Bertental Date: Wed, 7 Dec 2022 11:32:33 +0200 Subject: [PATCH] Fix get shared service by template name to filter by operating service only (#2947) * filter shared service by operating service only --- CHANGELOG.md | 1 + api_app/_version.py | 2 +- api_app/db/migrations/shared_services.py | 6 +++--- api_app/db/repositories/operations.py | 2 +- api_app/db/repositories/resources.py | 6 +++--- api_app/db/repositories/shared_services.py | 8 ++++---- .../test_repositories/test_operation_repository.py | 2 +- 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9758d72b09..bd9db0db88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ ENHANCEMENTS: BUG FIXES: * Private endpoints for AppInsights are now provisioning successfully and consistently ([#2841](https://github.com/microsoft/AzureTRE/pull/2841)) * Enable upgrade step of base workspace ([#2899](https://github.com/microsoft/AzureTRE/pull/2899)) +* Fix get shared service by template name to filter by active service only ([#2947](https://github.com/microsoft/AzureTRE/pull/2947)) COMPONENTS: diff --git a/api_app/_version.py b/api_app/_version.py index 43c4ab0058..22049ab2c4 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.6.1" +__version__ = "0.6.2" diff --git a/api_app/db/migrations/shared_services.py b/api_app/db/migrations/shared_services.py index 8651ccef2f..d7185b2384 100644 --- a/api_app/db/migrations/shared_services.py +++ b/api_app/db/migrations/shared_services.py @@ -2,7 +2,7 @@ from azure.cosmos import CosmosClient from db.repositories.shared_services import SharedServiceRepository -from db.repositories.resources import IS_OPERATING_SHARED_SERVICE +from db.repositories.resources import IS_ACTIVE_RESOURCE import semantic_version @@ -16,7 +16,7 @@ def deleteDuplicatedSharedServices(self) -> bool: migrated = False for template_name in template_names: for item in self.query(query=f'SELECT * FROM c WHERE c.resourceType = "shared-service" \ - AND c.templateName = "{template_name}" AND {IS_OPERATING_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')): @@ -31,7 +31,7 @@ def checkMinFirewallVersion(self) -> bool: min_template_version = semantic_version.Version('0.4.0') resources = self.query(query=f'SELECT * FROM c WHERE c.resourceType = "shared-service" \ - AND c.templateName = "{template_name}" AND {IS_OPERATING_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") diff --git a/api_app/db/repositories/operations.py b/api_app/db/repositories/operations.py index 2c09572f7e..d24ba4fc9e 100644 --- a/api_app/db/repositories/operations.py +++ b/api_app/db/repositories/operations.py @@ -96,7 +96,7 @@ def build_step_list(self, steps: List[OperationStep], resource_template_dict: di # if it's a shared service, should be a singleton across the TRE, get it by template name if step["resourceType"] == ResourceType.SharedService: - resource_for_step = resource_repo.get_resource_by_template_name(step["resourceTemplateName"]) + resource_for_step = resource_repo.get_active_resource_by_template_name(step["resourceTemplateName"]) # if it's a workspace, find the parent workspace of where we are if step["resourceType"] == ResourceType.Workspace: diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index eb1c74e53f..8d7781df0e 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -71,8 +71,8 @@ def get_resource_by_id(self, resource_id: UUID4) -> Resource: return parse_obj_as(Resource, resource) - def get_resource_by_template_name(self, template_name: str) -> Resource: - query = f"SELECT TOP 1 * FROM c WHERE c.templateName = '{template_name}'" + def get_active_resource_by_template_name(self, template_name: str) -> Resource: + query = f"SELECT TOP 1 * FROM c WHERE c.templateName = '{template_name}' AND {IS_ACTIVE_RESOURCE}" resources = self.query(query=query) if not resources: raise EntityDoesNotExist @@ -172,4 +172,4 @@ def get_timestamp(self) -> float: # Cosmos query consts IS_NOT_DELETED_CLAUSE = f'c.deploymentStatus != "{Status.Deleted}"' -IS_OPERATING_SHARED_SERVICE = f'c.deploymentStatus != "{Status.Deleted}" and c.deploymentStatus != "{Status.DeploymentFailed}"' +IS_ACTIVE_RESOURCE = f'c.deploymentStatus != "{Status.Deleted}" and c.deploymentStatus != "{Status.DeploymentFailed}"' diff --git a/api_app/db/repositories/shared_services.py b/api_app/db/repositories/shared_services.py index c6f3e639f6..bed8d0e52b 100644 --- a/api_app/db/repositories/shared_services.py +++ b/api_app/db/repositories/shared_services.py @@ -7,7 +7,7 @@ from models.domain.resource_template import ResourceTemplate from models.domain.authentication import User from db.repositories.resource_templates import ResourceTemplateRepository -from db.repositories.resources import ResourceRepository, IS_NOT_DELETED_CLAUSE, IS_OPERATING_SHARED_SERVICE +from db.repositories.resources import ResourceRepository, IS_NOT_DELETED_CLAUSE, IS_ACTIVE_RESOURCE from db.repositories.operations import OperationRepository from db.errors import DuplicateEntity, ResourceIsNotDeployed, EntityDoesNotExist from models.domain.shared_service import SharedService @@ -29,8 +29,8 @@ def active_shared_services_query(): return f'SELECT * FROM c WHERE {IS_NOT_DELETED_CLAUSE} AND c.resourceType = "{ResourceType.SharedService}"' @staticmethod - def operating_shared_service_with_template_name_query(template_name: str): - return f'SELECT * FROM c WHERE {IS_OPERATING_SHARED_SERVICE} AND c.resourceType = "{ResourceType.SharedService}" AND c.templateName = "{template_name}"' + def active_shared_service_with_template_name_query(template_name: str): + return f'SELECT * FROM c WHERE {IS_ACTIVE_RESOURCE} AND c.resourceType = "{ResourceType.SharedService}" AND c.templateName = "{template_name}"' def get_shared_service_by_id(self, shared_service_id: str): shared_services = self.query(self.shared_service_query(shared_service_id)) @@ -60,7 +60,7 @@ def get_shared_service_spec_params(self): def create_shared_service_item(self, shared_service_input: SharedServiceTemplateInCreate, user_roles: List[str]) -> Tuple[SharedService, ResourceTemplate]: shared_service_id = str(uuid.uuid4()) - existing_shared_services = self.query(self.operating_shared_service_with_template_name_query(shared_service_input.templateName)) + existing_shared_services = self.query(self.active_shared_service_with_template_name_query(shared_service_input.templateName)) # Duplicate is same template (=id), same version and deployed if existing_shared_services: diff --git a/api_app/tests_ma/test_db/test_repositories/test_operation_repository.py b/api_app/tests_ma/test_db/test_repositories/test_operation_repository.py index a05d2c6787..cec1da87ce 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_operation_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_operation_repository.py @@ -34,7 +34,7 @@ def test_create_operation_steps_from_multi_step_template(_, __, resource_repo, t expected_op.message = "This resource is waiting to be deployed" operations_repo.save_item = MagicMock() - resource_repo.get_resource_by_template_name = MagicMock(return_value=basic_shared_service) + resource_repo.get_active_resource_by_template_name = MagicMock(return_value=basic_shared_service) operation = operations_repo.create_operation_item( resource_id="resource-id", action="install",