fix!: Fix for hierarchical firewall policy (#1354)

Co-authored-by: Andrew Peabody <andrewpeabody@google.com>
terraform-google-modules · Dec 19, 2024 · a07e6a0 · a07e6a0
1 parent e372e5f
commit a07e6a0
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 56 deletions.
diff --git a/3-networks-dual-svpc/modules/hierarchical_firewall_policy/main.tf b/3-networks-dual-svpc/modules/hierarchical_firewall_policy/main.tf
@@ -15,7 +15,7 @@
  */
 
 locals {
-  policy_id = google_compute_organization_security_policy.policy.id
+  policy_id = google_compute_firewall_policy.policy.id
 }
 
 resource "random_string" "suffix" {
@@ -24,17 +24,15 @@ resource "random_string" "suffix" {
   special = false
 }
 
-resource "google_compute_organization_security_policy" "policy" {
-  provider     = google-beta
-  display_name = "${var.name}-${random_string.suffix.result}"
-  parent       = var.parent
+resource "google_compute_firewall_policy" "policy" {
+  short_name = "${var.name}-${random_string.suffix.result}"
+  parent     = var.parent
 }
 
-resource "google_compute_organization_security_policy_rule" "rule" {
-  provider = google-beta
+resource "google_compute_firewall_policy_rule" "rule" {
   for_each = var.rules
 
-  policy_id               = google_compute_organization_security_policy.policy.id
+  firewall_policy         = google_compute_firewall_policy.policy.id
   action                  = each.value.action
   direction               = each.value.direction
   priority                = each.value.priority
@@ -44,25 +42,24 @@ resource "google_compute_organization_security_policy_rule" "rule" {
   # preview                 = each.value.preview
   match {
     # description = each.value.description
-    config {
-      src_ip_ranges  = each.value.direction == "INGRESS" ? each.value.ranges : null
-      dest_ip_ranges = each.value.direction == "EGRESS" ? each.value.ranges : null
-      dynamic "layer4_config" {
-        for_each = each.value.ports
-        iterator = port
-        content {
-          ip_protocol = port.key
-          ports       = port.value
-        }
+
+    src_ip_ranges  = each.value.direction == "INGRESS" ? each.value.ranges : null
+    dest_ip_ranges = each.value.direction == "EGRESS" ? each.value.ranges : null
+    dynamic "layer4_configs" {
+      for_each = each.value.ports
+      iterator = port
+      content {
+        ip_protocol = port.key
+        ports       = port.value
       }
     }
   }
 }
 
-resource "google_compute_organization_security_policy_association" "association" {
-  provider      = google-beta
-  for_each      = toset(var.associations)
-  name          = "${local.policy_id}-${each.value}"
-  policy_id     = local.policy_id
-  attachment_id = each.value
+resource "google_compute_firewall_policy_association" "association" {
+  for_each = toset(var.associations)
+
+  name              = replace("${local.policy_id}-${each.value}", "/", "-")
+  firewall_policy   = google_compute_firewall_policy.policy.id
+  attachment_target = each.value
 }
diff --git a/3-networks-dual-svpc/modules/hierarchical_firewall_policy/outputs.tf b/3-networks-dual-svpc/modules/hierarchical_firewall_policy/outputs.tf
@@ -15,5 +15,5 @@
  */
 
 output "id" {
-  value = google_compute_organization_security_policy.policy.id
+  value = google_compute_firewall_policy.policy.id
 }
diff --git a/3-networks-dual-svpc/modules/hierarchical_firewall_policy/versions.tf b/3-networks-dual-svpc/modules/hierarchical_firewall_policy/versions.tf
@@ -19,11 +19,11 @@ terraform {
   required_providers {
     google = {
       source  = "hashicorp/google"
-      version = ">= 3.50"
+      version = ">= 3.82"
     }
     google-beta = {
       source  = "hashicorp/google-beta"
-      version = ">= 3.50"
+      version = ">= 3.82"
     }
     random = {
       source = "hashicorp/random"

diff --git a/3-networks-hub-and-spoke/modules/hierarchical_firewall_policy/main.tf b/3-networks-hub-and-spoke/modules/hierarchical_firewall_policy/main.tf
@@ -15,7 +15,7 @@
  */
 
 locals {
-  policy_id = google_compute_organization_security_policy.policy.id
+  policy_id = google_compute_firewall_policy.policy.id
 }
 
 resource "random_string" "suffix" {
@@ -24,17 +24,15 @@ resource "random_string" "suffix" {
   special = false
 }
 
-resource "google_compute_organization_security_policy" "policy" {
-  provider     = google-beta
-  display_name = "${var.name}-${random_string.suffix.result}"
-  parent       = var.parent
+resource "google_compute_firewall_policy" "policy" {
+  short_name = "${var.name}-${random_string.suffix.result}"
+  parent     = var.parent
 }
 
-resource "google_compute_organization_security_policy_rule" "rule" {
-  provider = google-beta
+resource "google_compute_firewall_policy_rule" "rule" {
   for_each = var.rules
 
-  policy_id               = google_compute_organization_security_policy.policy.id
+  firewall_policy         = google_compute_firewall_policy.policy.id
   action                  = each.value.action
   direction               = each.value.direction
   priority                = each.value.priority
@@ -44,25 +42,24 @@ resource "google_compute_organization_security_policy_rule" "rule" {
   # preview                 = each.value.preview
   match {
     # description = each.value.description
-    config {
-      src_ip_ranges  = each.value.direction == "INGRESS" ? each.value.ranges : null
-      dest_ip_ranges = each.value.direction == "EGRESS" ? each.value.ranges : null
-      dynamic "layer4_config" {
-        for_each = each.value.ports
-        iterator = port
-        content {
-          ip_protocol = port.key
-          ports       = port.value
-        }
+
+    src_ip_ranges  = each.value.direction == "INGRESS" ? each.value.ranges : null
+    dest_ip_ranges = each.value.direction == "EGRESS" ? each.value.ranges : null
+    dynamic "layer4_configs" {
+      for_each = each.value.ports
+      iterator = port
+      content {
+        ip_protocol = port.key
+        ports       = port.value
       }
     }
   }
 }
 
-resource "google_compute_organization_security_policy_association" "association" {
-  provider      = google-beta
-  for_each      = toset(var.associations)
-  name          = "${local.policy_id}-${each.value}"
-  policy_id     = local.policy_id
-  attachment_id = each.value
+resource "google_compute_firewall_policy_association" "association" {
+  for_each = toset(var.associations)
+
+  name              = replace("${local.policy_id}-${each.value}", "/", "-")
+  firewall_policy   = google_compute_firewall_policy.policy.id
+  attachment_target = each.value
 }
diff --git a/3-networks-hub-and-spoke/modules/hierarchical_firewall_policy/outputs.tf b/3-networks-hub-and-spoke/modules/hierarchical_firewall_policy/outputs.tf
@@ -15,5 +15,5 @@
  */
 
 output "id" {
-  value = google_compute_organization_security_policy.policy.id
+  value = google_compute_firewall_policy.policy.id
 }
diff --git a/3-networks-hub-and-spoke/modules/hierarchical_firewall_policy/versions.tf b/3-networks-hub-and-spoke/modules/hierarchical_firewall_policy/versions.tf
@@ -19,11 +19,11 @@ terraform {
   required_providers {
     google = {
       source  = "hashicorp/google"
-      version = ">= 3.50"
+      version = ">= 3.82"
     }
     google-beta = {
       source  = "hashicorp/google-beta"
-      version = ">= 3.50"
+      version = ">= 3.82"
     }
     random = {
       source = "hashicorp/random"

diff --git a/test/integration/org/org_test.go b/test/integration/org/org_test.go
@@ -222,7 +222,6 @@ func TestOrg(t *testing.T) {
 			assert.Equal(billingDatasetFullName, billingDataset.Get("id").String(), fmt.Sprintf("dataset %s should exist", billingDatasetFullName))
 
 			auditLogsProjectID := org.GetStringOutput("org_audit_logs_project_id")
-			auditLogsProjectNumber := gcloud.Runf(t, "projects describe %s", auditLogsProjectID).Get("projectNumber").String()
 
 			// Bucket destination
 			logsExportStorageBucketName := org.GetStringOutput("logs_export_storage_bucket_name")
@@ -247,7 +246,7 @@ func TestOrg(t *testing.T) {
 			prjLinkedDsName := org.GetStringOutput("logs_export_project_linked_dataset_name")
 			prjLinkedDs := gcloud.Runf(t, "logging links describe %s --bucket=%s --location=%s --project=%s", prjLinkedDatasetID, prjLogsExportLogBktName, defaultRegion, auditLogsProjectID)
 			assert.Equal(prjLinkedDsName, prjLinkedDs.Get("name").String(), "log bucket linked dataset name should match")
-			prjBigqueryDatasetID := fmt.Sprintf("bigquery.googleapis.com/projects/%s/datasets/%s", auditLogsProjectNumber, prjLinkedDatasetID)
+			prjBigqueryDatasetID := fmt.Sprintf("bigquery.googleapis.com/projects/%s/datasets/%s", auditLogsProjectID, prjLinkedDatasetID)
 			assert.Equal(prjBigqueryDatasetID, prjLinkedDs.Get("bigqueryDataset.datasetId").String(), "log bucket BigQuery dataset ID should match")
 
 			// add filter exclusion