Migrate Azure Firewall and route tables to core configuration #4342

marrobi · 2025-02-07T20:47:27Z

Move Azure Firewall Deployment to Core

Why:

Avoids the issue of the firewall and rules for core not being deployed when it gets deployed, this means if the shared shared service deployment hits an issue the system can still operate. ( Add docs around recovering from a failed firewall. #4243 )
Resolves one of the issues to have a reduced service bus sku that requires traffic to be routed through the firewall ( 'Service Bus Namespace' Continues Running Even After $ make tre-stop #3953 (comment) ).
Opens up the option in future to deploy the system without a firewall (for dev, hence reducing cost)
Removes the complexity of core config vars being used in shared service deployment.

Had to modify GitHub workflow for linting. Can someone verify this is correct?

Tested upgrade a few times, more wouldn't hurt.

…ve to core

….com/microsoft/AzureTRE into marrobi/firewall-to-core

…robi/firewall-to-core

github-actions · 2025-02-07T20:48:11Z

Unit Test Results

0 tests 0 ✅ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit c61ee60.

♻️ This comment has been updated with latest results.

martinpeck · 2025-03-20T15:36:41Z

@marrobi please update with main and then publish for review

…robi/firewall-to-core

Copilot

Pull Request Overview

This PR migrates the Azure Firewall deployment and associated route table configurations to the core configuration. Key changes include:

Upgrading the version in the porter.yaml template.
Removing firewall-related parameters and the tre_resource_id from the shared services deployment template.
Updating the CHANGELOG with the new migration entry.

Reviewed Changes

Copilot reviewed 6 out of 24 changed files in this pull request and generated no comments.

File	Description
templates/shared_services/firewall/porter.yaml	Updated version and removed firewall-related parameters and IDs.
CHANGELOG.md	Added a changelog entry for the migration of Azure Firewall configurations.

Files not reviewed (18)

Makefile: Language not supported
core/terraform/.terraform.lock.hcl: Language not supported
core/terraform/firewall/firewall.tf: Language not supported
core/terraform/firewall/import_state.sh: Language not supported
core/terraform/firewall/locals.tf: Language not supported
core/terraform/firewall/main.tf: Language not supported
core/terraform/firewall/outputs.tf: Language not supported
core/terraform/firewall/remove_state.sh: Language not supported
core/terraform/firewall/rules.tf: Language not supported
core/terraform/firewall/variables.tf: Language not supported
core/terraform/main.tf: Language not supported
core/terraform/network/outputs.tf: Language not supported
core/terraform/routetable.tf: Language not supported
core/terraform/variables.tf: Language not supported
core/version.txt: Language not supported
templates/shared_services/firewall/template_schema.json: Language not supported
templates/shared_services/firewall/terraform/data.tf: Language not supported
templates/shared_services/firewall/terraform/import_state.sh: Language not supported

Comments suppressed due to low confidence (3)

templates/shared_services/firewall/porter.yaml:45

The removal of firewall-related parameters is intended by the PR; however, please verify that all consumers of this template have been updated to use the new core configuration and no references to these parameters remain.

-  - name: firewall_sku

templates/shared_services/firewall/porter.yaml:59

Removing the tre_resource_id from the install/upgrade/uninstall sections requires confirmation that resource identification is now handled elsewhere; please ensure the new configuration covers this need.

-        tre_resource_id: ${ bundle.parameters.id }

CHANGELOG.md:9

[nitpick] Consider aligning the changelog entry with the PR title by using 'core configuration' instead of 'Core Terraform' for consistency.

+* Migrate Azure Firewall and Route Tables to Core Terraform ([#4342](https://github.com/microsoft/AzureTRE/pull/4342))

marrobi · 2025-03-31T13:02:00Z

This would avoid https://github.com/microsoft/AzureTRE/actions/runs/14170065338/job/39697620186

tamirkamara · 2025-04-02T07:45:02Z

core/terraform/firewall/import_state.sh

Why do you import the state here if we usually do it in the migration script?

Hmm, I moved the file over from the firewall bundle, from when we went the other way. I can move this to the migrate script.

tamirkamara · 2025-04-02T07:45:46Z

core/terraform/main.tf

+  depends_on = [
+    module.network,
+  ]


I think this is redundant.

Will remove.

tamirkamara · 2025-04-02T07:49:11Z

core/terraform/firewall/firewall.tf

Does the FW really needs to be a module?

The reason was to leave the option to have it conditional in the future. If we don't want to deploy it for a dev environment for example.

tamirkamara · 2025-04-02T07:50:52Z

core/terraform/firewall/remove_state.sh

Same here. I think it should be move to the migration script.

Will revisit.

tamirkamara · 2025-04-02T07:52:17Z

templates/shared_services/firewall/porter.yaml

@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-shared-service-firewall
-version: 1.3.3
+version: 1.4.0
 description: "An Azure TRE Firewall shared service"


I understand we can't change the name, but we can consider changing the description, template schema and docs to reflect the fact this is now just about managing the workspace level firewall policy rules.

Yup, primarily wanted to see if thought this works as a change in principal. If agreed, will go through and fix up.

tamirkamara · 2025-04-02T07:56:12Z

templates/shared_services/firewall/terraform/locals.tf

-  tre_shared_service_tags = {
-    tre_id                = var.tre_id
-    tre_shared_service_id = var.tre_resource_id
-  }

  api_driven_application_rule_collection = jsondecode(base64decode(var.api_driven_rule_collections_b64))
  api_driven_network_rule_collection     = jsondecode(base64decode(var.api_driven_network_rule_collections_b64))

  firewall_policy_name = "fw-policy-${var.tre_id}"


I think it is now quite easy to provide this at run time as since it is set in the core it should be provided from there as a bundle variable.

Not sure what you mean? The policy name?

tamirkamara · 2025-04-02T07:57:13Z

templates/shared_services/firewall/terraform/rules.tf

-          name = rule.value.name
-          # description      = rule.value.description
+          name             = rule.value.name
+          description      = rule.value.description


Is it confirmed that network rules now support a description?

Will check. It deploys and works... can't remember why changed this.

marrobi and others added 8 commits January 7, 2025 18:38

Add Terraform configuration for Azure Firewall and route tables to mo…

762ca46

…ve to core

Airlock function storage to use manage identity

49da7fc

version

83d3f07

changelog

5015aae

storage permissions

92d47e7

Merge branch 'tamirkamara/function-host-storage-mi' of https://github…

fa16b21

….com/microsoft/AzureTRE into marrobi/firewall-to-core

Fix merge issues.

57a30df

Merge branch 'main' of https://github.com/microsoft/AzureTRE into mar…

3646280

…robi/firewall-to-core

marrobi added 2 commits February 7, 2025 20:51

remove forced tunneling from shared service

e517835

Fix linting

f942623

marrobi mentioned this pull request Mar 20, 2025

Add Service Bus SKU configuration to Terraform resources #4256

Draft

martinpeck assigned marrobi Mar 20, 2025

martinpeck added the triage Being triaged before any action taken. label Mar 20, 2025

marrobi added 2 commits March 20, 2025 16:35

Merge branch 'main' of https://github.com/microsoft/AzureTRE into mar…

dbd3b15

…robi/firewall-to-core

Update to work with latest changes.

ff24de2

marrobi force-pushed the marrobi/firewall-to-core branch from af6ed73 to ff24de2 Compare March 21, 2025 12:06

marrobi added 3 commits March 21, 2025 12:43

Fix linting

9cbf076

fix linting

72fa0da

Update changelog

917efeb

marrobi requested a review from Copilot March 21, 2025 13:21

Copilot AI reviewed Mar 21, 2025

View reviewed changes

marrobi added 3 commits March 21, 2025 13:30

Remove Firewall SKU from shared services and RP

c73a127

fix linting

07ebda0

fix linting

c61ee60

marrobi marked this pull request as ready for review March 21, 2025 14:09

tamirkamara reviewed Apr 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Azure Firewall and route tables to core configuration #4342

Migrate Azure Firewall and route tables to core configuration #4342

marrobi commented Feb 7, 2025 •

edited

Loading

github-actions bot commented Feb 7, 2025 •

edited

Loading

martinpeck commented Mar 20, 2025

Copilot AI left a comment

marrobi commented Mar 31, 2025

tamirkamara Apr 2, 2025

marrobi Apr 2, 2025

tamirkamara Apr 2, 2025

marrobi Apr 2, 2025

tamirkamara Apr 2, 2025

marrobi Apr 2, 2025

tamirkamara Apr 2, 2025

marrobi Apr 2, 2025

tamirkamara Apr 2, 2025

marrobi Apr 2, 2025

tamirkamara Apr 2, 2025

marrobi Apr 2, 2025

tamirkamara Apr 2, 2025

marrobi Apr 2, 2025

Migrate Azure Firewall and route tables to core configuration #4342

Are you sure you want to change the base?

Migrate Azure Firewall and route tables to core configuration #4342

Conversation

marrobi commented Feb 7, 2025 • edited Loading

github-actions bot commented Feb 7, 2025 • edited Loading

Unit Test Results

martinpeck commented Mar 20, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

marrobi commented Mar 31, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marrobi commented Feb 7, 2025 •

edited

Loading

github-actions bot commented Feb 7, 2025 •

edited

Loading