Fix review comment: Change ports magic to explicit udpFirewall

googleforgames · Jan 24, 2023 · b50e320 · b50e320
1 parent 63bd753
commit b50e320
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 36 deletions.
diff --git a/build/terraform/e2e/gke-autopilot/module.tf b/build/terraform/e2e/gke-autopilot/module.tf
@@ -42,7 +42,7 @@ module "gke_cluster" {
     "location" = "us-west1"
   }
 
-  ports = "" // firewall is created at the project module level
+  udpFirewall = false // firewall is created at the project module level
 }
 
 provider "helm" {

diff --git a/build/terraform/e2e/gke-standard/module.tf b/build/terraform/e2e/gke-standard/module.tf
@@ -49,7 +49,7 @@ module "gke_cluster" {
     "project"              = var.project
   }
 
-  ports = "" // firewall is created at the project module level
+  udpFirewall = false // firewall is created at the project module level
 }
 
 provider "helm" {

diff --git a/install/terraform/modules/gke-autopilot/cluster.tf b/install/terraform/modules/gke-autopilot/cluster.tf
@@ -79,7 +79,7 @@ resource "google_container_cluster" "primary" {
 }
 
 resource "google_compute_firewall" "default" {
-  count   = var.ports != "" ? 1 : 0
+  count   = var.udpFirewall ? 1 : 0
   name    = length(var.firewallName) == 0 ? "game-server-firewall-${local.name}" : var.firewallName
   project = local.project
   network = local.network

diff --git a/install/terraform/modules/gke-autopilot/variables.tf b/install/terraform/modules/gke-autopilot/variables.tf
@@ -12,16 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# Ports can be overriden using tfvars file. If ports is empty, no firewall is declared.
-variable "ports" {
-  default = "7000-8000"
-}
-
-# SourceRanges can be overriden using tfvars file
-variable "sourceRanges" {
-  default = "0.0.0.0/0"
-}
-
 # Set of GKE cluster parameters which defines its name, zone
 # and primary node pool configuration.
 # It is crucial to set valid ProjectID for "project".
@@ -30,16 +20,32 @@ variable "cluster" {
   type        = map(any)
 
   default = {
-    "name"                    = "test-cluster"
-    "project"                 = "agones"
-    "location"                = "us-west1"
-    "network"                 = "default"
-    "subnetwork"              = ""
-    "releaseChannel"          = "REGULAR"
-    "kubernetesVersion"       = "1.24"
+    "name"              = "test-cluster"
+    "project"           = "agones"
+    "location"          = "us-west1"
+    "network"           = "default"
+    "subnetwork"        = ""
+    "releaseChannel"    = "REGULAR"
+    "kubernetesVersion" = "1.24"
   }
 }
 
+# udpFirewall specifies whether to create a UDP firewall named
+# `firewallName` with port range `ports`, source range `sourceRanges` 
+variable "udpFirewall" {
+  default = true
+}
+
+# Ports can be overriden using tfvars file
+variable "ports" {
+  default = "7000-8000"
+}
+
+# SourceRanges can be overriden using tfvars file
+variable "sourceRanges" {
+  default = "0.0.0.0/0"
+}
+
 variable "firewallName" {
   description = "name for the cluster firewall. Defaults to 'game-server-firewall-{local.name}' if not set."
   type        = string

diff --git a/install/terraform/modules/gke/cluster.tf b/install/terraform/modules/gke/cluster.tf
@@ -218,7 +218,7 @@ resource "google_container_cluster" "primary" {
 }
 
 resource "google_compute_firewall" "default" {
-  count   = var.ports != "" ? 1 : 0
+  count   = var.udpFirewall ? 1 : 0
   name    = length(var.firewallName) == 0 ? "game-server-firewall-${local.name}" : var.firewallName
   project = local.project
   network = local.network

diff --git a/install/terraform/modules/gke/variables.tf b/install/terraform/modules/gke/variables.tf
@@ -12,25 +12,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# Ports can be overriden using tfvars file
-variable "ports" {
-  default = "7000-8000"
-}
-
-# SourceRanges can be overriden using tfvars file
-variable "sourceRanges" {
-  default = "0.0.0.0/0"
-}
-
 # Set of GKE cluster parameters which defines its name, zone
 # and primary node pool configuration.
 # It is crucial to set valid ProjectID for "project".
 variable "cluster" {
   description = "Set of GKE cluster parameters."
-  type        = map
+  type        = map(any)
 
   default = {
-    "location"		      = "us-west1-c"
+    "location"                = "us-west1-c"
     "name"                    = "test-cluster"
     "machineType"             = "e2-standard-4"
     "initialNodeCount"        = "4"
@@ -40,12 +30,28 @@ variable "cluster" {
     "kubernetesVersion"       = "1.24"
     "windowsInitialNodeCount" = "0"
     "windowsMachineType"      = "e2-standard-4"
-    "autoscale"		      = false
-    "minNodeCount"	      = "1"
-    "maxNodeCount"	      = "5"
+    "autoscale"               = false
+    "minNodeCount"            = "1"
+    "maxNodeCount"            = "5"
   }
 }
 
+# udpFirewall specifies whether to create a UDP firewall named
+# `firewallName` with port range `ports`, source range `sourceRanges` 
+variable "udpFirewall" {
+  default = true
+}
+
+# Ports can be overriden using tfvars file
+variable "ports" {
+  default = "7000-8000"
+}
+
+# SourceRanges can be overriden using tfvars file
+variable "sourceRanges" {
+  default = "0.0.0.0/0"
+}
+
 variable "firewallName" {
   description = "name for the cluster firewall. Defaults to 'game-server-firewall-{local.name}' if not set."
   type        = string