Skip to content

feat(safer-cluster): add some missing variables #2429

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.

Sign up for GitHub

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
Open

feat(safer-cluster): add some missing variables #2429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 12 additions & 7 deletions autogen/safer-cluster/main.tf.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,16 @@ module "gke" {
// If removing the default node pool, initial_node_count should be at least 1.
initial_node_count = (var.initial_node_count == 0) ? 1 : var.initial_node_count

node_pools = var.node_pools
windows_node_pools = var.windows_node_pools
node_pools_labels = var.node_pools_labels
node_pools_resource_labels = var.node_pools_resource_labels
node_pools_metadata = var.node_pools_metadata
node_pools_taints = var.node_pools_taints
node_pools_tags = var.node_pools_tags
node_pools = var.node_pools
windows_node_pools = var.windows_node_pools
node_pools_labels = var.node_pools_labels
node_pools_resource_labels = var.node_pools_resource_labels
node_pools_resource_manager_tags = var.node_pools_resource_manager_tags
node_pools_metadata = var.node_pools_metadata
node_pools_cgroup_mode = var.node_pools_cgroup_mode
node_pools_hugepage_size_2m = var.node_pools_hugepage_size_2m
node_pools_taints = var.node_pools_taints
node_pools_tags = var.node_pools_tags

node_pools_oauth_scopes = var.node_pools_oauth_scopes

Expand Down Expand Up @@ -216,6 +219,8 @@ module "gke" {

enable_gcfs = var.enable_gcfs

enable_secret_manager_addon = var.enable_secret_manager_addon

// Enabling vulnerability and audit for workloads
workload_vulnerability_mode = var.workload_vulnerability_mode
workload_config_audit_mode = var.workload_config_audit_mode
Expand Down
38 changes: 38 additions & 0 deletions autogen/safer-cluster/variables.tf.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,16 @@ variable "node_pools_resource_labels" {
}
}

variable "node_pools_resource_manager_tags" {
type = map(map(string))
description = "Map of maps containing resource manager tags by node-pool name"

default = {
all = {}
default-node-pool = {}
}
}

variable "node_pools_metadata" {
type = map(map(string))
description = "Map of maps containing node metadata by node-pool name"
Expand All @@ -195,6 +205,28 @@ variable "node_pools_metadata" {
}
}

variable "node_pools_cgroup_mode" {
type = map(string)
description = "Map of strings containing cgroup node config by node-pool name"

# Default is being set in variables_defaults.tf
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

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

medium

This comment is misleading as the default value is defined in the block immediately below. Please remove this line to avoid confusion.

default = {
all = ""
default-node-pool = ""
}
}

variable "node_pools_hugepage_size_2m" {
type = map(string)
description = "Map of strings containing hugepage size 2m node config by node-pool name"

# Default is being set in variables_defaults.tf
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

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

medium

This comment is misleading as the default value is defined in the block immediately below. Please remove this line to avoid confusion.

default = {
all = ""
default-node-pool = ""
}
}

variable "node_pools_taints" {
type = map(list(object({ key = string, value = string, effect = string })))
description = "Map of lists containing node taints by node-pool name"
Expand Down Expand Up @@ -540,6 +572,12 @@ variable "enable_gcfs" {
default = false
}

variable "enable_secret_manager_addon" {
description = "Enable the Secret Manager add-on for this cluster"
type = bool
default = false
}

variable "enable_mesh_certificates" {
type = bool
default = false
Expand Down
3 changes: 3 additions & 0 deletions metadata.display.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,9 @@ spec:
master_authorized_networks:
name: master_authorized_networks
title: Master Authorized Networks
monitoring_auto_monitoring_config_scope:
name: monitoring_auto_monitoring_config_scope
title: Monitoring Auto Monitoring Config Scope
Comment on lines +288 to +290
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

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

medium

The addition of monitoring_auto_monitoring_config_scope seems unrelated to the other changes in this pull request. To keep PRs focused and easier to review, it's best to submit unrelated changes in a separate PR.

monitoring_enable_managed_prometheus:
name: monitoring_enable_managed_prometheus
title: Monitoring Enable Managed Prometheus
Expand Down
3 changes: 3 additions & 0 deletions modules/beta-private-cluster-update-variant/metadata.display.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ spec:
master_ipv4_cidr_block:
name: master_ipv4_cidr_block
title: Master Ipv4 Cidr Block
monitoring_auto_monitoring_config_scope:
name: monitoring_auto_monitoring_config_scope
title: Monitoring Auto Monitoring Config Scope
monitoring_enable_managed_prometheus:
name: monitoring_enable_managed_prometheus
title: Monitoring Enable Managed Prometheus
Expand Down
3 changes: 3 additions & 0 deletions modules/beta-private-cluster/metadata.display.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ spec:
master_ipv4_cidr_block:
name: master_ipv4_cidr_block
title: Master Ipv4 Cidr Block
monitoring_auto_monitoring_config_scope:
name: monitoring_auto_monitoring_config_scope
title: Monitoring Auto Monitoring Config Scope
monitoring_enable_managed_prometheus:
name: monitoring_enable_managed_prometheus
title: Monitoring Enable Managed Prometheus
Expand Down
3 changes: 3 additions & 0 deletions modules/beta-public-cluster-update-variant/metadata.display.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,9 @@ spec:
master_authorized_networks:
name: master_authorized_networks
title: Master Authorized Networks
monitoring_auto_monitoring_config_scope:
name: monitoring_auto_monitoring_config_scope
title: Monitoring Auto Monitoring Config Scope
monitoring_enable_managed_prometheus:
name: monitoring_enable_managed_prometheus
title: Monitoring Enable Managed Prometheus
Expand Down
3 changes: 3 additions & 0 deletions modules/beta-public-cluster/metadata.display.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,9 @@ spec:
master_authorized_networks:
name: master_authorized_networks
title: Master Authorized Networks
monitoring_auto_monitoring_config_scope:
name: monitoring_auto_monitoring_config_scope
title: Monitoring Auto Monitoring Config Scope
monitoring_enable_managed_prometheus:
name: monitoring_enable_managed_prometheus
title: Monitoring Enable Managed Prometheus
Expand Down
3 changes: 3 additions & 0 deletions modules/private-cluster-update-variant/metadata.display.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ spec:
master_ipv4_cidr_block:
name: master_ipv4_cidr_block
title: Master Ipv4 Cidr Block
monitoring_auto_monitoring_config_scope:
name: monitoring_auto_monitoring_config_scope
title: Monitoring Auto Monitoring Config Scope
monitoring_enable_managed_prometheus:
name: monitoring_enable_managed_prometheus
title: Monitoring Enable Managed Prometheus
Expand Down
3 changes: 3 additions & 0 deletions modules/private-cluster/metadata.display.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ spec:
master_ipv4_cidr_block:
name: master_ipv4_cidr_block
title: Master Ipv4 Cidr Block
monitoring_auto_monitoring_config_scope:
name: monitoring_auto_monitoring_config_scope
title: Monitoring Auto Monitoring Config Scope
monitoring_enable_managed_prometheus:
name: monitoring_enable_managed_prometheus
title: Monitoring Enable Managed Prometheus
Expand Down
4 changes: 4 additions & 0 deletions modules/safer-cluster-update-variant/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ For simplicity, we suggest using `roles/container.admin` and
| enable\_mesh\_certificates | Controls the issuance of workload mTLS certificates. When enabled the GKE Workload Identity Certificates controller and node agent will be deployed in the cluster. Requires Workload Identity. | `bool` | `false` | no |
| enable\_pod\_security\_policy | enabled - Enable the PodSecurityPolicy controller for this cluster. If enabled, pods must be valid under a PodSecurityPolicy to be created. | `bool` | `false` | no |
| enable\_private\_endpoint | When true, the cluster's private endpoint is used as the cluster endpoint and access through the public endpoint is disabled. When false, either endpoint can be used. This field only applies to private clusters, when enable\_private\_nodes is true | `bool` | `true` | no |
| enable\_secret\_manager\_addon | Enable the Secret Manager add-on for this cluster | `bool` | `false` | no |
| enable\_shielded\_nodes | Enable Shielded Nodes features on all nodes in this cluster. | `bool` | `true` | no |
| enable\_vertical\_pod\_autoscaling | Vertical Pod Autoscaling automatically adjusts the resources of pods controlled by it | `bool` | `false` | no |
| filestore\_csi\_driver | The status of the Filestore CSI driver addon, which allows the usage of filestore instance as volumes | `bool` | `false` | no |
Expand Down Expand Up @@ -261,10 +262,13 @@ For simplicity, we suggest using `roles/container.admin` and
| network | The VPC network to host the cluster in | `string` | n/a | yes |
| network\_project\_id | The project ID of the shared VPC's host (for shared vpc support) | `string` | `""` | no |
| node\_pools | List of maps containing node pools | `list(map(string))` | <pre>[<br> {<br> "name": "default-node-pool"<br> }<br>]</pre> | no |
| node\_pools\_cgroup\_mode | Map of strings containing cgroup node config by node-pool name | `map(string)` | <pre>{<br> "all": "",<br> "default-node-pool": ""<br>}</pre> | no |
| node\_pools\_hugepage\_size\_2m | Map of strings containing hugepage size 2m node config by node-pool name | `map(string)` | <pre>{<br> "all": "",<br> "default-node-pool": ""<br>}</pre> | no |
| node\_pools\_labels | Map of maps containing node labels by node-pool name | `map(map(string))` | <pre>{<br> "all": {},<br> "default-node-pool": {}<br>}</pre> | no |
| node\_pools\_metadata | Map of maps containing node metadata by node-pool name | `map(map(string))` | <pre>{<br> "all": {},<br> "default-node-pool": {}<br>}</pre> | no |
| node\_pools\_oauth\_scopes | Map of lists containing node oauth scopes by node-pool name | `map(list(string))` | <pre>{<br> "all": [<br> "https://www.googleapis.com/auth/cloud-platform"<br> ],<br> "default-node-pool": []<br>}</pre> | no |
| node\_pools\_resource\_labels | Map of maps containing resource labels by node-pool name | `map(map(string))` | <pre>{<br> "all": {},<br> "default-node-pool": {}<br>}</pre> | no |
| node\_pools\_resource\_manager\_tags | Map of maps containing resource manager tags by node-pool name | `map(map(string))` | <pre>{<br> "all": {},<br> "default-node-pool": {}<br>}</pre> | no |
| node\_pools\_tags | Map of lists containing node network tags by node-pool name | `map(list(string))` | <pre>{<br> "all": [],<br> "default-node-pool": []<br>}</pre> | no |
| node\_pools\_taints | Map of lists containing node taints by node-pool name | `map(list(object({ key = string, value = string, effect = string })))` | <pre>{<br> "all": [],<br> "default-node-pool": []<br>}</pre> | no |
| notification\_config\_topic | The desired Pub/Sub topic to which notifications will be sent by GKE. Format is projects/{project}/topics/{topic}. | `string` | `""` | no |
Expand Down
19 changes: 12 additions & 7 deletions modules/safer-cluster-update-variant/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,16 @@ module "gke" {
// If removing the default node pool, initial_node_count should be at least 1.
initial_node_count = (var.initial_node_count == 0) ? 1 : var.initial_node_count

node_pools = var.node_pools
windows_node_pools = var.windows_node_pools
node_pools_labels = var.node_pools_labels
node_pools_resource_labels = var.node_pools_resource_labels
node_pools_metadata = var.node_pools_metadata
node_pools_taints = var.node_pools_taints
node_pools_tags = var.node_pools_tags
node_pools = var.node_pools
windows_node_pools = var.windows_node_pools
node_pools_labels = var.node_pools_labels
node_pools_resource_labels = var.node_pools_resource_labels
node_pools_resource_manager_tags = var.node_pools_resource_manager_tags
node_pools_metadata = var.node_pools_metadata
node_pools_cgroup_mode = var.node_pools_cgroup_mode
node_pools_hugepage_size_2m = var.node_pools_hugepage_size_2m
node_pools_taints = var.node_pools_taints
node_pools_tags = var.node_pools_tags

node_pools_oauth_scopes = var.node_pools_oauth_scopes

Expand Down Expand Up @@ -212,6 +215,8 @@ module "gke" {

enable_gcfs = var.enable_gcfs

enable_secret_manager_addon = var.enable_secret_manager_addon

// Enabling vulnerability and audit for workloads
workload_vulnerability_mode = var.workload_vulnerability_mode
workload_config_audit_mode = var.workload_config_audit_mode
Expand Down
18 changes: 18 additions & 0 deletions modules/safer-cluster-update-variant/metadata.display.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ spec:
enable_private_endpoint:
name: enable_private_endpoint
title: Enable Private Endpoint
enable_secret_manager_addon:
name: enable_secret_manager_addon
title: Enable Secret Manager Addon
enable_shielded_nodes:
name: enable_shielded_nodes
title: Enable Shielded Nodes
Expand Down Expand Up @@ -208,9 +211,21 @@ spec:
node_pools:
name: node_pools
title: Node Pools
node_pools_cgroup_mode:
name: node_pools_cgroup_mode
title: Node Pools Cgroup Mode
node_pools_hugepage_size_1g:
name: node_pools_hugepage_size_1g
title: Node Pools Hugepage Size 1G
node_pools_hugepage_size_2m:
name: node_pools_hugepage_size_2m
title: Node Pools Hugepage Size 2M
node_pools_labels:
name: node_pools_labels
title: Node Pools Labels
node_pools_linux_node_configs_sysctls:
name: node_pools_linux_node_configs_sysctls
title: Node Pools Linux Node Configs Sysctls
Comment on lines +214 to +228
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

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

high

There are a couple of issues here:

  1. The variables node_pools_hugepage_size_1g and node_pools_linux_node_configs_sysctls are added to this metadata file, but they don't seem to be defined in variables.tf or used in main.tf. This looks like an incomplete implementation. Please either implement them fully or remove them for now.
  2. The variables in this section are not sorted alphabetically. Please sort them to improve readability and maintainability.

node_pools_metadata:
name: node_pools_metadata
title: Node Pools Metadata
Expand All @@ -220,6 +235,9 @@ spec:
node_pools_resource_labels:
name: node_pools_resource_labels
title: Node Pools Resource Labels
node_pools_resource_manager_tags:
name: node_pools_resource_manager_tags
title: Node Pools Resource Manager Tags
node_pools_tags:
name: node_pools_tags
title: Node Pools Tags
Expand Down
22 changes: 22 additions & 0 deletions modules/safer-cluster-update-variant/metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,30 @@ spec:
defaultValue:
all: {}
default-node-pool: {}
- name: node_pools_resource_manager_tags
description: Map of maps containing resource manager tags by node-pool name
varType: map(map(string))
defaultValue:
all: {}
default-node-pool: {}
- name: node_pools_metadata
description: Map of maps containing node metadata by node-pool name
varType: map(map(string))
defaultValue:
all: {}
default-node-pool: {}
- name: node_pools_cgroup_mode
description: Map of strings containing cgroup node config by node-pool name
varType: map(string)
defaultValue:
all: ""
default-node-pool: ""
- name: node_pools_hugepage_size_2m
description: Map of strings containing hugepage size 2m node config by node-pool name
varType: map(string)
defaultValue:
all: ""
default-node-pool: ""
- name: node_pools_taints
description: Map of lists containing node taints by node-pool name
varType: map(list(object({ key = string, value = string, effect = string })))
Expand Down Expand Up @@ -478,6 +496,10 @@ spec:
description: Enable image streaming on cluster level.
varType: bool
defaultValue: false
- name: enable_secret_manager_addon
description: Enable the Secret Manager add-on for this cluster
varType: bool
defaultValue: false
- name: enable_mesh_certificates
description: Controls the issuance of workload mTLS certificates. When enabled the GKE Workload Identity Certificates controller and node agent will be deployed in the cluster. Requires Workload Identity.
varType: bool
Expand Down
38 changes: 38 additions & 0 deletions modules/safer-cluster-update-variant/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,16 @@ variable "node_pools_resource_labels" {
}
}

variable "node_pools_resource_manager_tags" {
type = map(map(string))
description = "Map of maps containing resource manager tags by node-pool name"

default = {
all = {}
default-node-pool = {}
}
}

variable "node_pools_metadata" {
type = map(map(string))
description = "Map of maps containing node metadata by node-pool name"
Expand All @@ -195,6 +205,28 @@ variable "node_pools_metadata" {
}
}

variable "node_pools_cgroup_mode" {
type = map(string)
description = "Map of strings containing cgroup node config by node-pool name"

# Default is being set in variables_defaults.tf
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

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

medium

This comment is misleading as the default value is defined in the block below. Please remove this line.

default = {
all = ""
default-node-pool = ""
}
}

variable "node_pools_hugepage_size_2m" {
type = map(string)
description = "Map of strings containing hugepage size 2m node config by node-pool name"

# Default is being set in variables_defaults.tf
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

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

medium

This comment is misleading as the default value is defined in the block below. Please remove this line.

default = {
all = ""
default-node-pool = ""
}
}

variable "node_pools_taints" {
type = map(list(object({ key = string, value = string, effect = string })))
description = "Map of lists containing node taints by node-pool name"
Expand Down Expand Up @@ -540,6 +572,12 @@ variable "enable_gcfs" {
default = false
}

variable "enable_secret_manager_addon" {
description = "Enable the Secret Manager add-on for this cluster"
type = bool
default = false
}

variable "enable_mesh_certificates" {
type = bool
default = false
Expand Down
Loading