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

Update setting of read only fields and include deleted address spaces when assigning new address spaces #3714

Merged
merged 18 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ FEATURES:
ENHANCEMENTS:

BUG FIXES:
* Fix issue where updates fail as read only is not configured consistently on schema fields ([#3691](https://github.com/microsoft/AzureTRE/issues/3691))
* When geting avaialble address spaces allow those allocated to deleted workspaces to be reassigned ([#3691](https://github.com/microsoft/AzureTRE/issues/3691))
* Update Python packages, and fix breaking changes ([#3764](https://github.com/microsoft/AzureTRE/issues/3764))
* Enabling support for more than 20 users/groups in Workspace API ([#3759](https://github.com/microsoft/AzureTRE/pull/3759 ))
* Airlock Import Review workspace uses dedicated DNS zone to prevent conflict with core ([#3767](https://github.com/microsoft/AzureTRE/pull/3767))
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.16.7"
__version__ = "0.16.8"
4 changes: 3 additions & 1 deletion api_app/api/routes/workspaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from api.dependencies.database import get_repository
from api.dependencies.workspaces import get_operation_by_id_from_path, get_workspace_by_id_from_path, get_deployed_workspace_by_id_from_path, get_deployed_workspace_service_by_id_from_path, get_workspace_service_by_id_from_path, get_user_resource_by_id_from_path
from db.errors import MajorVersionUpdateDenied, TargetTemplateVersionDoesNotExist, UserNotAuthorizedToUseTemplate, VersionDowngradeDenied
from db.errors import InvalidInput, MajorVersionUpdateDenied, TargetTemplateVersionDoesNotExist, UserNotAuthorizedToUseTemplate, VersionDowngradeDenied
from db.repositories.operations import OperationRepository
from db.repositories.resource_templates import ResourceTemplateRepository
from db.repositories.resources_history import ResourceHistoryRepository
Expand Down Expand Up @@ -107,6 +107,8 @@ async def create_workspace(workspace_create: WorkspaceInCreate, response: Respon
except UserNotAuthorizedToUseTemplate as e:
logging.exception("User not authorized to use template")
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e))
except InvalidInput as e:
raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e))

operation = await save_and_deploy_resource(
resource=workspace,
Expand Down
4 changes: 2 additions & 2 deletions api_app/db/repositories/workspaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,11 @@ async def validate_address_space(self, address_space):
if (address_space is None):
raise InvalidInput("Missing 'address_space' from properties.")

allocated_networks = [x.properties["address_space"] for x in await self.get_workspaces()]
allocated_networks = [x.properties["address_space"] for x in await self.get_active_workspaces()]
return is_network_available(allocated_networks, address_space)

async def get_new_address_space(self, cidr_netmask: int = 24):
workspaces = await self.get_workspaces()
workspaces = await self.get_active_workspaces()
networks = [[x.properties.get("address_space")] for x in workspaces]
networks = networks + [x.properties.get("address_spaces", []) for x in workspaces]
networks = [i for s in networks for i in s if i is not None]
Expand Down
10 changes: 9 additions & 1 deletion api_app/services/schema_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,17 @@ def enrich_template(original_template, extra_properties, is_update: bool = False
# this will help the UI render fields appropriately and know what it can send in a PATCH
if is_update:
for prop in template["properties"].values():
if "updateable" not in prop.keys() or prop["updateable"] is not True:
if not prop.get("updateable", False):
prop["readOnly"] = True

if "allOf" in template:
for conditional_property in template["allOf"]:
for condition in ["then", "else"]:
if condition in conditional_property and "properties" in conditional_property[condition]:
for prop in conditional_property[condition]["properties"].values():
if not prop.get("updateable", False):
prop["readOnly"] = True

# if there is an 'allOf' property which is empty, the validator fails - so remove the key
if "allOf" in template and template["allOf"] is None:
template.pop("allOf")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,29 +209,29 @@ async def test_get_address_space_based_on_size_with_custom_address_space_and_mis


@pytest.mark.asyncio
@patch('db.repositories.workspaces.WorkspaceRepository.get_workspaces')
@patch('db.repositories.workspaces.WorkspaceRepository.get_active_workspaces')
@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_get_address_space_based_on_size_with_address_space_only(get_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
async def test_get_address_space_based_on_size_with_address_space_only(get_active_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
workspace_with_address_space = copy.deepcopy(workspace)
workspace_with_address_space.properties["address_space"] = "10.1.4.0/24"

get_workspaces_mock.return_value = [workspace_with_address_space]
get_active_workspaces_mock.return_value = [workspace_with_address_space]
workspace_to_create = basic_workspace_request
address_space = await workspace_repo.get_address_space_based_on_size(workspace_to_create.properties)

assert "10.1.5.0/24" == address_space


@pytest.mark.asyncio
@patch('db.repositories.workspaces.WorkspaceRepository.get_workspaces')
@patch('db.repositories.workspaces.WorkspaceRepository.get_active_workspaces')
@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_get_address_space_based_on_size_with_address_space_and_address_spaces(get_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
async def test_get_address_space_based_on_size_with_address_space_and_address_spaces(get_active_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
workspace_with_address_space = copy.deepcopy(workspace)
workspace_with_address_space.properties["address_space"] = "10.1.4.0/24"

Expand All @@ -242,7 +242,7 @@ async def test_get_address_space_based_on_size_with_address_space_and_address_sp
workspace_with_both.properties["address_spaces"] = ["10.1.7.0/24", "10.1.8.0/24"]
workspace_with_both.properties["address_space"] = "10.1.7.0/24"

get_workspaces_mock.return_value = [workspace_with_address_space, workspace_with_address_spaces, workspace_with_both]
get_active_workspaces_mock.return_value = [workspace_with_address_space, workspace_with_address_spaces, workspace_with_both]
workspace_to_create = basic_workspace_request
address_space = await workspace_repo.get_address_space_based_on_size(workspace_to_create.properties)

Expand Down
10 changes: 7 additions & 3 deletions templates/workspaces/airlock-import-review/template_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,15 @@
"client_id": {
"type": "string",
"title": "Application (Client) ID",
"description": "The AAD Application Registration ID for the workspace."
"description": "The AAD Application Registration ID for the workspace.",
"updateable": true
},
"client_secret": {
"type": "string",
"title": "Application (Client) Secret",
"description": "The AAD Application Registration secret for the workspace. This value will be stored in the Workspace Key Vault.",
"sensitive": true
"sensitive": true,
"updateable": true
}
},
"required": [
Expand All @@ -113,12 +115,14 @@
"type": "boolean",
"title": "Create AAD Groups for each workspace role",
"description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.",
"default": false
"default": false,
"updateable": true
},
"aad_redirect_uris": {
"type": "array",
"title": "AAD Redirect URIs",
"description": "Redirect URIs for the AAD app in Automatic Auth mode",
"updateable": true,
"items": {
"title": "items",
"type": "object",
Expand Down
2 changes: 1 addition & 1 deletion templates/workspaces/base/porter.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
schemaVersion: 1.0.0
name: tre-workspace-base
version: 1.5.1
version: 1.5.3
description: "A base Azure TRE workspace"
dockerfile: Dockerfile.tmpl
registry: azuretre
Expand Down
14 changes: 10 additions & 4 deletions templates/workspaces/base/template_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@
"type": "boolean",
"title": "Configure Review VMs",
"description": "Allow TRE to automatically create and delete review VMs for airlock approvals",
"default": false
"default": false,
"updateable": true
}
}
}
Expand All @@ -108,6 +109,7 @@
"title": "Airlock Review Config",
"default": null,
"description": "Configuration for Airlock Review feature. Needs to be set up after workspace creation",
"updateable": true,
"properties": {
"import": {
"title": "Import Review Settings",
Expand Down Expand Up @@ -206,13 +208,15 @@
"client_id": {
"type": "string",
"title": "Application (Client) ID",
"description": "The AAD Application Registration ID for the workspace."
"description": "The AAD Application Registration ID for the workspace.",
"updateable": true
},
"client_secret": {
"type": "string",
"title": "Application (Client) Secret",
"description": "The AAD Application Registration secret for the workspace. This value will be stored in the Workspace Key Vault.",
"sensitive": true
"sensitive": true,
"updateable": true
}
},
"required": [
Expand All @@ -225,12 +229,14 @@
"type": "boolean",
"title": "Create AAD Groups for each workspace role",
"description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.",
"default": false
"default": false,
"updateable": true
},
"aad_redirect_uris": {
"type": "array",
"title": "AAD Redirect URIs",
"description": "Redirect URIs for the AAD app in Automatic Auth mode",
"updateable": true,
"items": {
"title": "items",
"type": "object",
Expand Down
2 changes: 1 addition & 1 deletion templates/workspaces/unrestricted/porter.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
schemaVersion: 1.0.0
name: tre-workspace-unrestricted
version: 0.11.1
version: 0.11.4
description: "A base Azure TRE workspace"
dockerfile: Dockerfile.tmpl
registry: azuretre
Expand Down
10 changes: 7 additions & 3 deletions templates/workspaces/unrestricted/template_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,15 @@
"client_id": {
"type": "string",
"title": "Application (Client) ID",
"description": "The AAD Application Registration ID for the workspace."
"description": "The AAD Application Registration ID for the workspace.",
"updateable": true
},
"client_secret": {
"type": "string",
"title": "Application (Client) Secret",
"description": "The AAD Application Registration secret for the workspace. This value will be stored in the Workspace Key Vault.",
"sensitive": true
"sensitive": true,
"updateable": true
}
},
"required": [
Expand All @@ -225,12 +227,14 @@
"type": "boolean",
"title": "Create AAD Groups for each workspace role",
"description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.",
"default": false
"default": false,
"updateable": true
},
"aad_redirect_uris": {
"type": "array",
"title": "AAD Redirect URIs",
"description": "Redirect URIs for the AAD app in Automatic Auth mode",
"updateable": true,
"items": {
"title": "items",
"type": "object",
Expand Down
Loading