Skip to content

fix: Invalid index for google_container_cluster.primary.private_cluster_config[0] #2354

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions autogen/main/main.tf.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ locals {
default_auto_upgrade = var.regional || var.release_channel != "UNSPECIFIED" ? true : false
{% endif %}

cluster_subnet_cidr = var.add_cluster_firewall_rules ? data.google_compute_subnetwork.gke_subnetwork[0].ip_cidr_range : null
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork[0].secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
cluster_subnet_cidr = data.google_compute_subnetwork.gke_subnetwork.ip_cidr_range
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork.secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
{% if autopilot_cluster != true %}
pod_all_ip_ranges = var.add_cluster_firewall_rules ? compact(concat([local.cluster_alias_ranges_cidr[var.ip_range_pods]], [for range in var.additional_ip_range_pods : local.cluster_alias_ranges_cidr[range] if length(range) > 0], [for k, v in merge(local.node_pools, local.windows_node_pools) : local.cluster_alias_ranges_cidr[v.pod_range] if length(lookup(v, "pod_range", "")) > 0])) : []
{% else %}
Expand Down Expand Up @@ -147,7 +147,13 @@ locals {
{% if private_cluster %}
cluster_endpoint = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (var.enable_private_endpoint || var.deploy_using_private_endpoint ? google_container_cluster.primary.private_cluster_config[0].private_endpoint : google_container_cluster.primary.private_cluster_config[0].public_endpoint) : google_container_cluster.primary.endpoint
cluster_peering_name = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? google_container_cluster.primary.private_cluster_config[0].peering_name : null
cluster_endpoint_for_nodes = google_container_cluster.primary.private_cluster_config[0].master_ipv4_cidr_block
cluster_endpoint_for_nodes = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cluster_endpoint_for_nodes is only used in firewall.tf.tmpl resources which are already gated var.add_cluster_firewall_rules, etc. So it might be simpler to remove this local (which is always evaluated, hence the issue), and instead evaluate directly in those (gated) resources?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback :)

Are cluster firewall rules only relevant when enabling private nodes? Because the module allows their creation even without the flag which would also necessitate the same logic.

Before 35.0.0 I noticed that the master_ipv4_cidr_block had a default string value which I guess prevented this since private clusters always had this option specified.

Now it seems that we may need this since there are many possibilities.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firewalls rules can still be created with var.add_cluster_firewall_rules, var.add_master_webhook_firewall_rules, and var.add_shadow_firewall_rules, regardless of the different scenarios which i described in my first comment:

  1. Create a GKE cluster with no private nodes
  2. Create a GKE cluster with private nodes and master_ipv4_cidr_block set
  3. Create a GKE cluster with private nodes and private_endpoint_subnetwork set
  4. Create a GKE cluster with private nodes and no options

This is based on the documentation which states the order of precedence in order to define the control plane network - https://cloud.google.com/kubernetes-engine/docs/how-to/alias-ips#create_a_cluster_and_select_the_control_plane_ip_address_range.

Removing local.cluster_endpoint_for_nodes and evaluating directly inside the firewall resources wouldn't simplify the code as it still requires the same logic in order to determine the correct value for the master ip cidr range when firewall rules are created.

var.private_endpoint_subnetwork != null ?
data.google_compute_subnetwork.private_endpoint_subnetwork[0].ip_cidr_range :
var.master_ipv4_cidr_block != null ?
google_container_cluster.primary.private_cluster_config[0].master_ipv4_cidr_block :
local.cluster_subnet_cidr
) : local.cluster_subnet_cidr
{% else %}
cluster_endpoint = google_container_cluster.primary.endpoint
cluster_endpoint_for_nodes = "${google_container_cluster.primary.endpoint}/32"
Expand Down
12 changes: 11 additions & 1 deletion autogen/main/networks.tf.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,18 @@
data "google_compute_subnetwork" "gke_subnetwork" {
provider = google

count = var.add_cluster_firewall_rules ? 1 : 0
name = var.subnetwork
region = local.region
project = local.network_project_id
}

{% if private_cluster %}
data "google_compute_subnetwork" "private_endpoint_subnetwork" {
provider = google

count = var.private_endpoint_subnetwork != null ? 1 : 0
name = var.private_endpoint_subnetwork
region = local.region
project = local.network_project_id
}
{% endif %}
26 changes: 13 additions & 13 deletions examples/simple_autopilot_private/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ module "gke" {
source = "terraform-google-modules/kubernetes-engine/google//modules/beta-autopilot-private-cluster"
version = "~> 36.0"

project_id = var.project_id
name = "${local.cluster_type}-cluster"
regional = true
region = var.region
network = module.gcp-network.network_name
subnetwork = local.subnet_names[index(module.gcp-network.subnets_names, local.subnet_name)]
ip_range_pods = local.pods_range_name
ip_range_services = local.svc_range_name
release_channel = "REGULAR"
enable_vertical_pod_autoscaling = true
enable_private_endpoint = true
enable_private_nodes = true
network_tags = [local.cluster_type]
project_id = var.project_id
name = "${local.cluster_type}-cluster"
regional = true
region = var.region
network = module.gcp-network.network_name
subnetwork = local.subnet_names[index(module.gcp-network.subnets_names, local.subnet_name)]
ip_range_pods = local.pods_range_name
ip_range_services = local.svc_range_name
release_channel = "REGULAR"
enable_vertical_pod_autoscaling = true
enable_private_endpoint = true
enable_private_nodes = true
network_tags = [local.cluster_type]
# TODO: b/413643369
# node_pools_cgroup_mode = "CGROUP_MODE_V2"
deletion_protection = false
Expand Down
4 changes: 2 additions & 2 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ locals {
// When a release channel is used, node auto-upgrade is enabled and cannot be disabled.
default_auto_upgrade = var.regional || var.release_channel != "UNSPECIFIED" ? true : false

cluster_subnet_cidr = var.add_cluster_firewall_rules ? data.google_compute_subnetwork.gke_subnetwork[0].ip_cidr_range : null
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork[0].secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
cluster_subnet_cidr = data.google_compute_subnetwork.gke_subnetwork.ip_cidr_range
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork.secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
pod_all_ip_ranges = var.add_cluster_firewall_rules ? compact(concat([local.cluster_alias_ranges_cidr[var.ip_range_pods]], [for range in var.additional_ip_range_pods : local.cluster_alias_ranges_cidr[range] if length(range) > 0], [for k, v in merge(local.node_pools, local.windows_node_pools) : local.cluster_alias_ranges_cidr[v.pod_range] if length(lookup(v, "pod_range", "")) > 0])) : []

cluster_network_policy = var.network_policy ? [{
Expand Down
16 changes: 11 additions & 5 deletions modules/beta-autopilot-private-cluster/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ locals {
zone_count = length(var.zones)
cluster_type = var.regional ? "regional" : "zonal"

cluster_subnet_cidr = var.add_cluster_firewall_rules ? data.google_compute_subnetwork.gke_subnetwork[0].ip_cidr_range : null
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork[0].secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
cluster_subnet_cidr = data.google_compute_subnetwork.gke_subnetwork.ip_cidr_range
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork.secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
pod_all_ip_ranges = var.add_cluster_firewall_rules ? compact(concat([local.cluster_alias_ranges_cidr[var.ip_range_pods]], [for range in var.additional_ip_range_pods : local.cluster_alias_ranges_cidr[range] if length(range) > 0])) : []

gke_backup_agent_config = var.gke_backup_agent_config ? [{ enabled = true }] : [{ enabled = false }]
Expand All @@ -76,9 +76,15 @@ locals {
cluster_output_regional_zones = google_container_cluster.primary.node_locations
cluster_output_zones = local.cluster_output_regional_zones

cluster_endpoint = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (var.enable_private_endpoint || var.deploy_using_private_endpoint ? google_container_cluster.primary.private_cluster_config[0].private_endpoint : google_container_cluster.primary.private_cluster_config[0].public_endpoint) : google_container_cluster.primary.endpoint
cluster_peering_name = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? google_container_cluster.primary.private_cluster_config[0].peering_name : null
cluster_endpoint_for_nodes = google_container_cluster.primary.private_cluster_config[0].master_ipv4_cidr_block
cluster_endpoint = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (var.enable_private_endpoint || var.deploy_using_private_endpoint ? google_container_cluster.primary.private_cluster_config[0].private_endpoint : google_container_cluster.primary.private_cluster_config[0].public_endpoint) : google_container_cluster.primary.endpoint
cluster_peering_name = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? google_container_cluster.primary.private_cluster_config[0].peering_name : null
cluster_endpoint_for_nodes = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (
var.private_endpoint_subnetwork != null ?
data.google_compute_subnetwork.private_endpoint_subnetwork[0].ip_cidr_range :
var.master_ipv4_cidr_block != null ?
google_container_cluster.primary.private_cluster_config[0].master_ipv4_cidr_block :
local.cluster_subnet_cidr
) : local.cluster_subnet_cidr

cluster_output_master_auth = concat(google_container_cluster.primary[*].master_auth, [])
cluster_output_master_version = google_container_cluster.primary.master_version
Expand Down
10 changes: 9 additions & 1 deletion modules/beta-autopilot-private-cluster/networks.tf
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@
data "google_compute_subnetwork" "gke_subnetwork" {
provider = google

count = var.add_cluster_firewall_rules ? 1 : 0
name = var.subnetwork
region = local.region
project = local.network_project_id
}

data "google_compute_subnetwork" "private_endpoint_subnetwork" {
provider = google

count = var.private_endpoint_subnetwork != null ? 1 : 0
name = var.private_endpoint_subnetwork
region = local.region
project = local.network_project_id
}
4 changes: 2 additions & 2 deletions modules/beta-autopilot-public-cluster/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ locals {
zone_count = length(var.zones)
cluster_type = var.regional ? "regional" : "zonal"

cluster_subnet_cidr = var.add_cluster_firewall_rules ? data.google_compute_subnetwork.gke_subnetwork[0].ip_cidr_range : null
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork[0].secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
cluster_subnet_cidr = data.google_compute_subnetwork.gke_subnetwork.ip_cidr_range
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork.secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
pod_all_ip_ranges = var.add_cluster_firewall_rules ? compact(concat([local.cluster_alias_ranges_cidr[var.ip_range_pods]], [for range in var.additional_ip_range_pods : local.cluster_alias_ranges_cidr[range] if length(range) > 0])) : []

gke_backup_agent_config = var.gke_backup_agent_config ? [{ enabled = true }] : [{ enabled = false }]
Expand Down
2 changes: 1 addition & 1 deletion modules/beta-autopilot-public-cluster/networks.tf
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
data "google_compute_subnetwork" "gke_subnetwork" {
provider = google

count = var.add_cluster_firewall_rules ? 1 : 0
name = var.subnetwork
region = local.region
project = local.network_project_id
}

16 changes: 11 additions & 5 deletions modules/beta-private-cluster-update-variant/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ locals {
// When a release channel is used, node auto-upgrade is enabled and cannot be disabled.
default_auto_upgrade = var.regional || var.release_channel != "UNSPECIFIED" ? true : false

cluster_subnet_cidr = var.add_cluster_firewall_rules ? data.google_compute_subnetwork.gke_subnetwork[0].ip_cidr_range : null
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork[0].secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
cluster_subnet_cidr = data.google_compute_subnetwork.gke_subnetwork.ip_cidr_range
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork.secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
pod_all_ip_ranges = var.add_cluster_firewall_rules ? compact(concat([local.cluster_alias_ranges_cidr[var.ip_range_pods]], [for range in var.additional_ip_range_pods : local.cluster_alias_ranges_cidr[range] if length(range) > 0], [for k, v in merge(local.node_pools, local.windows_node_pools) : local.cluster_alias_ranges_cidr[v.pod_range] if length(lookup(v, "pod_range", "")) > 0])) : []

cluster_network_policy = var.network_policy ? [{
Expand Down Expand Up @@ -122,9 +122,15 @@ locals {
cluster_output_regional_zones = google_container_cluster.primary.node_locations
cluster_output_zones = local.cluster_output_regional_zones

cluster_endpoint = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (var.enable_private_endpoint || var.deploy_using_private_endpoint ? google_container_cluster.primary.private_cluster_config[0].private_endpoint : google_container_cluster.primary.private_cluster_config[0].public_endpoint) : google_container_cluster.primary.endpoint
cluster_peering_name = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? google_container_cluster.primary.private_cluster_config[0].peering_name : null
cluster_endpoint_for_nodes = google_container_cluster.primary.private_cluster_config[0].master_ipv4_cidr_block
cluster_endpoint = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (var.enable_private_endpoint || var.deploy_using_private_endpoint ? google_container_cluster.primary.private_cluster_config[0].private_endpoint : google_container_cluster.primary.private_cluster_config[0].public_endpoint) : google_container_cluster.primary.endpoint
cluster_peering_name = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? google_container_cluster.primary.private_cluster_config[0].peering_name : null
cluster_endpoint_for_nodes = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (
var.private_endpoint_subnetwork != null ?
data.google_compute_subnetwork.private_endpoint_subnetwork[0].ip_cidr_range :
var.master_ipv4_cidr_block != null ?
google_container_cluster.primary.private_cluster_config[0].master_ipv4_cidr_block :
local.cluster_subnet_cidr
) : local.cluster_subnet_cidr

cluster_output_master_auth = concat(google_container_cluster.primary[*].master_auth, [])
cluster_output_master_version = google_container_cluster.primary.master_version
Expand Down
10 changes: 9 additions & 1 deletion modules/beta-private-cluster-update-variant/networks.tf
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@
data "google_compute_subnetwork" "gke_subnetwork" {
provider = google

count = var.add_cluster_firewall_rules ? 1 : 0
name = var.subnetwork
region = local.region
project = local.network_project_id
}

data "google_compute_subnetwork" "private_endpoint_subnetwork" {
provider = google

count = var.private_endpoint_subnetwork != null ? 1 : 0
name = var.private_endpoint_subnetwork
region = local.region
project = local.network_project_id
}
16 changes: 11 additions & 5 deletions modules/beta-private-cluster/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ locals {
// When a release channel is used, node auto-upgrade is enabled and cannot be disabled.
default_auto_upgrade = var.regional || var.release_channel != "UNSPECIFIED" ? true : false

cluster_subnet_cidr = var.add_cluster_firewall_rules ? data.google_compute_subnetwork.gke_subnetwork[0].ip_cidr_range : null
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork[0].secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
cluster_subnet_cidr = data.google_compute_subnetwork.gke_subnetwork.ip_cidr_range
cluster_alias_ranges_cidr = var.add_cluster_firewall_rules ? { for range in toset(data.google_compute_subnetwork.gke_subnetwork.secondary_ip_range) : range.range_name => range.ip_cidr_range } : {}
pod_all_ip_ranges = var.add_cluster_firewall_rules ? compact(concat([local.cluster_alias_ranges_cidr[var.ip_range_pods]], [for range in var.additional_ip_range_pods : local.cluster_alias_ranges_cidr[range] if length(range) > 0], [for k, v in merge(local.node_pools, local.windows_node_pools) : local.cluster_alias_ranges_cidr[v.pod_range] if length(lookup(v, "pod_range", "")) > 0])) : []

cluster_network_policy = var.network_policy ? [{
Expand Down Expand Up @@ -122,9 +122,15 @@ locals {
cluster_output_regional_zones = google_container_cluster.primary.node_locations
cluster_output_zones = local.cluster_output_regional_zones

cluster_endpoint = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (var.enable_private_endpoint || var.deploy_using_private_endpoint ? google_container_cluster.primary.private_cluster_config[0].private_endpoint : google_container_cluster.primary.private_cluster_config[0].public_endpoint) : google_container_cluster.primary.endpoint
cluster_peering_name = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? google_container_cluster.primary.private_cluster_config[0].peering_name : null
cluster_endpoint_for_nodes = google_container_cluster.primary.private_cluster_config[0].master_ipv4_cidr_block
cluster_endpoint = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (var.enable_private_endpoint || var.deploy_using_private_endpoint ? google_container_cluster.primary.private_cluster_config[0].private_endpoint : google_container_cluster.primary.private_cluster_config[0].public_endpoint) : google_container_cluster.primary.endpoint
cluster_peering_name = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? google_container_cluster.primary.private_cluster_config[0].peering_name : null
cluster_endpoint_for_nodes = (var.enable_private_nodes && length(google_container_cluster.primary.private_cluster_config) > 0) ? (
var.private_endpoint_subnetwork != null ?
data.google_compute_subnetwork.private_endpoint_subnetwork[0].ip_cidr_range :
var.master_ipv4_cidr_block != null ?
google_container_cluster.primary.private_cluster_config[0].master_ipv4_cidr_block :
local.cluster_subnet_cidr
) : local.cluster_subnet_cidr

cluster_output_master_auth = concat(google_container_cluster.primary[*].master_auth, [])
cluster_output_master_version = google_container_cluster.primary.master_version
Expand Down
10 changes: 9 additions & 1 deletion modules/beta-private-cluster/networks.tf
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@
data "google_compute_subnetwork" "gke_subnetwork" {
provider = google

count = var.add_cluster_firewall_rules ? 1 : 0
name = var.subnetwork
region = local.region
project = local.network_project_id
}

data "google_compute_subnetwork" "private_endpoint_subnetwork" {
provider = google

count = var.private_endpoint_subnetwork != null ? 1 : 0
name = var.private_endpoint_subnetwork
region = local.region
project = local.network_project_id
}
Loading