-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
base: main
Are you sure you want to change the base?
change: ip_range_services to optional value (#1949) #2365
Conversation
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. |
There was a problem hiding this 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.
65c3299
to
0d31c83
Compare
0d31c83
to
fa314cd
Compare
/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 |
There was a problem hiding this comment.
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).
There was a problem hiding this 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." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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." |
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.