Skip to content

change: ip_range_services to optional value (#1949) #2365

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 1 commit into
base: main
Choose a base branch
from

Conversation

martin-kulich-whalebone

Based on #1989 by @0Delta , fixed the comments that were not addressed in the original PR and updated tests.

Fixes #1949.

As of GKE version 1.29 and Autopilot 1.27, the service ip range is given a default of 34.118.224.0/20 per cluster.
Versions earlier than the specified version may be omitted, but will be rejected by the validator.

This change has a big impact, but has not been thoroughly discussed yet.
We hope to merge it after sufficient discussion and making the necessary corrections.

Copy link

google-cla bot commented Jun 11, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @martin-kulich-whalebone!

A few findings from the initial LINT:

terraform_validate ./examples/simple_autopilot_private
╷
│ Error: Reference to undeclared local value
│ 
│   on network.tf line 45, in module "gcp-network":
│   45:         range_name    = local.svc_range_name
│ 
│ A local value with the name "svc_range_name" has not been declared.
╵
terraform_validate ./examples/simple_regional_private
╷
│ Error: Reference to undeclared input variable
│ 
│   on test_outputs.tf line 52, in output "ip_range_services":
│   52:   value       = var.ip_range_services
│ 
│ An input variable with the name "ip_range_services" has not been declared.
│ This variable can be declared with a variable "ip_range_services" {} block.
╵
terraform_validate ./test/fixtures/simple_autopilot_private
╷
│ Error: Reference to undeclared local value
│ 
│   on ../../../examples/simple_autopilot_private/network.tf line 45, in module "gcp-network":
│   45:         range_name    = local.svc_range_name
│ 
│ A local value with the name "svc_range_name" has not been declared.
╵
terraform_validate ./test/fixtures/simple_regional_private
╷
│ Error: Unsupported argument
│ 
│   on example.tf line 26, in module "example":
│   26:   ip_range_services              = google_compute_subnetwork.main.secondary_ip_range[1].range_name
│ 
│ An argument named "ip_range_services" is not expected here.

@apeabody
Copy link
Collaborator

/gcbrun

lifecycle {
precondition {
{% if autopilot_cluster %}
condition = var.ip_range_services == null && var.kubernetes_version != "latest" ? tonumber(split(".", var.kubernetes_version)[0]) >= 1 && tonumber(split(".", var.kubernetes_version)[1]) >= 27 : true
Copy link
Collaborator

@apeabody apeabody Jun 12, 2025

Choose a reason for hiding this comment

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

Can we verify these conditions (using variables) are supported in Terraform v1.3 (currently the minimum required version).

Copy link
Collaborator

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @martin-kulich-whalebone!

Please verify the condition syntax works with Terraform v1.3

precondition {
{% if autopilot_cluster %}
condition = var.ip_range_services == null && var.kubernetes_version != "latest" ? tonumber(split(".", var.kubernetes_version)[0]) >= 1 && tonumber(split(".", var.kubernetes_version)[1]) >= 27 : true
error_message = "The ip_range_services is require for this gke version. Please set ip_range_services or use kubernetes_version 1.27 or upper."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
error_message = "The ip_range_services is require for this gke version. Please set ip_range_services or use kubernetes_version 1.27 or upper."
error_message = "Setting ip_range_services is require for this GKE version. Please set ip_range_services or use kubernetes_version 1.27 or later."

error_message = "The ip_range_services is require for this gke version. Please set ip_range_services or use kubernetes_version 1.27 or upper."
{% else %}
condition = var.ip_range_services == null && var.kubernetes_version != "latest" ? tonumber(split(".", var.kubernetes_version)[0]) >= 1 && tonumber(split(".", var.kubernetes_version)[1]) >= 29 : true
error_message = "The ip_range_services is require for this gke version. Please set ip_range_services or use kubernetes_version 1.29 or upper."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
error_message = "The ip_range_services is require for this gke version. Please set ip_range_services or use kubernetes_version 1.29 or upper."
error_message = "Setting ip_range_services is require for this GKE version. Please set ip_range_services or use kubernetes_version 1.29 or later."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make service range optional
2 participants