Removed SUBNETWORK_PROJECT (using self links instead)

chris-chilvers-qodea · chris-chilvers-qodea · commit d0ed8bb72aab · 2024-01-19T14:44:07.000Z
Changed the format for shared VPC to use ID or self link instead.
This is because some resources such as Cloud SQL always require self
links, and other resources require self links when using shared VPC.

* Network ID format:
  projects/{{project}}/global/networks/{{name}}

* Subnetwork ID format:
  projects/{{project}}/regions/{{region}}/subnetworks/{{name}}

Simple names such as "default" are still supported. They will be
converted automatically to a network/subnetwork ID. The project will be
assumed to be the same project as the knfsd proxy cluster.

Change-Id: I8cb44038d69654a23f1e3bac6e981f5618abc724
diff --git a/deployment/README.md b/deployment/README.md
@@ -86,9 +86,8 @@ terraform apply
 
 | Variable                            | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        | Required | Default                              |
 | ----------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------- | ------------------------------------ |
-| NETWORK                             | The network name (VPC) to use for the deployment of the Knfsd Compute Engine Instances.                                                                                                                                                                                                                                                                                                                                                                                                                            | False    | `default`                            |
-| SUBNETWORK                          | The subnetwork name (subnet) to use for the deployment of the Knfsd Compute Engine Instances.                                                                                                                                                                                                                                                                                                                                                                                                                      | False    | `default`                            |
-| SUBNETWORK_PROJECT                  | The project that the subnetwork exists in. This only needs to be set if using a Shared VPC, where the subnetwork exists in a different project. Otherwise it defaults to the provider project.                                                                                                                                                                                                                                                                                                                     | False    | null                                 |
+| NETWORK                             | The network id or self link (VPC) to use for the deployment of the Knfsd Compute Engine Instances. A network id is in the format `projects/{{project}}/global/networks/{{name}}`                                                                                                                                                                                                                                                                                                                                   | False    | `default`                            |
+| SUBNETWORK                          | The subnetwork id or self link (subnet) to use for the deployment of the Knfsd Compute Engine Instances. A subnetwork id is in the format `projects/{{project}}/regions/{{region}}/subnetworks/{{name}}`                                                                                                                                                                                                                                                                                                           | False    | `default`                            |
 | AUTO_CREATE_FIREWALL_RULES          | Should firewall rules automatically be created to allow [health check connectivity](https://cloud.google.com/load-balancing/docs/health-check-concepts#ip-ranges)? Best practice is to set this to false and create the firewall rules yourself; see [firewall configuration](../deployment/firewall.md) for more details.                                                                                                                                                                                         | False    | `true`                               |
 | TRAFFIC_DISTRIBUTION_MODE           | The [client traffic distribution mode](./traffic-distribution.md) used to distribute traffic between proxy instances in the KNFSD proxy cluster. Can be either `dns_round_robin`, `loadbalancer`, or `none`. The recommended option is `dns_round_robin`. If using `none` you will need to provide your own solution to handle traffic distribution.                                                                                                                                                               | True     |                                      |
 | LOADBALANCER_IP                     | The IP address to use for the Internal Load Balancer when `TRAFFIC_DISTRIBUTION_MODE = "loadbalancer"`. If not specified, a random IP address will be assigned within the subnet.                                                                                                                                                                                                                                                                                                                                  | False    | null                                 |
diff --git a/deployment/terraform-module-knfsd/compute.tf b/deployment/terraform-module-knfsd/compute.tf
@@ -128,9 +128,8 @@ resource "google_compute_instance_template" "nfsproxy-template" {
   }
 
   network_interface {
-    network            = var.NETWORK
-    subnetwork         = var.SUBNETWORK
-    subnetwork_project = var.SUBNETWORK_PROJECT != "" ? var.SUBNETWORK_PROJECT : null
+    network            = local.network
+    subnetwork         = local.subnetwork
     nic_type           = (var.ENABLE_HIGH_BANDWIDTH_CONFIGURATION || var.ENABLE_GVNIC) ? "GVNIC" : "VIRTIO_NET"
   }
 
@@ -388,7 +387,7 @@ resource "google_compute_firewall" "allow-tcp-healthcheck" {
 
   project  = var.PROJECT
   name     = "allow-nfs-tcp-healthcheck"
-  network  = var.NETWORK
+  network  = local.network
   priority = 1000
 
   allow {
diff --git a/deployment/terraform-module-knfsd/loadbalancer.tf b/deployment/terraform-module-knfsd/loadbalancer.tf
@@ -29,7 +29,7 @@ resource "google_compute_address" "nfsproxy_static" {
   region       = var.REGION
   name         = "${var.PROXY_BASENAME}-static-ip"
   address_type = "INTERNAL"
-  subnetwork   = var.SUBNETWORK
+  subnetwork   = local.subnetwork
   address      = var.LOADBALANCER_IP
   purpose      = "SHARED_LOADBALANCER_VIP"
 
@@ -50,8 +50,8 @@ module "loadbalancer" {
   PROJECT        = var.PROJECT
   REGION         = var.REGION
   PROXY_BASENAME = var.PROXY_BASENAME
-  NETWORK        = var.NETWORK
-  SUBNETWORK     = var.SUBNETWORK
+  NETWORK        = local.network
+  SUBNETWORK     = local.subnetwork
   SERVICE_LABEL  = var.SERVICE_LABEL
   IP_ADDRESS     = google_compute_address.nfsproxy_static[0].address
   ENABLE_UDP     = var.ENABLE_UDP
diff --git a/deployment/terraform-module-knfsd/main.tf b/deployment/terraform-module-knfsd/main.tf
@@ -27,6 +27,11 @@ locals {
   )
   MIG_REPLACEMENT_METHOD_DEFAULT = var.ASSIGN_STATIC_IPS ? "RECREATE" : "SUBSTITUTE"
   deploy_fsid_database           = var.FSID_MODE == "external" && var.FSID_DATABASE_DEPLOY
+
+  # Check if network/subnetwork are using simple names. If so, convert them to
+  # IDs so that they can be used with resources such as Cloud SQL.
+  network    = strcontains(var.NETWORK, "/") ? var.NETWORK : "projects/${var.PROJECT}/global/networks/${var.NETWORK}"
+  subnetwork = strcontains(var.SUBNETWORK, "/") ? var.SUBNETWORK : "projects/${var.PROJECT}/regions/${var.REGION}/subnetworks/${var.SUBNETWORK}"
 }
 
 # Validate that SERVICE_ACCOUNT is set when deploying an external database.
diff --git a/deployment/terraform-module-knfsd/variables.tf b/deployment/terraform-module-knfsd/variables.tf
@@ -53,11 +53,6 @@ variable "PROJECT" {
   }
 }
 
-variable "SUBNETWORK_PROJECT" {
-  type    = string
-  default = ""
-}
-
 variable "REGION" {
   type     = string
   nullable = false
diff --git a/docs/changes/changelog.md b/docs/changes/changelog.md
@@ -5,6 +5,7 @@
 * Fix proxy startup script always prints "Error starting proxy"
 * Fix auto-reexporting the root of an NFS v4 server
 * Assign a public IP to the build machine by default
+* Removed `SUBNETWORK_PROJECT` (using self links instead)
 
 ## Fetch Ubuntu Kernel source from launchpad
 
@@ -31,6 +32,15 @@ When building using packer, assign a public IP (`omit_external_ip = false`) to t
 
 When `omit_external_ip = true` the GCP network will require that Cloud NAT is configured so that the build instance can fetch packages and source code from the public internet.
 
+## Removed `SUBNETWORK_PROJECT` (using self links instead)
+
+Changed the format for shared VPC to use ID or self link instead. This is because some resources such as Cloud SQL always require self links, and other resources require self links when using shared VPC.
+
+* Network ID format: `projects/{{project}}/global/networks/{{name}}`
+* Subnetwork ID format: `projects/{{project}}/regions/{{region}}/subnetworks/{{name}}`
+
+Simple names such as "default" are still supported. They will be converted automatically to a network/subnetwork ID. The project will be assumed to be the same project as the knfsd proxy cluster.
+
 # v1.0.0-beta7
 
 * Fix error applying Terraform when nodes greater than 1
diff --git a/image/smoke-tests/terraform/main.tf b/image/smoke-tests/terraform/main.tf
@@ -49,9 +49,8 @@ module "proxy" {
   REGION  = var.region
   ZONE    = var.zone
 
-  NETWORK            = google_compute_network.this.name
-  SUBNETWORK         = google_compute_subnetwork.this.name
-  SUBNETWORK_PROJECT = google_compute_subnetwork.this.project
+  NETWORK            = google_compute_network.this.id
+  SUBNETWORK         = google_compute_subnetwork.this.id
 
   AUTO_CREATE_FIREWALL_RULES = false
   TRAFFIC_DISTRIBUTION_MODE  = "dns_round_robin"

Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,11 @@ locals {`
`27`	`27`	`)`
`28`	`28`	`MIG_REPLACEMENT_METHOD_DEFAULT = var.ASSIGN_STATIC_IPS ? "RECREATE" : "SUBSTITUTE"`
`29`	`29`	`deploy_fsid_database = var.FSID_MODE == "external" && var.FSID_DATABASE_DEPLOY`
	`30`	`+`
	`31`	`+ # Check if network/subnetwork are using simple names. If so, convert them to`
	`32`	`+ # IDs so that they can be used with resources such as Cloud SQL.`
	`33`	`+ network = strcontains(var.NETWORK, "/") ? var.NETWORK : "projects/${var.PROJECT}/global/networks/${var.NETWORK}"`
	`34`	`+ subnetwork = strcontains(var.SUBNETWORK, "/") ? var.SUBNETWORK : "projects/${var.PROJECT}/regions/${var.REGION}/subnetworks/${var.SUBNETWORK}"`
`30`	`35`	`}`
`31`	`36`
`32`	`37`	`# Validate that SERVICE_ACCOUNT is set when deploying an external database.`
Original file line number	Diff line number	Diff line change
`@@ -53,11 +53,6 @@ variable "PROJECT" {`
`53`	`53`	`}`
`54`	`54`	`}`
`55`	`55`
`56`		`-variable "SUBNETWORK_PROJECT" {`
`57`		`- type = string`
`58`		`- default = ""`
`59`		`-}`
`60`		`-`
`61`	`56`	`variable "REGION" {`
`62`	`57`	`type = string`
`63`	`58`	`nullable = false`