Cannot update workspace with custom address space

[jlabhard-sg]

Describe the bug
Creating a workspace with a custom address space and then updating it will fail with The custom 'address_space' you requested does not fit in the current network. This is because the method workspaces.validate_address_space() will check the allocated network on update, find that the network is already allocated (to the current workspace), and return the error.

The same error arises when creating a workspace and specifying a custom address space which overlaps with a workspace that has been deleted.

Steps to reproduce

1. Create a workspace with custom address space
2. Update it without any changes

Suggested changes

• Custom address space should not be updateable
• The address space should not be subject to validation on update
• The allocated network validation should not included the deleted workspaces and the current workspace

[marrobi]

@jlabhard-sg thanks for reporing. Not many people use custom address spaces as far as I am aware.

Updating should not be possible as you can't change the address space once it has things on it. Do you agree?

Can you try changing:

https://github.com/marrobi/AzureTRE/blob/main/templates/workspaces/base/template_schema.json#L52-L53

To false and see if that sorts things out?

[jlabhard-sg]

Hi @marrobi ,
Yes I agree, my issue arises when I update the workspace without changing the address space.
I will try your suggested change to see if it fixes it and update you here.

[jlabhard-sg]

@marrobi Setting the address_spaces properties to non-updateable did not solve the issue.

[marrobi]

Ok, it will need further debugging/time to look for an appropriate solution.

If you can come up with a working solution, welcome a contribution, it's unlikely we will have chance to dig into it for a while. I tend to think not validating the address space if it has not changed/is and update , makes most sense.

[marrobi]

@jlabhard-sg just taking a look at this, I'm trying to work out the code path that results in validate_address_space being called on an update.

It is called in get_address_space_based_on_size which is called only on create, here
-https://github.com/marrobi/AzureTRE/blob/0c5d945f84086daf479a906b2e362d2ae544702b/api_app/db/repositories/workspaces.py#L75

II will try to reproduce myself, but confused as to why this would occur.

[jlabhard-sg]

@marrobi It is possible that I mixed the error message from two different issues when looking for them in the app insights. "Does not fit in the current network" probably happened when trying to create a workspace with an address space that was used by a deleted workspace. I believe this is still undesirable.

However, updating a workspace with custom address space also doesn't work. But the only error message I can find is "Unevaluated properties <address_space> is not allowed". Currently, I am able to bypass it by updating the resource in CosmosDB and using the swagger UI to update the workspace.

[marrobi]

Ok, other than the excluding deleted the issue is as address_space_size is not updatable, its not passed with the patch payload. This means that validation fails as it's not expecting the actual address space size in the payload, as its only accepted if address space size is in the payload as custom.

address_space should not be updatable, but read only, this is configured here:

AzureTRE/api_app/services/schema_service.py

Lines 51 to 54 in 63aa23d

	if is_update:
	for prop in template["properties"].values():
	if "updateable" not in prop.keys() or prop["updateable"] is not True:
	prop["readOnly"] = True

We are only iterating through the top level properties and making fields read only, not the allOf ones, which address space is.

[marrobi]

@jlabhard-sg any change you can build the image in #3714 and give it a go and see if the issue is resolved.

I've given it a go and is good for me. Thanks.

[jlabhard-sg]

@marrobi Was able to test and validate the fix in #3714. Thank you!

[marrobi]

Great, might take a little while to get it merged in, but good to know it works.