Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add storage account check to avoid name clash #3863

Merged
merged 7 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ ENHANCEMENTS:
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
* 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
Loading