Enable Workspace to deploy to separate subscription #4455
Conversation
Unit Test Results658 tests 658 ✅ 7s ⏱️ Results for commit 0cfbea0. ♻️ This comment has been updated with latest results. |
askew
changed the base branch from
main
to
Azure-Single_server-to-flexible-server
askew
changed the base branch from
Azure-Single_server-to-flexible-server
to
main
askew
force-pushed
the
askew/multi-subscription
branch
from
2da7efb to
26f2c98
Compare
There was a problem hiding this comment.
Pull Request Overview
Enables deploying workspace services to a separate Azure subscription by passing through a new workspace_subscription_id and configuring Terraform and CostService to handle multiple subscriptions.
- Introduces
workspace_subscription_idvariable and provider configuration for dual subscriptions. - Updates Terraform templates and Porter bundles to accept and propagate the workspace subscription ID.
- Extends
CostServiceand its tests to aggregate costs across default and workspace-specific subscriptions.
Reviewed Changes
Copilot reviewed 54 out of 56 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/workspace_services/guacamole/terraform/web_app.tf | Updated azapi_update_resource body syntax |
| templates/workspace_services/guacamole/terraform/variables.tf | Added workspace_subscription_id variable |
| templates/workspace_services/guacamole/terraform/providers.tf | Configured dual azurerm providers for multi-subscription |
| templates/workspace_services/guacamole/terraform/data.tf | Assigned core provider to client_config and data blocks |
| templates/workspace_services/guacamole/template_schema.json | Added empty uiSchema section |
| templates/workspace_services/guacamole/porter.yaml | Bumped version and added workspace_subscription_id parameter |
| templates/workspace_services/guacamole/parameters.json | Mapped WORKSPACE_SUBSCRIPTION_ID environment variable |
| templates/shared_services/gitea/terraform/main.tf | Updated azapi provider version |
| templates/shared_services/gitea/terraform/gitea-webapp.tf | Updated azapi_update_resource body syntax |
| templates/shared_services/cyclecloud/porter.yaml | Added steps to select Azure subscription |
| templates/shared_services/certs/porter.yaml | Added steps to select Azure subscription |
| templates/shared_services/airlock_notifier/porter.yaml | Added steps to select Azure subscription |
| api_app/tests_ma/test_services/test_cost_service.py | Added tests for multi-subscription cost retrieval |
| api_app/services/cost_service.py | Enhanced cost queries and client management for multiple subscriptions |
| api_app/resources/strings.py | Added new error string for unsupported multi-subscription |
| api_app/api/routes/workspaces.py | Injected workspace_subscription_id into create flows |
| api_app/api/routes/costs.py | Corrected typo in query parameter descriptions |
| api_app/_version.py | Bumped API version |
Files not reviewed (2)
- templates/shared_services/gitea/terraform/.terraform.lock.hcl: Language not supported
- templates/workspace_services/guacamole/terraform/.terraform.lock.hcl: Language not supported
|
@askew are they any docs for this PR? What do I need to do in the second subscription? |
There was a problem hiding this comment.
I've tested this today across two subscription. Happy existing functionality seems to be fine, and deploy of workspace, guacamole and VMs seems to be fine into a second subscription.
@askew could do with something in the docs around the permissions needed to deploy to a second subscription.
Thanks, great job.
askew
force-pushed
the
askew/multi-subscription
branch
from
58b2cc3 to
921d40b
Compare
|
/test-extended 3c9b4d7 |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/15731724231 (with refid (in response to this comment from @marrobi) |
|
/test-extended 51ca8d9 |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/15875355716 (with refid (in response to this comment from @marrobi) |
askew
force-pushed
the
askew/multi-subscription
branch
from
695034f to
e090208
Compare
There was a problem hiding this comment.
Hi, I haven’t completed the full review yet, but I wanted to pause and ask a couple of questions before continuing:
As I understand it, not all workspace services support installation in a separate subscription — is that correct? What happens if a user tries to install a workspace service that doesn’t support it? Do we have a plan on supporting it in all WS ? If not maybe worth adding a documentation saying what is supported in this case
How would this be handled for services like Databricks, which depend on both a workspace component and a shared service (e.g., Databricks Auth)? Is cross-subscription deployment supported for these kinds of dependencies as well? I know it is not part of this PR, I just have a feeling this is where there might be a problem. Again if shouldn't be supported then maybe worth add a documentation on that
| config.SUBSCRIPTION_ID, | ||
| base_url=config.RESOURCE_MANAGER_ENDPOINT, | ||
| credential_scopes=config.CREDENTIAL_SCOPES) | ||
| self.__resource_clients = { |
There was a problem hiding this comment.
Maybe we can use the new get_resource_management_client method here and pass config.SUBSCRIPTION_ID to it?
|
@LizaShak, to answer your questions: Firstly regarding the workspace service bundles needed to support the change. There is nothing to prevent any workspace service working in a separate subscription, as @marrobi points out, virtual network peering works across subscriptions, so any hub connectivity will still work. However the Terraform templates needed to deploy workspace services need to be updated as they need to know the context of the workspace subscription, and they also need the context of the hub subscription. As mentioned in the description this PR includes this change only for the Guacamole and VM templates. Further PRs will be required to update other workspace service templates. What happens if you try to deploy a template that doesn't support separate subscription? It will simply failed during the Terraform deployment. We did look at making this cleaner, in fact this PR had this earlier on, but it required additional effort in the templates and we rolled this back opting for the simpler Terraform fail. |
Resolves #1073
What is being addressed
To enable workspaces to be deployed to separate subscriptions to the TRE core components, the workspace bundle needs take the workspace subscription id as a parameter and the Terraform needs to manage two sets of AzureRM providers settings, one for the workspace, and one core that it links to.
How is this addressed
The API has been modified to automatically inject the workspace subscription id when a workspace service or user resource is created. Each workspace service bundle must be modified to have a
workspace_subscription_idparameter and deploy its resources to this subscription. This PR includes this change for the Guacamole bundle and the Windows and Linux virtual machine bundles.In order for the bundle to deploy to the separate subscription, the Porter workload identity (
id-vmss-treid) must be granted the Owner (or Contributor + User Access Administrator) role on the workspace subscription. This is not currently enabled by this PR, but could be added as an enhancement if the feature request #4528 is implemented.As the identity
id-vmss-treidwill have permissions on multiple subscriptions, any time a Porter pipeline is usingaz loginit must also select the core subscription. These changes are made in this PR in the variousporter.yamlfiles.