Skip to content

Commit

Permalink
Add storage account check to avoid name clash (#3863)
Browse files Browse the repository at this point in the history
* Add storage account check

* update changelog. version and fix linting

* Update _version.py

---------

Co-authored-by: Tim Allen <tim.allen@cloudkubed.com>
  • Loading branch information
marrobi and tim-allen-ck authored May 10, 2024
1 parent bd0aa62 commit d866ec5
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ BUG FIXES:
* Update to Resource Processor Image, now using Ubuntu 22.04 (jammy). Part of ([#3523](https://github.com/microsoft/AzureTRE/issues/3523))
* Remove TLS1.0/1.1 support from Application Gateway ([#3914](https://github.com/microsoft/AzureTRE/issues/3914))
* GitHub Actions version updates. ([#3847](https://github.com/microsoft/AzureTRE/issues/3847))
* Add workaround to avoid name clashes for storage accounts([#3863](https://github.com/microsoft/AzureTRE/pull/3858))

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.18.6"
__version__ = "0.18.7"
19 changes: 12 additions & 7 deletions api_app/db/repositories/workspaces.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import uuid
from typing import List, Tuple

from azure.mgmt.storage import StorageManagementClient
from pydantic import parse_obj_as
from db.repositories.resources_history import ResourceHistoryRepository
from models.domain.resource_template import ResourceTemplate
from models.domain.authentication import User

from core import config
from core import config, credentials
from db.errors import EntityDoesNotExist, InvalidInput, ResourceIsNotDeployed
from db.repositories.resource_templates import ResourceTemplateRepository
from db.repositories.resources import ResourceRepository, IS_NOT_DELETED_CLAUSE
Expand Down Expand Up @@ -66,17 +66,22 @@ async def get_workspace_by_id(self, workspace_id: str) -> Workspace:
return parse_obj_as(Workspace, workspaces[0])

# Remove this method once not using last 4 digits for naming - https://github.com/microsoft/AzureTRE/issues/3666
async def is_workspace_with_last_4_id(self, workspace_id: str) -> bool:
query = self.workspaces_query_string() + f' AND ENDSWITH(c.id, "{workspace_id[-4:]}")'
workspaces = await self.query(query=query)
return len(workspaces) > 0
async def is_worksapce_storage_account_available(self, workspace_id: str) -> bool:
storage_client = StorageManagementClient(credentials.get_credential(), config.SUBSCRIPTION_ID)
# check for storage account with last 4 digits of workspace_id
availability_result = storage_client.storage_accounts.check_name_availability(
{
"name": f"stgws{workspace_id[-4:]}"
}
)
return availability_result.name_available

async def create_workspace_item(self, workspace_input: WorkspaceInCreate, auth_info: dict, workspace_owner_object_id: str, user_roles: List[str]) -> Tuple[Workspace, ResourceTemplate]:

full_workspace_id = str(uuid.uuid4())

# Ensure workspace with last four digits of ID does not already exist - remove when https://github.com/microsoft/AzureTRE/issues/3666 is resolved
while await self.is_workspace_with_last_4_id(full_workspace_id):
while not await self.is_worksapce_storage_account_available(full_workspace_id):
full_workspace_id = str(uuid.uuid4())

template = await self.validate_input_against_template(workspace_input.templateName, workspace_input, ResourceType.Workspace, user_roles)
Expand Down
1 change: 1 addition & 0 deletions api_app/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ azure-mgmt-compute==30.3.0
azure-mgmt-cosmosdb==9.3.0
azure-mgmt-costmanagement==4.0.1
azure-mgmt-resource==23.0.1
azure-mgmt-storage==21.1.0
azure-monitor-opentelemetry==1.2.0
azure-servicebus==7.11.3
azure-storage-blob==12.19.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,18 @@ async def test_get_workspace_by_id_queries_db(workspace_repo, workspace):
@pytest.mark.asyncio
@patch('db.repositories.workspaces.generate_new_cidr')
@patch('db.repositories.workspaces.WorkspaceRepository.validate_input_against_template')
@patch('db.repositories.workspaces.WorkspaceRepository.is_worksapce_storage_account_available')
@patch('core.config.RESOURCE_LOCATION', "useast2")
@patch('core.config.TRE_ID', "9876")
async def test_create_workspace_item_creates_a_workspace_with_the_right_values(validate_input_mock, new_cidr_mock, workspace_repo, basic_workspace_request, basic_resource_template):
async def test_create_workspace_item_creates_a_workspace_with_the_right_values(mock_is_workspace_storage_account_available, validate_input_mock, new_cidr_mock, workspace_repo, basic_workspace_request, basic_resource_template):
workspace_to_create = basic_workspace_request
# make sure the input has 'None' for values that we expect to be set
workspace_to_create.properties.pop("address_space", None)
workspace_to_create.properties.pop("address_spaces", None)
workspace_to_create.properties.pop("workspace_owner_object_id", None)

mock_is_workspace_storage_account_available.return_value = AsyncMock().return_value
mock_is_workspace_storage_account_available.return_value.return_value = False
validate_input_mock.return_value = basic_resource_template
new_cidr_mock.return_value = "1.2.3.4/24"

Expand Down Expand Up @@ -165,14 +168,18 @@ async def test_get_address_space_based_on_size_with_large_address_space(workspac

@pytest.mark.asyncio
@patch('db.repositories.workspaces.WorkspaceRepository.validate_input_against_template')
@patch('db.repositories.workspaces.WorkspaceRepository.is_worksapce_storage_account_available')
@patch('core.config.RESOURCE_LOCATION', "useast2")
@patch('core.config.TRE_ID', "9876")
@patch('core.config.CORE_ADDRESS_SPACE', "10.1.0.0/22")
@patch('core.config.TRE_ADDRESS_SPACE', "10.0.0.0/12")
async def test_create_workspace_item_creates_a_workspace_with_custom_address_space(validate_input_mock, workspace_repo, basic_workspace_request, basic_resource_template):
async def test_create_workspace_item_creates_a_workspace_with_custom_address_space(mock_is_workspace_storage_account_available, validate_input_mock, workspace_repo, basic_workspace_request, basic_resource_template):
workspace_to_create = basic_workspace_request
workspace_to_create.properties["address_space_size"] = "custom"
workspace_to_create.properties["address_space"] = "10.2.4.0/24"

mock_is_workspace_storage_account_available.return_value = AsyncMock().return_value
mock_is_workspace_storage_account_available.return_value.return_value = False
validate_input_mock.return_value = basic_resource_template

workspace, _ = await workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id", ["test_role"])
Expand All @@ -182,14 +189,18 @@ async def test_create_workspace_item_creates_a_workspace_with_custom_address_spa

@pytest.mark.asyncio
@patch('db.repositories.workspaces.WorkspaceRepository.validate_input_against_template')
@patch('db.repositories.workspaces.WorkspaceRepository.is_worksapce_storage_account_available')
@patch('core.config.RESOURCE_LOCATION', "useast2")
@patch('core.config.TRE_ID', "9876")
@patch('core.config.CORE_ADDRESS_SPACE', "10.1.0.0/22")
@patch('core.config.TRE_ADDRESS_SPACE', "10.0.0.0/12")
async def test_create_workspace_item_throws_exception_with_bad_custom_address_space(validate_input_mock, workspace_repo, basic_workspace_request, basic_resource_template):
async def test_create_workspace_item_throws_exception_with_bad_custom_address_space(mock_is_workspace_storage_account_available, validate_input_mock, workspace_repo, basic_workspace_request, basic_resource_template):
workspace_to_create = basic_workspace_request
workspace_to_create.properties["address_space_size"] = "custom"
workspace_to_create.properties["address_space"] = "192.168.0.0/24"

mock_is_workspace_storage_account_available.return_value = AsyncMock().return_value
mock_is_workspace_storage_account_available.return_value.return_value = False
validate_input_mock.return_value = basic_resource_template

with pytest.raises(InvalidInput):
Expand Down Expand Up @@ -249,8 +260,12 @@ async def test_get_address_space_based_on_size_with_address_space_and_address_sp

@pytest.mark.asyncio
@patch('db.repositories.workspaces.WorkspaceRepository.validate_input_against_template')
async def test_create_workspace_item_raises_value_error_if_template_is_invalid(validate_input_mock, workspace_repo, basic_workspace_request):
@patch('db.repositories.workspaces.WorkspaceRepository.is_worksapce_storage_account_available')
async def test_create_workspace_item_raises_value_error_if_template_is_invalid(mock_is_workspace_storage_account_available, validate_input_mock, workspace_repo, basic_workspace_request):
workspace_input = basic_workspace_request

mock_is_workspace_storage_account_available.return_value = AsyncMock().return_value
mock_is_workspace_storage_account_available.return_value.return_value = False
validate_input_mock.side_effect = ValueError

with pytest.raises(ValueError):
Expand Down Expand Up @@ -281,3 +296,29 @@ def test_workspace_owner_is_not_overwritten_if_present_in_workspace_properties(w
not_expected_object_id = "Not Expected"

assert workspace_repo.get_workspace_owner(dictToTest, not_expected_object_id) == "Expected"


@patch('azure.mgmt.storage.StorageManagementClient')
async def test_is_worksapce_storage_account_available_when_name_available(mock_storage_client):
workspace_id = "workspace1234"
mock_storage_client.return_value.storage_accounts.check_name_availability.return_value = AsyncMock()
mock_storage_client.return_value.storage_accounts.check_name_availability.return_value.name_available = True
workspace_repo = WorkspaceRepository()

result = await workspace_repo.is_workspace_with_last_4_id(workspace_id)

mock_storage_client.return_value.storage_accounts.check_name_availability.assert_called_once_with({"name": f"stgws{workspace_id[-4:]}"})
assert result is False


@patch('azure.mgmt.storage.StorageManagementClient')
async def test_is_worksapce_storage_account_available_when_name_not_available(mock_storage_client):
workspace_id = "workspace1234"
mock_storage_client.return_value.storage_accounts.check_name_availability.return_value = AsyncMock()
mock_storage_client.return_value.storage_accounts.check_name_availability.return_value.name_available = False
workspace_repo = WorkspaceRepository()

result = await workspace_repo.is_workspace_with_last_4_id(workspace_id)

mock_storage_client.return_value.storage_accounts.check_name_availability.assert_called_once_with({"name": f"stgws{workspace_id[-4:]}"})
assert result is True

0 comments on commit d866ec5

Please sign in to comment.