Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix factory default value for rule ports in firewall policy module #1996

Merged
merged 1 commit into from
Jan 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 21 additions & 1 deletion modules/net-firewall-policy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ module "firewall-policy" {
ingress_rules_file_path = "configs/ingress.yaml"
}
}
# tftest modules=1 resources=5 files=cidrs,egress,ingress inventory=factory.yaml
# tftest modules=1 resources=6 files=cidrs,egress,ingress inventory=factory.yaml
```

```yaml
Expand All @@ -205,6 +205,8 @@ rfc1918:
- 10.0.0.0/8
- 172.16.0.0/12
- 192.168.0.0/24
gke-nodes-range:
- 10.0.1.0/24
```

```yaml
Expand All @@ -229,7 +231,25 @@ icmp:
- 10.0.0.0/8
layer4_configs:
- protocol: icmp
issue-1995:
priority: 10020
description: Allow intra-cluster communication required by k8s networking model
enable_logging: true
target_service_accounts:
- sa-gke-cluster@burner-project.iam.gserviceaccount.com
match:
source_ranges:
- gke-nodes-range
layer4_configs:
- protocol: tcp
ports:
- 1-65535
- protocol: udp
ports:
- 1-65535
- protocol: icmp
```

<!-- BEGIN TFDOC -->
## Variables

Expand Down
4 changes: 2 additions & 2 deletions modules/net-firewall-policy/factory.tf
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ locals {
? [{ protocol = "all", ports = null }]
: [
for c in v.match.layer4_configs :
merge({ protocol = "all", ports = null }, c)
merge({ protocol = "all", ports = [] }, c)
]
)
}
Expand Down Expand Up @@ -106,7 +106,7 @@ locals {
? [{ protocol = "all", ports = null }]
: [
for c in v.match.layer4_configs :
merge({ protocol = "all", ports = null }, c)
merge({ protocol = "all", ports = [] }, c)
]
)
}
Expand Down
7 changes: 0 additions & 7 deletions modules/net-firewall-policy/hierarchical.tf
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,6 @@ resource "google_compute_firewall_policy_association" "hierarchical" {
firewall_policy = google_compute_firewall_policy.hierarchical.0.name
}

output "foo" {
value = {
rules = local.rules
cidrs = local.factory_cidrs
}
}

resource "google_compute_firewall_policy_rule" "hierarchical" {
# Terraform's type system barfs in the condition if we use the locals map
for_each = toset(
Expand Down
74 changes: 60 additions & 14 deletions tests/modules/net_firewall_policy/examples/factory.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,48 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

values:
module.firewall-policy.google_compute_firewall_policy.hierarchical[0]:
description: null
parent: folders/1234567890
short_name: test-1
timeouts: null
module.firewall-policy.google_compute_firewall_policy_association.hierarchical["test"]:
attachment_target: folders/4567890123
name: test-1-test
timeouts: null
module.firewall-policy.google_compute_firewall_policy_rule.hierarchical["egress/smtp"]:
action: deny
description: null
direction: EGRESS
disabled: false
enable_logging: null
match:
- dest_address_groups: null
dest_fqdns: null
dest_ip_ranges:
- 10.0.0.0/8
- 172.16.0.0/12
- 192.168.0.0/24
dest_region_codes: null
dest_threat_intelligences: null
layer4_configs:
- ip_protocol: tcp
ports:
- '25'
src_address_groups: null
src_fqdns: null
src_ip_ranges: null
src_region_codes: null
src_threat_intelligences: null
priority: 900
target_resources: null
target_service_accounts: null
timeouts: null
module.firewall-policy.google_compute_firewall_policy_rule.hierarchical["ingress/icmp"]:
action: allow
description: null
direction: INGRESS
disabled: false
enable_logging: null
Expand All @@ -31,7 +64,7 @@ values:
dest_threat_intelligences: null
layer4_configs:
- ip_protocol: icmp
ports: null
ports: []
src_address_groups: null
src_fqdns: null
src_ip_ranges:
Expand All @@ -41,34 +74,42 @@ values:
priority: 1000
target_resources: null
target_service_accounts: null
module.firewall-policy.google_compute_firewall_policy_rule.hierarchical["egress/smtp"]:
action: deny
direction: EGRESS
timeouts: null
module.firewall-policy.google_compute_firewall_policy_rule.hierarchical["ingress/issue-1995"]:
action: allow
description: Allow intra-cluster communication required by k8s networking model
direction: INGRESS
disabled: false
enable_logging: null
enable_logging: true
match:
- dest_address_groups: null
dest_fqdns: null
dest_ip_ranges:
- 10.0.0.0/8
- 172.16.0.0/12
- 192.168.0.0/24
dest_ip_ranges: null
dest_region_codes: null
dest_threat_intelligences: null
layer4_configs:
- ip_protocol: tcp
ports:
- '25'
- 1-65535
- ip_protocol: udp
ports:
- 1-65535
- ip_protocol: icmp
ports: []
src_address_groups: null
src_fqdns: null
src_ip_ranges: null
src_ip_ranges:
- 10.0.1.0/24
src_region_codes: null
src_threat_intelligences: null
priority: 900
priority: 10020
target_resources: null
target_service_accounts: null
target_service_accounts:
- sa-gke-cluster@burner-project.iam.gserviceaccount.com
timeouts: null
module.firewall-policy.google_compute_firewall_policy_rule.hierarchical["ingress/ssh"]:
action: allow
description: null
direction: INGRESS
disabled: false
enable_logging: null
Expand All @@ -91,8 +132,13 @@ values:
priority: 1002
target_resources: null
target_service_accounts: null
timeouts: null

counts:
google_compute_firewall_policy: 1
google_compute_firewall_policy_association: 1
google_compute_firewall_policy_rule: 3
google_compute_firewall_policy_rule: 4
modules: 1
resources: 6

outputs: {}