firewall pipeline step in template_schema.json fails if rule description is an empty string or not set

[JC-wk]

In the firewall template_schema.json description defaults to an empty string. When an empty string is provided to terraform for the rule description an error occurs. firewall policy description field is commented out for network rules

AzureTRE/templates/shared_services/firewall/terraform/rules.tf

Line 19 in 9aa5073

# description = rule.value.description

the application rule however description is optional but if set it can't be an empty string due to the error it produces.

https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/firewall_policy_rule_collection_group#description-1

There is inconsistent behaviour between "network_rule_collections" which has descriptions marked as deprecated (and commented out in terraform) but "rule_collections" which still accept a description but defaults to blank causing this error. It would be better if they behaved the same, either both taking descriptions (and putting a non-empty string requirement) or removing that entirely (name should be sufficient)

as a minimum application rules should not accept empty strings for the description field

Describe the bug
firewall pipeline step in template_schema.json fails if rule description is an empty string or not set

 Error: expected "application_rule_collection.8.rule.4.description" to not be an empty string, got  
  
   with azurerm_firewall_policy_rule_collection_group.dynamic_application, 
   on rules.tf line 36, in resource "azurerm_firewall_policy_rule_collection_group" "dynamic_application": 
   36: resource "azurerm_firewall_policy_rule_collection_group" "dynamic_application" { 

Steps to reproduce

1. Create a firewall application rule collection "rule_collections" in a pipeline via template_schema.json.
2. Create a rule without a description set
3. Deploy the bundle and observe the error

**Azure TRE release version 0.25.0 **

Deployed Azure TRE components

Azure TRE
UI Version:
0.8.15
API Version:
0.24.5

            "name": "rule_collections",
              "type": "array",
              "arraySubstitutionAction": "replace",
              "arrayMatchField": "name",
              "value": {
                "name": "arc_svc_{{ resource.id }}",
                "action": "Allow",
                "rules": [
                {
                  "name": "Azure",
                  "description": "Allow Azure", <--- To reproduce don't set this or set it to an empty string
                  "source_addresses": "{{ resource.properties.service_addresses }}",
                  "protocols": [
                    {
                      "type": "Https",
                      "port": "443"
                    }
                  ],
                  "fqdn_tags": [
                    "AzureCloud"
                  ]

                }

[marrobi]

@JC-wk Thanks. I think descriptions can be useful to explain why the rules are there.

Where is the commented out description field?

I think we go for both taking descriptions and having a none empty string requirement.

[JC-wk]

@marrobi It's commented out here for network rules and has the deprecated text against it:

AzureTRE/templates/shared_services/firewall/terraform/rules.tf

Line 19 in 9aa5073

# description = rule.value.description

AzureTRE/templates/shared_services/firewall/template_schema.json

Line 239 in 9aa5073

"title": "description DEPRECATED",

For the application rules it's still implemented (needs a non-null default and non-null pattern)

AzureTRE/templates/shared_services/firewall/template_schema.json

Line 62 in 9aa5073

"title": "description",

So is it worth un-deprecating the network rule description and putting the same requirements in to remain consistent?
It should allow the user to omit the description and use a sensible default value to not break existing network rules the description is never used anywhere inside the firewall policy rule and is optional in terraform

[marrobi]

@tamirkamara any idea why this is marked as depreciated?

@JC-wk good shout on the backward compact, my view is it should be in the TF too so we see it in the Azure resource.

[marrobi]

Both are supported here - https://learn.microsoft.com/en-us/rest/api/virtualnetwork/firewall-policy-rule-collection-groups/create-or-update?view=rest-virtualnetwork-2024-07-01&tabs=HTTP#networkrule

[JC-wk]

Ah, makes sense to un-deprecate then and ensure non-null, they are not displayed in the azure portal when viewing a rule but they do show when you export the policy template.

[marrobi]

@JC-wk what you think of #4694?

[JC-wk]

It looks good, I will give it a try