From cccabcb0eca2a7755908e8d17b40ffba87f4839e Mon Sep 17 00:00:00 2001 From: Josh Lubawy Date: Thu, 17 Oct 2024 08:43:58 -0700 Subject: [PATCH] feat(safer-cluster): add create_service_account variable (#2138) --- autogen/safer-cluster/main.tf.tmpl | 9 +++- autogen/safer-cluster/variables.tf.tmpl | 8 ++- docs/upgrading_to_v34.0.md | 54 +++++++++++++++++++ .../safer-cluster-update-variant/README.md | 3 +- modules/safer-cluster-update-variant/main.tf | 13 +++-- .../safer-cluster-update-variant/variables.tf | 8 ++- modules/safer-cluster/README.md | 3 +- modules/safer-cluster/main.tf | 13 +++-- modules/safer-cluster/variables.tf | 8 ++- 9 files changed, 104 insertions(+), 15 deletions(-) create mode 100644 docs/upgrading_to_v34.0.md diff --git a/autogen/safer-cluster/main.tf.tmpl b/autogen/safer-cluster/main.tf.tmpl index 95a2fc6e2e..3a06a87e3f 100644 --- a/autogen/safer-cluster/main.tf.tmpl +++ b/autogen/safer-cluster/main.tf.tmpl @@ -119,11 +119,16 @@ module "gke" { // All applications should run with an identity defined via Workload Identity anyway. // - Use a service account passed as a parameter to the module, in case the user // wants to maintain control of their service accounts. - create_service_account = var.compute_engine_service_account == "" ? true : false service_account = var.compute_engine_service_account registry_project_ids = var.registry_project_ids grant_registry_access = var.grant_registry_access + // If create_service_account is explicitly set to false we short-circuit the + // compute_engine_service_account check to potentially avoid an error (see variables.tf documentation). + // Otherwise if true (the default), we check if compute_engine_service_account is set for backwards compatability + // before the create_service_account variable was added. + create_service_account = var.create_service_account == false ? var.create_service_account : (var.compute_engine_service_account == "" ? true : false) + issue_client_certificate = false cluster_resource_labels = var.cluster_resource_labels @@ -209,7 +214,7 @@ module "gke" { // Enabling vulnerability and audit for workloads workload_vulnerability_mode = var.workload_vulnerability_mode workload_config_audit_mode = var.workload_config_audit_mode - + // Enabling security posture security_posture_mode = var.security_posture_mode security_posture_vulnerability_mode = var.security_posture_vulnerability_mode diff --git a/autogen/safer-cluster/variables.tf.tmpl b/autogen/safer-cluster/variables.tf.tmpl index eeb1b77ec4..678eaa2a3a 100644 --- a/autogen/safer-cluster/variables.tf.tmpl +++ b/autogen/safer-cluster/variables.tf.tmpl @@ -411,10 +411,16 @@ variable "authenticator_security_group" { variable "compute_engine_service_account" { type = string - description = "Use the given service account for nodes rather than creating a new dedicated service account." + description = "Use the given service account for nodes rather than creating a new dedicated service account. If set then also set var.create_service_account to false to avoid 'value depends on resource attributes that cannot be determined until apply' errors." default = "" } +variable "create_service_account" { + type = bool + description = "Defines if service account specified to run nodes should be created. Explicitly set to false if var.compute_engine_service_account is set to avoid 'value depends on resource attributes that cannot be determined until apply' errors." + default = true +} + variable "enable_shielded_nodes" { type = bool description = "Enable Shielded Nodes features on all nodes in this cluster." diff --git a/docs/upgrading_to_v34.0.md b/docs/upgrading_to_v34.0.md new file mode 100644 index 0000000000..f2f184be0a --- /dev/null +++ b/docs/upgrading_to_v34.0.md @@ -0,0 +1,54 @@ +# Upgrading to v34.0 + +The v34.0 release of _kubernetes-engine_ is a backwards incompatible release. + +### safer-cluster modules: Added create_service_account variable + +This only affects users of the `safer-cluster` modules that have set `var.compute_engine_service_account` to something other than the default `""`. + +A variable `var.create_service_account` was added to the `safer-cluster` modules that when explicitly set to `false` avoids the following error withing the `private-cluster` modules: + +```sh +Error: Invalid count argument + + on .terraform/modules/gke_cluster.gke/modules/beta-private-cluster/sa.tf line 35, in resource "random_string" "cluster_service_account_suffix": + 35: count = var.create_service_account && var.service_account_name == "" ? 1 : 0 + +The "count" value depends on resource attributes that cannot be determined +until apply, so Terraform cannot predict how many instances will be created. +To work around this, use the -target argument to first apply only the +resources that the count depends on. +``` + +This seems to happen if `var.compute_engine_service_account` is passed in, and the externally created service account is being created at the same time, so the name/email is not computed yet: + +```terraform +resource "google_service_account" "cluster_service_account" { + project = var.project_id + account_id = "tf-gke-${var.cluster_name}-${random_string.cluster_service_account_suffix.result}" + display_name = "Terraform-managed service account for cluster ${var.cluster_name}" +} + +module "gke" { + source = "terraform-google-modules/kubernetes-engine/google//modules/safer-cluster" + version = "~> 33.0" + + project_id = var.project_id + name = var.cluster_name + + create_service_account = false + compute_engine_service_account = google_service_account.cluster_service_account.email +} +``` + +By explicitly passing a `var.create_service_account = false` it short circuits the calculations dependent on `var.service_account_name`: + +```terraform +resource "random_string" "cluster_service_account_suffix" { + count = var.create_service_account && var.service_account_name == "" ? 1 : 0 + upper = false + lower = true + special = false + length = 4 +} +``` diff --git a/modules/safer-cluster-update-variant/README.md b/modules/safer-cluster-update-variant/README.md index d2203ddf33..759d11f8b1 100644 --- a/modules/safer-cluster-update-variant/README.md +++ b/modules/safer-cluster-update-variant/README.md @@ -209,8 +209,9 @@ For simplicity, we suggest using `roles/container.admin` and | cluster\_dns\_provider | Which in-cluster DNS provider should be used. PROVIDER\_UNSPECIFIED (default) or PLATFORM\_DEFAULT or CLOUD\_DNS. | `string` | `"PROVIDER_UNSPECIFIED"` | no | | cluster\_dns\_scope | The scope of access to cluster DNS records. DNS\_SCOPE\_UNSPECIFIED (default) or CLUSTER\_SCOPE or VPC\_SCOPE. | `string` | `"DNS_SCOPE_UNSPECIFIED"` | no | | cluster\_resource\_labels | The GCE resource labels (a map of key/value pairs) to be applied to the cluster | `map(string)` | `{}` | no | -| compute\_engine\_service\_account | Use the given service account for nodes rather than creating a new dedicated service account. | `string` | `""` | no | +| compute\_engine\_service\_account | Use the given service account for nodes rather than creating a new dedicated service account. If set then also set var.create\_service\_account to false to avoid 'value depends on resource attributes that cannot be determined until apply' errors. | `string` | `""` | no | | config\_connector | Whether ConfigConnector is enabled for this cluster. | `bool` | `false` | no | +| create\_service\_account | Defines if service account specified to run nodes should be created. Explicitly set to false if var.compute\_engine\_service\_account is set to avoid 'value depends on resource attributes that cannot be determined until apply' errors. | `bool` | `true` | no | | database\_encryption | Application-layer Secrets Encryption settings. The object format is {state = string, key\_name = string}. Valid values of state are: "ENCRYPTED"; "DECRYPTED". key\_name is the name of a CloudKMS key. | `list(object({ state = string, key_name = string }))` | 
[
  {
    "key_name": "",
    "state": "DECRYPTED"
  }
]
| no | | datapath\_provider | The desired datapath provider for this cluster. By default, `ADVANCED_DATAPATH` enables Dataplane-V2 feature. `DATAPATH_PROVIDER_UNSPECIFIED` enables the IPTables-based kube-proxy implementation as a fallback since upgrading to V2 requires a cluster re-creation. | `string` | `"ADVANCED_DATAPATH"` | no | | default\_max\_pods\_per\_node | The maximum number of pods to schedule per node | `number` | `110` | no | diff --git a/modules/safer-cluster-update-variant/main.tf b/modules/safer-cluster-update-variant/main.tf index 85f55f3e7b..a13fafe5fe 100644 --- a/modules/safer-cluster-update-variant/main.tf +++ b/modules/safer-cluster-update-variant/main.tf @@ -115,10 +115,15 @@ module "gke" { // All applications should run with an identity defined via Workload Identity anyway. // - Use a service account passed as a parameter to the module, in case the user // wants to maintain control of their service accounts. - create_service_account = var.compute_engine_service_account == "" ? true : false - service_account = var.compute_engine_service_account - registry_project_ids = var.registry_project_ids - grant_registry_access = var.grant_registry_access + service_account = var.compute_engine_service_account + registry_project_ids = var.registry_project_ids + grant_registry_access = var.grant_registry_access + + // If create_service_account is explicitly set to false we short-circuit the + // compute_engine_service_account check to potentially avoid an error (see variables.tf documentation). + // Otherwise if true (the default), we check if compute_engine_service_account is set for backwards compatability + // before the create_service_account variable was added. + create_service_account = var.create_service_account == false ? var.create_service_account : (var.compute_engine_service_account == "" ? true : false) issue_client_certificate = false diff --git a/modules/safer-cluster-update-variant/variables.tf b/modules/safer-cluster-update-variant/variables.tf index 1934526404..02d6f8e526 100644 --- a/modules/safer-cluster-update-variant/variables.tf +++ b/modules/safer-cluster-update-variant/variables.tf @@ -411,10 +411,16 @@ variable "authenticator_security_group" { variable "compute_engine_service_account" { type = string - description = "Use the given service account for nodes rather than creating a new dedicated service account." + description = "Use the given service account for nodes rather than creating a new dedicated service account. If set then also set var.create_service_account to false to avoid 'value depends on resource attributes that cannot be determined until apply' errors." default = "" } +variable "create_service_account" { + type = bool + description = "Defines if service account specified to run nodes should be created. Explicitly set to false if var.compute_engine_service_account is set to avoid 'value depends on resource attributes that cannot be determined until apply' errors." + default = true +} + variable "enable_shielded_nodes" { type = bool description = "Enable Shielded Nodes features on all nodes in this cluster." diff --git a/modules/safer-cluster/README.md b/modules/safer-cluster/README.md index d2203ddf33..759d11f8b1 100644 --- a/modules/safer-cluster/README.md +++ b/modules/safer-cluster/README.md @@ -209,8 +209,9 @@ For simplicity, we suggest using `roles/container.admin` and | cluster\_dns\_provider | Which in-cluster DNS provider should be used. PROVIDER\_UNSPECIFIED (default) or PLATFORM\_DEFAULT or CLOUD\_DNS. | `string` | `"PROVIDER_UNSPECIFIED"` | no | | cluster\_dns\_scope | The scope of access to cluster DNS records. DNS\_SCOPE\_UNSPECIFIED (default) or CLUSTER\_SCOPE or VPC\_SCOPE. | `string` | `"DNS_SCOPE_UNSPECIFIED"` | no | | cluster\_resource\_labels | The GCE resource labels (a map of key/value pairs) to be applied to the cluster | `map(string)` | `{}` | no | -| compute\_engine\_service\_account | Use the given service account for nodes rather than creating a new dedicated service account. | `string` | `""` | no | +| compute\_engine\_service\_account | Use the given service account for nodes rather than creating a new dedicated service account. If set then also set var.create\_service\_account to false to avoid 'value depends on resource attributes that cannot be determined until apply' errors. | `string` | `""` | no | | config\_connector | Whether ConfigConnector is enabled for this cluster. | `bool` | `false` | no | +| create\_service\_account | Defines if service account specified to run nodes should be created. Explicitly set to false if var.compute\_engine\_service\_account is set to avoid 'value depends on resource attributes that cannot be determined until apply' errors. | `bool` | `true` | no | | database\_encryption | Application-layer Secrets Encryption settings. The object format is {state = string, key\_name = string}. Valid values of state are: "ENCRYPTED"; "DECRYPTED". key\_name is the name of a CloudKMS key. | `list(object({ state = string, key_name = string }))` | 
[
  {
    "key_name": "",
    "state": "DECRYPTED"
  }
]
| no | | datapath\_provider | The desired datapath provider for this cluster. By default, `ADVANCED_DATAPATH` enables Dataplane-V2 feature. `DATAPATH_PROVIDER_UNSPECIFIED` enables the IPTables-based kube-proxy implementation as a fallback since upgrading to V2 requires a cluster re-creation. | `string` | `"ADVANCED_DATAPATH"` | no | | default\_max\_pods\_per\_node | The maximum number of pods to schedule per node | `number` | `110` | no | diff --git a/modules/safer-cluster/main.tf b/modules/safer-cluster/main.tf index 3c67db4830..e113c09a6a 100644 --- a/modules/safer-cluster/main.tf +++ b/modules/safer-cluster/main.tf @@ -115,10 +115,15 @@ module "gke" { // All applications should run with an identity defined via Workload Identity anyway. // - Use a service account passed as a parameter to the module, in case the user // wants to maintain control of their service accounts. - create_service_account = var.compute_engine_service_account == "" ? true : false - service_account = var.compute_engine_service_account - registry_project_ids = var.registry_project_ids - grant_registry_access = var.grant_registry_access + service_account = var.compute_engine_service_account + registry_project_ids = var.registry_project_ids + grant_registry_access = var.grant_registry_access + + // If create_service_account is explicitly set to false we short-circuit the + // compute_engine_service_account check to potentially avoid an error (see variables.tf documentation). + // Otherwise if true (the default), we check if compute_engine_service_account is set for backwards compatability + // before the create_service_account variable was added. + create_service_account = var.create_service_account == false ? var.create_service_account : (var.compute_engine_service_account == "" ? true : false) issue_client_certificate = false diff --git a/modules/safer-cluster/variables.tf b/modules/safer-cluster/variables.tf index 1934526404..02d6f8e526 100644 --- a/modules/safer-cluster/variables.tf +++ b/modules/safer-cluster/variables.tf @@ -411,10 +411,16 @@ variable "authenticator_security_group" { variable "compute_engine_service_account" { type = string - description = "Use the given service account for nodes rather than creating a new dedicated service account." + description = "Use the given service account for nodes rather than creating a new dedicated service account. If set then also set var.create_service_account to false to avoid 'value depends on resource attributes that cannot be determined until apply' errors." default = "" } +variable "create_service_account" { + type = bool + description = "Defines if service account specified to run nodes should be created. Explicitly set to false if var.compute_engine_service_account is set to avoid 'value depends on resource attributes that cannot be determined until apply' errors." + default = true +} + variable "enable_shielded_nodes" { type = bool description = "Enable Shielded Nodes features on all nodes in this cluster."