From c392c29c475584b98abfe5af48fb0b9b7b1224d8 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 03:35:00 +0200 Subject: [PATCH] sec(iac/gcp): GKE cluster defaults to private nodes with a closed public endpoint google_container_cluster.main declared no private_cluster_config and no master_authorized_networks_config, so nodes got public IPs and the control plane's public endpoint accepted connections from 0.0.0.0/0. Verified via terraform/environments/gcp/variables.tf that compute_platform defaults to "cloud-run" (GKE is opt-in), and that all three checked-in tfvars use cloud-run, so no currently-deployed environment uses this module; the fix hardens defaults rather than touching a live cluster. Fix (terraform/modules/compute/gcp/gke/{main,variables}.tf): - Added private_cluster_config { enable_private_nodes, enable_private_endpoint }. - Added master_authorized_networks_config, driven by a master_authorized_networks variable that defaults to an empty list and validates against "0.0.0.0/0". Wired the new knobs through terraform/environments/gcp/{variables,compute}.tf as gke_enable_private_nodes / gke_enable_private_endpoint / gke_master_authorized_networks. Review round 2 fixes: - BLOCKER: added master_ipv4_cidr_block. Google provider 5.45.2 (this module's locked version) rejects enable_private_nodes = true on a Standard cluster at CREATE time unless master_ipv4_cidr_block or private_endpoint_subnetwork is set -- terraform validate and plan cannot see this (it is a provider-side check that only runs against live API calls at apply), so the default configuration from round 1 would have failed apply outright despite passing every static check. New var.master_ipv4_cidr_block (module) / gke_master_ipv4_cidr_block (environment) defaults to 172.16.0.0/28, validated to be exactly a /28. Checked terraform/modules/networking/gcp: the VPC's subnet_cidr defaults to 10.0.0.0/24 and connector_subnet_cidr to 10.8.0.0/28, so 172.16.0.0/28 (a distinct RFC1918 /16 outside that 10.0.0.0/8 space) cannot collide with this repo's default networking layout. - MAJOR: the PR's original "empty allowlist denies all external access" claim was false. master_authorized_networks_config's gcp_public_cidrs_access_enabled attribute defaults to true at the provider level (independent of cidr_blocks), so any Google Cloud public IP could still reach the endpoint with an empty allowlist. Added a gcp_public_cidrs_access_enabled variable (module and environment) defaulting to false, wired explicitly into the master_authorized_networks_config block, and corrected the descriptions that made the false claim. - minor: documented the CI-reachability consequence directly in the enable_private_endpoint and master_authorized_networks variable descriptions (module and environment level): a standard GitHub-hosted runner (ubuntu-latest) has no stable, allowlistable egress CIDR, so var.deploy_kubernetes_resources = true from ordinary CI needs a self-hosted/VNet-reachable runner rather than an allowlist entry. Regression test: terraform/modules/compute/gcp/gke_hardening_test.go now also asserts gcp_public_cidrs_access_enabled is wired into master_authorized_networks_config from a variable defaulting to false, and master_ipv4_cidr_block is wired into private_cluster_config from a variable that validates it is a /28. Verified with GOTOOLCHAIN=go1.26.6 that all seven assertions fail against the actual current origin/main source (git show origin/main:.../gke/{main,variables}.tf swapped in, test left in place) and all seven pass with this fix restored. No ci-cd-permissions bootstrap re-apply is required; this only touches the GKE compute module and its (opt-in) environment wiring. Closes #125 Co-Authored-By: claude-flow --- terraform/environments/gcp/compute.tf | 7 ++ terraform/environments/gcp/variables.tf | 41 +++++++++++ terraform/modules/compute/gcp/gke/main.tf | 39 +++++++++++ .../modules/compute/gcp/gke/variables.tf | 68 +++++++++++++++++++ .../modules/compute/gcp/gke_hardening_test.go | 53 +++++++++++++++ 5 files changed, 208 insertions(+) create mode 100644 terraform/modules/compute/gcp/gke_hardening_test.go diff --git a/terraform/environments/gcp/compute.tf b/terraform/environments/gcp/compute.tf index 743144d3c..59847e2d2 100644 --- a/terraform/environments/gcp/compute.tf +++ b/terraform/environments/gcp/compute.tf @@ -155,6 +155,13 @@ module "compute_gke" { enable_auto_upgrade = var.gke_enable_auto_upgrade enable_workload_identity = var.gke_enable_workload_identity + # API server exposure (#125) + enable_private_nodes = var.gke_enable_private_nodes + enable_private_endpoint = var.gke_enable_private_endpoint + master_ipv4_cidr_block = var.gke_master_ipv4_cidr_block + gcp_public_cidrs_access_enabled = var.gke_gcp_public_cidrs_access_enabled + master_authorized_networks = var.gke_master_authorized_networks + # Database connection database_host = module.database.instance_connection_name database_name = module.database.database_name diff --git a/terraform/environments/gcp/variables.tf b/terraform/environments/gcp/variables.tf index 32ce42316..c2ee9e771 100644 --- a/terraform/environments/gcp/variables.tf +++ b/terraform/environments/gcp/variables.tf @@ -401,6 +401,47 @@ variable "gke_enable_workload_identity" { default = true } +variable "gke_enable_private_nodes" { + description = "Give GKE nodes private IPs only. Defaults to true." + type = bool + default = true +} + +variable "gke_enable_private_endpoint" { + description = "Remove the GKE control plane's public IP entirely. Defaults to false; the public endpoint still denies every external IP by default (see gke_master_authorized_networks, gke_gcp_public_cidrs_access_enabled). CONSEQUENCE: with the default empty gke_master_authorized_networks, a standard GitHub-hosted runner (ubuntu-latest) cannot reach the public endpoint -- deploy_kubernetes_resources=true from ordinary CI needs a self-hosted/VNet-reachable runner or a real allowlist entry." + type = bool + default = false +} + +variable "gke_master_ipv4_cidr_block" { + description = <<-EOT + /28 CIDR for the GKE control plane's own VPC peering endpoint. Required + by the google provider whenever GKE nodes are private on a Standard + cluster (always true here) -- terraform validate/plan do not catch the + omission, apply fails outright. Must not overlap subnet_cidr (default + 10.0.0.0/24) or connector_subnet_cidr (default 10.8.0.0/28). Default is + a distinct RFC1918 /16 (172.16.0.0/16) from the 10.0.0.0/8 space those + use. + EOT + type = string + default = "172.16.0.0/28" +} + +variable "gke_gcp_public_cidrs_access_enabled" { + description = "Whether Google Cloud's own public IP ranges can reach the GKE control plane's public endpoint, independent of gke_master_authorized_networks. Defaults to false: the provider's own default (true) would mean an empty allowlist does not actually deny all external access." + type = bool + default = false +} + +variable "gke_master_authorized_networks" { + description = "CIDR allowlist for the GKE control plane's public endpoint. Defaults to empty; combined with gke_gcp_public_cidrs_access_enabled=false (also default), that denies all external access. Must not include 0.0.0.0/0." + type = list(object({ + cidr_block = string + display_name = optional(string, "") + })) + default = [] +} + # ============================================== # Frontend (Load Balancer) Configuration # ============================================== diff --git a/terraform/modules/compute/gcp/gke/main.tf b/terraform/modules/compute/gcp/gke/main.tf index f193944d3..cfb916694 100644 --- a/terraform/modules/compute/gcp/gke/main.tf +++ b/terraform/modules/compute/gcp/gke/main.tf @@ -77,6 +77,45 @@ resource "google_container_cluster" "main" { services_ipv4_cidr_block = "" } + # Private nodes always (no reason for nodes to hold public IPs); the + # control plane's own public endpoint is a separate toggle + # (var.enable_private_endpoint) since fully removing it requires VPC + # peering/a bastion for kubectl access, which this module does not set up. + # Either way, master_authorized_networks_config below denies every public + # IP by default, so a public endpoint is unreachable until an operator + # explicitly allowlists something. + # + # master_ipv4_cidr_block is required by the provider whenever + # enable_private_nodes = true on a Standard cluster and neither this nor + # private_endpoint_subnetwork is set: terraform validate/plan do not catch + # the omission (a provider-side check at create), apply fails outright with + # "master_ipv4_cidr_block or private_endpoint_subnetwork is required". + private_cluster_config { + enable_private_nodes = var.enable_private_nodes + enable_private_endpoint = var.enable_private_endpoint + master_ipv4_cidr_block = var.master_ipv4_cidr_block + } + + # No entries and gcp_public_cidrs_access_enabled = false = the public + # endpoint (when enabled) accepts no external traffic at all; only + # in-VPC/private access works. Google Cloud's own public IP ranges bypass + # cidr_blocks entirely unless gcp_public_cidrs_access_enabled is explicitly + # false -- the provider defaults it to true, which would otherwise let any + # Google Cloud public IP reach the endpoint even with an empty allowlist. + # An operator opts in to public kubectl access by adding CIDRs to + # var.master_authorized_networks (rejects 0.0.0.0/0). + master_authorized_networks_config { + gcp_public_cidrs_access_enabled = var.gcp_public_cidrs_access_enabled + + dynamic "cidr_blocks" { + for_each = var.master_authorized_networks + content { + cidr_block = cidr_blocks.value.cidr_block + display_name = cidr_blocks.value.display_name + } + } + } + # Workload Identity workload_identity_config { workload_pool = var.enable_workload_identity ? "${var.project_id}.svc.id.goog" : null diff --git a/terraform/modules/compute/gcp/gke/variables.tf b/terraform/modules/compute/gcp/gke/variables.tf index 8c3602433..ca0d57a17 100644 --- a/terraform/modules/compute/gcp/gke/variables.tf +++ b/terraform/modules/compute/gcp/gke/variables.tf @@ -289,3 +289,71 @@ variable "scheduled_task_auth_mode_override" { error_message = "scheduled_task_auth_mode_override must be one of: \"oidc\", \"bearer\", \"disabled\"." } } + +variable "enable_private_nodes" { + description = "Give GKE nodes private IPs only (no public node IPs). Defaults to true; there is no legitimate reason to expose node IPs publicly." + type = bool + default = true +} + +variable "enable_private_endpoint" { + description = "Remove the GKE control plane's public IP entirely (kubectl then requires VPC peering, Private Service Connect, or a bastion inside the network). Defaults to false: the public endpoint stays present but is denied to every external IP by default (see var.master_authorized_networks and var.gcp_public_cidrs_access_enabled) rather than reachable from 0.0.0.0/0. CONSEQUENCE either way: with the default empty var.master_authorized_networks, a standard GitHub-hosted runner (ubuntu-latest) cannot reach the public endpoint at all -- ubuntu-latest has no stable, allowlistable egress CIDR. terraform apply itself (creating/updating the cluster) does not need API server reachability, but var.deploy_kubernetes_resources=true (which configures kubernetes/helm providers against this cluster) does, and needs either a self-hosted/VNet-reachable runner or a real entry in var.master_authorized_networks." + type = bool + default = false +} + +variable "master_ipv4_cidr_block" { + description = <<-EOT + /28 CIDR for the GKE control plane's own VPC (used for its private peering + endpoint). Required by the google provider whenever enable_private_nodes = + true on a Standard cluster and neither this nor + private_endpoint_subnetwork is set -- terraform validate and plan do not + catch the omission (it is a provider-side check at apply/create), but + apply fails outright with "master_ipv4_cidr_block or + private_endpoint_subnetwork is required". + + Must not overlap var.network_name / var.subnetwork_name's ranges. The + default is a distinct RFC1918 /16 (172.16.0.0/16) from the 10.0.0.0/8 + space terraform/environments/gcp's subnet_cidr (default 10.0.0.0/24) and + connector_subnet_cidr (default 10.8.0.0/28) use, so it cannot collide + with this repo's default networking layout; an operator using a + non-default VPC CIDR plan must override this to a /28 outside it. + EOT + type = string + default = "172.16.0.0/28" + + validation { + condition = can(cidrnetmask(var.master_ipv4_cidr_block)) && tonumber(split("/", var.master_ipv4_cidr_block)[1]) == 28 + error_message = "master_ipv4_cidr_block must be a valid /28 CIDR (e.g. 172.16.0.0/28); GKE requires exactly a /28 for the control plane's private peering range." + } +} + +variable "gcp_public_cidrs_access_enabled" { + description = <<-EOT + Whether Google Cloud's own public IP ranges (used by services like Cloud + Build) can reach the GKE control plane's public endpoint, independent of + var.master_authorized_networks. Defaults to false: leaving this at the + provider's own default (true) would mean an empty + master_authorized_networks allowlist does NOT actually deny all external + access -- any Google Cloud public IP could still reach the endpoint. Set + to true only if a Google-managed service genuinely needs direct API + server access (most integrations use the GKE API instead and do not need + this). + EOT + type = bool + default = false +} + +variable "master_authorized_networks" { + description = "CIDR allowlist for the GKE control plane's public endpoint (CI egress ranges, an operator bastion). Defaults to empty; combined with gcp_public_cidrs_access_enabled=false (also this module's default), that denies all external access -- only in-VPC/private traffic reaches the API server until an operator explicitly adds a range here. Must not include 0.0.0.0/0. CONSEQUENCE: a standard GitHub-hosted runner (ubuntu-latest) has no stable CIDR to allowlist, so var.deploy_kubernetes_resources=true from ordinary CI needs a self-hosted/VNet-reachable runner rather than an entry here." + type = list(object({ + cidr_block = string + display_name = optional(string, "") + })) + default = [] + + validation { + condition = !contains([for n in var.master_authorized_networks : n.cidr_block], "0.0.0.0/0") + error_message = "master_authorized_networks must not contain 0.0.0.0/0; that reopens the control plane's public endpoint to the whole internet, exactly what this allowlist exists to prevent." + } +} diff --git a/terraform/modules/compute/gcp/gke_hardening_test.go b/terraform/modules/compute/gcp/gke_hardening_test.go new file mode 100644 index 000000000..144d561a8 --- /dev/null +++ b/terraform/modules/compute/gcp/gke_hardening_test.go @@ -0,0 +1,53 @@ +// Regression test for issue #125: google_container_cluster.main declared no +// private_cluster_config and no master_authorized_networks_config, so the +// GKE control plane got a public endpoint accepting connections from +// 0.0.0.0/0 and nodes got public IPs. +package gcp_test + +import ( + "os" + "regexp" + "testing" +) + +func TestGKEClusterIsPrivateWithAuthorizedNetworks(t *testing.T) { + data, err := os.ReadFile("gke/main.tf") + if err != nil { + t.Fatalf("reading gke/main.tf: %v", err) + } + content := string(data) + + if !regexp.MustCompile(`private_cluster_config\s*\{[^}]*enable_private_nodes\s*=\s*var\.enable_private_nodes`).MatchString(content) { + t.Error(`google_container_cluster.main must have a private_cluster_config block with enable_private_nodes wired from a variable (issue #125); nodes were getting public IPs`) + } + + if !regexp.MustCompile(`master_authorized_networks_config\s*\{[^}]*gcp_public_cidrs_access_enabled\s*=\s*var\.gcp_public_cidrs_access_enabled`).MatchString(content) { + t.Error(`google_container_cluster.main's master_authorized_networks_config must set gcp_public_cidrs_access_enabled from a variable (issue #125 round 2); the provider defaults this to true, so an empty cidr_blocks allowlist alone does NOT deny all external access -- any Google Cloud public IP could still reach the endpoint`) + } + + if !regexp.MustCompile(`private_cluster_config\s*\{[^}]*master_ipv4_cidr_block\s*=\s*var\.master_ipv4_cidr_block`).MatchString(content) { + t.Error(`google_container_cluster.main's private_cluster_config must set master_ipv4_cidr_block from a variable (issue #125 round 2); the google provider rejects enable_private_nodes=true on a Standard cluster at apply time (not caught by validate/plan) unless master_ipv4_cidr_block or private_endpoint_subnetwork is set`) + } + + varsData, err := os.ReadFile("gke/variables.tf") + if err != nil { + t.Fatalf("reading gke/variables.tf: %v", err) + } + vars := string(varsData) + + if !regexp.MustCompile(`variable\s+"enable_private_nodes"\s*\{[^}]*default\s*=\s*true`).MatchString(vars) { + t.Error(`var.enable_private_nodes must default to true, so a caller who does not override it gets private nodes`) + } + + if !regexp.MustCompile(`variable\s+"master_authorized_networks"[\s\S]*?contains\(\[for n in var\.master_authorized_networks[^\]]*\],\s*"0\.0\.0\.0/0"\)`).MatchString(vars) { + t.Error(`var.master_authorized_networks must validate against "0.0.0.0/0", so the control plane's public endpoint can't be silently reopened to the whole internet`) + } + + if !regexp.MustCompile(`variable\s+"gcp_public_cidrs_access_enabled"\s*\{[^}]*default\s*=\s*false`).MatchString(vars) { + t.Error(`var.gcp_public_cidrs_access_enabled must default to false, or the empty master_authorized_networks default does not actually deny all external access`) + } + + if !regexp.MustCompile(`variable\s+"master_ipv4_cidr_block"\s*\{[\s\S]*?tonumber\(split\("/",\s*var\.master_ipv4_cidr_block\)\[1\]\)\s*==\s*28`).MatchString(vars) { + t.Error(`var.master_ipv4_cidr_block must validate that its value is a /28 CIDR`) + } +}