Firewall Rules Submodule Requires Ports (even for ICMP or "all" protocols)

[huron25]

When trying to use the firewall rules submodule, I get an error when trying to use this allow block:

allow = [{ protocol = "icmp" }]

I also get it when I try this allow block:

allow = [{ protocol = "all"
}]

Error message:

Error: Invalid value for module argument on .terraform/modules/project-manual-test.vpc/main.tf line 80, in module "firewall_rules": 80: rules = local.rules

The given value is not suitable for child module variable "rules" defined at .terraform/modules/project-manual-test.vpc/modules/firewall-rules/variables.tf:25,1-17: element 2: attribute "allow": element 0: attribute "ports" is required.

I tested by creating the resources outside the module and both tests worked (https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_firewall)

resource "google_compute_firewall" "test_rule_icmp" { name = "test-firewall-rule-icmp" network = module.vpc.network_self_link

allow { protocol = "icmp"
}
source_ranges = local.networks }

resource "google_compute_firewall" "test_rule_all" { name = "test-firewall-rule-all-in" network = module.vpc.network_self_link

allow { protocol = "all"
}

target_tags = ["block"] source_ranges = local.networks }

I tried to determine where it's requiring ports in a variable if protocol is set, but it appears that it should only require it for TCP/UDP (as is documented for compute.firewalls api - https://cloud.google.com/compute/docs/reference/rest/v1/firewalls/list - An optional list of ports to which this rule applies. This field is only applicable for the UDP or TCP protocol. Each entry must be either an integer or a range. If not specified, this rule applies to connections through any port.)

[huron25]

As a workaround, it appears that I may be able to circumvent it by setting the ports to null.

I guess the request now is to validate on TCP/UDP and automatically set null for the others

[andrea-berling]

@huron25 I'm having the same issue. Can you please let me know if setting ports to null worked out for you? I'm afraid it wouldn't, since in the plan it shows that setting ports to null results in an empty list for the ports argument of google_compute_firewall:allow, and thus the workaround would be to set ports to [1-65536] instead

Edit: nevermind, I checked with explicit google_compute_firewall blocks, it looks like omitting the ports field also results in an empty list for the field in the plan, so your workaround should definitely work

[huron25]

Yeah @andrea-berling - I think the issue stems from requiring ports for everything when it should only be for TCP/UDP and others, like ICMP, do not have ports (and should be automatically set to "null").

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

[huron25]

Commenting to see if anyone has taken a look at this.

[bharathkkb]

@huron25 Unfortunately terraform core does not allow us to have defaults for object variables. The ports are a required input in the module due to the variable type https://github.com/terraform-google-modules/terraform-google-network/blob/e1a1a6b48ef8ca15a0e0b35a475c7e4d4fbc874a/modules/firewall-rules/variables.tf#L38-L41

The good news is they are working on it and there is experimental support. I will mark this as FR to use this feature when it graduates.

[rk-p2p]

Working with:

    allow = [{
      protocol = "icmp"
      ports    = []
    }

[logicbomb421]

Just ran into this myself.

@bharathkkb, it looks like optional variables with defaults have graduated from experimental as of 1.3.0. I believe we can rewrite the allow/deny variable definition as:

  ...
   allow = list(object({ 
     protocol = string 
     ports    = optional(list(string), []) 
   }))
  ...

[imrannayer]

fixed in #438