feat!: add support for pod_cidr_overprovision_config #2452

DrFaust92 · 2025-09-17T16:55:12Z

No description provided.

apeabody · 2025-09-19T22:45:22Z

/gcbrun

apeabody · 2025-09-20T00:16:37Z

From the TEST CI:

TestSimpleAutopilotPrivate 2025-09-19T23:05:37Z command.go:206:   + service_account = (known after apply)
TestSimpleAutopilotPrivate 2025-09-19T23:05:37Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; 
Error: Missing required argument

  with module.example.module.gke.google_container_cluster.primary,
  on ../../../modules/beta-autopilot-private-cluster/cluster.tf line 306, in resource "google_container_cluster" "primary":
 306:   ip_allocation_policy {

The argument
"ip_allocation_policy.0.pod_cidr_overprovision_config.0.disabled" is
required, but no definition was found.}

DrFaust92 · 2025-09-21T15:42:10Z

moving to draft, will test locally

DrFaust92 · 2025-09-21T15:57:01Z

apeabody should be good and added a keeper for node level as the arg is ForceNew in the provider (ill see if i can open an upstream PR to make it updateable)

apeabody · 2025-09-23T16:11:32Z

/gcbrun

apeabody · 2025-09-26T18:52:13Z

/gcbrun

Signed-off-by: drfaust92 <[email protected]>

apeabody · 2025-09-29T16:35:57Z

/gcbrun

apeabody · 2025-09-29T16:36:16Z

/gemini review

gemini-code-assist

Code Review

This pull request adds support for pod_cidr_overprovision_config at both the cluster and node pool levels across various GKE modules. While the feature addition is valuable, the implementation has a critical flaw in all modules where the cluster-level configuration is applied. The for_each argument in a dynamic block is being used with an object, which is invalid and will cause Terraform to fail. I've suggested replacing the dynamic block with a static one. Additionally, there's a recurring medium-severity issue in how the node-pool level configuration is conditionally applied, making the code fragile. I've provided suggestions to make this more robust. These issues are present in multiple files due to the templated nature of the repository.

gemini-code-assist · 2025-09-29T16:39:58Z

modules/beta-private-cluster/cluster.tf

+    dynamic "pod_cidr_overprovision_config" {
+      for_each = var.pod_cidr_overprovision_config
+      content {
+        disabled = var.pod_cidr_overprovision_config.disabled
+      }
+    }


The for_each in this dynamic block is set to var.pod_cidr_overprovision_config, which is an object. The for_each meta-argument only accepts a map or a set of strings, so this will cause a Terraform error. Since var.pod_cidr_overprovision_config has a default value, this block is always intended to be present. It should be defined as a static block instead of a dynamic one.

pod_cidr_overprovision_config { disabled = var.pod_cidr_overprovision_config.disabled }

@DrFaust92 - We should keep as a dynamic block as it could be set to null, however probably need to pass as a map.

gemini-code-assist · 2025-09-29T16:39:58Z

modules/private-cluster/cluster.tf

+    dynamic "pod_cidr_overprovision_config" {
+      for_each = var.pod_cidr_overprovision_config
+      content {
+        disabled = var.pod_cidr_overprovision_config.disabled
+      }
+    }


The for_each in this dynamic block is set to var.pod_cidr_overprovision_config, which is an object. The for_each meta-argument only accepts a map or a set of strings, so this will cause a Terraform error. Since var.pod_cidr_overprovision_config has a default value, this block is always intended to be present. It should be defined as a static block instead of a dynamic one.

pod_cidr_overprovision_config { disabled = var.pod_cidr_overprovision_config.disabled }

gemini-code-assist · 2025-09-29T16:39:58Z

cluster.tf

+    dynamic "pod_cidr_overprovision_config" {
+      for_each = var.pod_cidr_overprovision_config
+      content {
+        disabled = var.pod_cidr_overprovision_config.disabled
+      }
+    }


The for_each in this dynamic block is set to var.pod_cidr_overprovision_config, which is an object. The for_each meta-argument only accepts a map or a set of strings, so this will cause a Terraform error. Since var.pod_cidr_overprovision_config has a default value, this block is always intended to be present. It should be defined as a static block instead of a dynamic one.

pod_cidr_overprovision_config { disabled = var.pod_cidr_overprovision_config.disabled }

gemini-code-assist · 2025-09-29T16:39:59Z

modules/private-cluster-update-variant/cluster.tf

+    dynamic "pod_cidr_overprovision_config" {
+      for_each = var.pod_cidr_overprovision_config
+      content {
+        disabled = var.pod_cidr_overprovision_config.disabled
+      }
+    }


The for_each in this dynamic block is set to var.pod_cidr_overprovision_config, which is an object. The for_each meta-argument only accepts a map or a set of strings, so this will cause a Terraform error. Since var.pod_cidr_overprovision_config has a default value, this block is always intended to be present. It should be defined as a static block instead of a dynamic one.

pod_cidr_overprovision_config { disabled = var.pod_cidr_overprovision_config.disabled }

gemini-code-assist · 2025-09-29T16:39:59Z

modules/beta-public-cluster/cluster.tf

+    dynamic "pod_cidr_overprovision_config" {
+      for_each = var.pod_cidr_overprovision_config
+      content {
+        disabled = var.pod_cidr_overprovision_config.disabled
+      }
+    }


The for_each in this dynamic block is set to var.pod_cidr_overprovision_config, which is an object. The for_each meta-argument only accepts a map or a set of strings, so this will cause a Terraform error. Since var.pod_cidr_overprovision_config has a default value, this block is always intended to be present. It should be defined as a static block instead of a dynamic one.

pod_cidr_overprovision_config { disabled = var.pod_cidr_overprovision_config.disabled }

gemini-code-assist · 2025-09-29T16:40:01Z

modules/private-cluster-update-variant/cluster.tf

      enable_private_nodes = lookup(network_config.value, "enable_private_nodes", var.enable_private_nodes)

+      dynamic "pod_cidr_overprovision_config" {
+        for_each = lookup(network_config.value, "pod_cidr_overprovision_config", "") != "" ? [1] : []


The condition lookup(..., "") != "" is fragile for checking if an optional attribute is set. It's more robust to check against null. This ensures that the logic is not dependent on how different types are converted to strings.

for_each = lookup(network_config.value, "pod_cidr_overprovision_config", null) != null ? [1] : []

gemini-code-assist · 2025-09-29T16:40:01Z

modules/private-cluster-update-variant/cluster.tf

      enable_private_nodes = lookup(network_config.value, "enable_private_nodes", var.enable_private_nodes)

+      dynamic "pod_cidr_overprovision_config" {
+        for_each = lookup(network_config.value, "pod_cidr_overprovision_config", "") != "" ? [1] : []


The condition lookup(..., "") != "" is fragile for checking if an optional attribute is set. It's more robust to check against null. This ensures that the logic is not dependent on how different types are converted to strings.

for_each = lookup(network_config.value, "pod_cidr_overprovision_config", null) != null ? [1] : []

gemini-code-assist · 2025-09-29T16:40:01Z

autogen/main/cluster.tf.tmpl

      {% endif %}

+      dynamic "pod_cidr_overprovision_config" {
+        for_each = lookup(network_config.value, "pod_cidr_overprovision_config", "") != "" ? [1] : []


The condition lookup(..., "") != "" is fragile for checking if an optional attribute is set. It's more robust to check against null. This ensures that the logic is not dependent on how different types are converted to strings.

for_each = lookup(network_config.value, "pod_cidr_overprovision_config", null) != null ? [1] : []

gemini-code-assist · 2025-09-29T16:40:01Z

modules/private-cluster/cluster.tf

      enable_private_nodes = lookup(network_config.value, "enable_private_nodes", var.enable_private_nodes)

+      dynamic "pod_cidr_overprovision_config" {
+        for_each = lookup(network_config.value, "pod_cidr_overprovision_config", "") != "" ? [1] : []


The condition lookup(..., "") != "" is fragile for checking if an optional attribute is set. It's more robust to check against null. This ensures that the logic is not dependent on how different types are converted to strings.

for_each = lookup(network_config.value, "pod_cidr_overprovision_config", null) != null ? [1] : []

gemini-code-assist · 2025-09-29T16:40:01Z

modules/private-cluster/cluster.tf

      enable_private_nodes = lookup(network_config.value, "enable_private_nodes", var.enable_private_nodes)

+      dynamic "pod_cidr_overprovision_config" {
+        for_each = lookup(network_config.value, "pod_cidr_overprovision_config", "") != "" ? [1] : []


The condition lookup(..., "") != "" is fragile for checking if an optional attribute is set. It's more robust to check against null. This ensures that the logic is not dependent on how different types are converted to strings.

for_each = lookup(network_config.value, "pod_cidr_overprovision_config", null) != null ? [1] : []

apeabody

Thanks for the contribution @DrFaust92!

Two notes, and please add to an example (e.g. node_pool) for test coverage.

DrFaust92 marked this pull request as ready for review September 18, 2025 20:30

DrFaust92 requested review from apeabody, ericyz and a team as code owners September 18, 2025 20:30

DrFaust92 force-pushed the pod-over-provision branch from 94683b9 to 3aaeedc Compare September 19, 2025 20:53

DrFaust92 marked this pull request as draft September 21, 2025 15:42

DrFaust92 marked this pull request as ready for review September 21, 2025 15:55

DrFaust92 changed the title ~~feat: add support for pod_cidr_overprovision_config~~ feat!: add support for pod_cidr_overprovision_config Sep 21, 2025

DrFaust92 added 4 commits September 27, 2025 21:39

add support for pod_cidr_overprovision_config

cb3c136

Signed-off-by: drfaust92 <[email protected]>

check

df23997

Signed-off-by: drfaust92 <[email protected]>

check

ba22224

Signed-off-by: drfaust92 <[email protected]>

rebase

1460744

Signed-off-by: drfaust92 <[email protected]>

DrFaust92 force-pushed the pod-over-provision branch from 9ab7925 to 1460744 Compare September 28, 2025 01:46

gemini-code-assist bot reviewed Sep 29, 2025

View reviewed changes

apeabody requested changes Sep 29, 2025

View reviewed changes

feat!: add support for pod_cidr_overprovision_config #2452

Are you sure you want to change the base?

feat!: add support for pod_cidr_overprovision_config #2452

Uh oh!

Conversation

DrFaust92 commented Sep 17, 2025

Uh oh!

apeabody commented Sep 19, 2025

Uh oh!

apeabody commented Sep 20, 2025

Uh oh!

DrFaust92 commented Sep 21, 2025

Uh oh!

DrFaust92 commented Sep 21, 2025

Uh oh!

apeabody commented Sep 23, 2025

Uh oh!

apeabody commented Sep 26, 2025

Uh oh!

apeabody commented Sep 29, 2025

Uh oh!

apeabody commented Sep 29, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

apeabody Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

apeabody left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!