Skip to content

Commit

Permalink
Fix get shared service by template name to filter by operating servic…
Browse files Browse the repository at this point in the history
…e only (#2947)

* filter shared service by operating service only
  • Loading branch information
guybartal committed Dec 7, 2022
1 parent a06ac27 commit c7da5d3
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
2 changes: 1 addition & 1 deletion api_app/_version.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.6.1"
__version__ = "0.6.2"
6 changes: 3 additions & 3 deletions api_app/db/migrations/shared_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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')):
Expand All @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion api_app/db/repositories/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions api_app/db/repositories/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}"'
8 changes: 4 additions & 4 deletions api_app/db/repositories/shared_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit c7da5d3

Please sign in to comment.