Merge pull request terraform-google-modules#225 from kopachevsky/fix/…

…27/dynamic-sa Bugfix: Can not use dynamic Service Account terraform-google-modules#27
WALTER-GROUP · Aug 14, 2019 · b31c733 · b31c733
2 parents fd4839c + 27e5577
commit b31c733
Show file tree

Hide file tree

Showing 34 changed files with 165 additions and 113 deletions.
diff --git a/Makefile b/Makefile
@@ -18,7 +18,7 @@ SHELL := /usr/bin/env bash
 # Docker build config variables
 CREDENTIALS_PATH ?= /cft/workdir/credentials.json
 DOCKER_ORG := gcr.io/cloud-foundation-cicd
-DOCKER_TAG_BASE_KITCHEN_TERRAFORM ?= 2.1.0
+DOCKER_TAG_BASE_KITCHEN_TERRAFORM ?= 2.3.0
 DOCKER_REPO_BASE_KITCHEN_TERRAFORM := ${DOCKER_ORG}/cft/kitchen-terraform:${DOCKER_TAG_BASE_KITCHEN_TERRAFORM}
 
 # All is the first target in the file so it will get picked up when you just run 'make' on its own

diff --git a/README.md b/README.md
@@ -134,6 +134,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | cluster\_ipv4\_cidr | The IP address range of the kubernetes pods in this cluster. Default is an automatically assigned CIDR. | string | `""` | no |
 | cluster\_resource\_labels | The GCE resource labels (a map of key/value pairs) to be applied to the cluster | map(string) | `<map>` | no |
 | configure\_ip\_masq | Enables the installation of ip masquerading, which is usually no longer required when using aliasied IP addresses. IP masquerading uses a kubectl call, so when you have a private cluster, you will need access to the API server. | string | `"false"` | no |
+| create\_service\_account | Defines if service account specified to run nodes should be created. | bool | `"true"` | no |
 | description | The description of the cluster | string | `""` | no |
 | disable\_legacy\_metadata\_endpoints | Disable the /0.1/ and /v1beta1/ metadata server endpoints on the node. Changing this value will cause all node pools to be recreated. | bool | `"true"` | no |
 | horizontal\_pod\_autoscaling | Enable horizontal pod autoscaling addon | bool | `"true"` | no |
@@ -167,7 +168,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | region | The region to host the cluster in (required) | string | n/a | yes |
 | regional | Whether is a regional cluster (zonal cluster if set false. WARNING: changing this after cluster creation is destructive!) | bool | `"true"` | no |
 | remove\_default\_node\_pool | Remove default node pool while setting up the cluster | bool | `"false"` | no |
-| service\_account | The service account to run nodes as if not overridden in `node_pools`. The default value will cause a cluster-specific service account to be created. | string | `"create"` | no |
+| service\_account | The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created. | string | `""` | no |
 | stub\_domains | Map of stub domains and their resolvers to forward DNS queries for a certain domain to an external DNS server | map(list(string)) | `<map>` | no |
 | subnetwork | The subnetwork to host the cluster in (required) | string | n/a | yes |
 | upstream\_nameservers | If specified, the values replace the nameservers taken by default from the node’s /etc/resolv.conf | list | `<list>` | no |

diff --git a/autogen/outputs.tf b/autogen/outputs.tf
@@ -45,7 +45,7 @@ output "endpoint" {
   sensitive   = true
   description = "Cluster endpoint"
   value       = local.cluster_endpoint
-  depends_on  = [
+  depends_on = [
     /* Nominally, the endpoint is populated as soon as it is known to Terraform.
     * However, the cluster may not be in a usable state yet.  Therefore any
     * resources dependent on the cluster being up will fail to deploy.  With

diff --git a/autogen/sa.tf b/autogen/sa.tf
@@ -23,7 +23,8 @@ locals {
       ["dummy"],
     ),
   )
-  service_account = var.service_account == "create" ? element(local.service_account_list, 0) : var.service_account
+  // if user set var.service_accont it will be used even if var.create_service_account==true, so service account will be created but not used
+  service_account = (var.service_account == "" || var.service_account == "create") && var.create_service_account ? element(local.service_account_list, 0) : var.service_account
 }
 
 resource "random_string" "cluster_service_account_suffix" {
@@ -34,28 +35,28 @@ resource "random_string" "cluster_service_account_suffix" {
 }
 
 resource "google_service_account" "cluster_service_account" {
-  count        = var.service_account == "create" ? 1 : 0
+  count        = var.create_service_account ? 1 : 0
   project      = var.project_id
   account_id   = "tf-gke-${substr(var.name, 0, min(15, length(var.name)))}-${random_string.cluster_service_account_suffix.result}"
   display_name = "Terraform-managed service account for cluster ${var.name}"
 }
 
 resource "google_project_iam_member" "cluster_service_account-log_writer" {
-  count   = var.service_account == "create" ? 1 : 0
+  count   = var.create_service_account ? 1 : 0
   project = google_service_account.cluster_service_account[0].project
   role    = "roles/logging.logWriter"
   member  = "serviceAccount:${google_service_account.cluster_service_account[0].email}"
 }
 
 resource "google_project_iam_member" "cluster_service_account-metric_writer" {
-  count   = var.service_account == "create" ? 1 : 0
+  count   = var.create_service_account ? 1 : 0
   project = google_project_iam_member.cluster_service_account-log_writer[0].project
   role    = "roles/monitoring.metricWriter"
   member  = "serviceAccount:${google_service_account.cluster_service_account[0].email}"
 }
 
 resource "google_project_iam_member" "cluster_service_account-monitoring_viewer" {
-  count   = var.service_account == "create" ? 1 : 0
+  count   = var.create_service_account ? 1 : 0
   project = google_project_iam_member.cluster_service_account-metric_writer[0].project
   role    = "roles/monitoring.viewer"
   member  = "serviceAccount:${google_service_account.cluster_service_account[0].email}"

diff --git a/autogen/variables.tf b/autogen/variables.tf
@@ -255,10 +255,16 @@ variable "monitoring_service" {
   default     = "monitoring.googleapis.com"
 }
 
+variable "create_service_account" {
+  type        = bool
+  description = "Defines if service account specified to run nodes should be created."
+  default     = true
+}
+
 variable "service_account" {
   type        = string
-  description = "The service account to run nodes as if not overridden in `node_pools`. The default value will cause a cluster-specific service account to be created."
-  default     = "create"
+  description = "The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created."
+  default     = ""
 }
 
 variable "basic_auth_username" {

diff --git a/examples/deploy_service/main.tf b/examples/deploy_service/main.tf
@@ -46,9 +46,10 @@ module "gke" {
   network    = var.network
   subnetwork = var.subnetwork
 
-  ip_range_pods     = var.ip_range_pods
-  ip_range_services = var.ip_range_services
-  service_account   = var.compute_engine_service_account
+  ip_range_pods          = var.ip_range_pods
+  ip_range_services      = var.ip_range_services
+  create_service_account = false
+  service_account        = var.compute_engine_service_account
 }
 
 resource "kubernetes_pod" "nginx-example" {

diff --git a/examples/disable_client_cert/main.tf b/examples/disable_client_cert/main.tf
@@ -39,6 +39,7 @@ module "gke" {
   subnetwork               = var.subnetwork
   ip_range_pods            = var.ip_range_pods
   ip_range_services        = var.ip_range_services
+  create_service_account   = false
   service_account          = var.compute_engine_service_account
   issue_client_certificate = false
 }

diff --git a/examples/node_pool/main.tf b/examples/node_pool/main.tf
@@ -39,6 +39,7 @@ module "gke" {
   subnetwork                        = var.subnetwork
   ip_range_pods                     = var.ip_range_pods
   ip_range_services                 = var.ip_range_services
+  create_service_account            = false
   remove_default_node_pool          = true
   disable_legacy_metadata_endpoints = false
 

diff --git a/examples/shared_vpc/main.tf b/examples/shared_vpc/main.tf
@@ -29,16 +29,17 @@ provider "google-beta" {
 }
 
 module "gke" {
-  source             = "../../"
-  project_id         = var.project_id
-  name               = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
-  region             = var.region
-  network            = var.network
-  network_project_id = var.network_project_id
-  subnetwork         = var.subnetwork
-  ip_range_pods      = var.ip_range_pods
-  ip_range_services  = var.ip_range_services
-  service_account    = var.compute_engine_service_account
+  source                 = "../../"
+  project_id             = var.project_id
+  name                   = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
+  region                 = var.region
+  network                = var.network
+  network_project_id     = var.network_project_id
+  subnetwork             = var.subnetwork
+  ip_range_pods          = var.ip_range_pods
+  ip_range_services      = var.ip_range_services
+  create_service_account = false
+  service_account        = var.compute_engine_service_account
 }
 
 data "google_client_config" "default" {

diff --git a/examples/simple_regional/main.tf b/examples/simple_regional/main.tf
@@ -29,16 +29,17 @@ provider "google-beta" {
 }
 
 module "gke" {
-  source            = "../../"
-  project_id        = var.project_id
-  name              = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
-  regional          = true
-  region            = var.region
-  network           = var.network
-  subnetwork        = var.subnetwork
-  ip_range_pods     = var.ip_range_pods
-  ip_range_services = var.ip_range_services
-  service_account   = var.compute_engine_service_account
+  source                 = "../../"
+  project_id             = var.project_id
+  name                   = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
+  regional               = true
+  region                 = var.region
+  network                = var.network
+  subnetwork             = var.subnetwork
+  ip_range_pods          = var.ip_range_pods
+  ip_range_services      = var.ip_range_services
+  create_service_account = false
+  service_account        = var.compute_engine_service_account
 }
 
 data "google_client_config" "default" {

diff --git a/examples/simple_regional_beta/main.tf b/examples/simple_regional_beta/main.tf
@@ -31,18 +31,19 @@ provider "google-beta" {
 }
 
 module "gke" {
-  source            = "../../modules/beta-public-cluster/"
-  project_id        = var.project_id
-  name              = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
-  regional          = true
-  region            = var.region
-  network           = var.network
-  subnetwork        = var.subnetwork
-  ip_range_pods     = var.ip_range_pods
-  ip_range_services = var.ip_range_services
-  service_account   = var.compute_engine_service_account
-  istio             = var.istio
-  cloudrun          = var.cloudrun
+  source                 = "../../modules/beta-public-cluster/"
+  project_id             = var.project_id
+  name                   = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
+  regional               = true
+  region                 = var.region
+  network                = var.network
+  subnetwork             = var.subnetwork
+  ip_range_pods          = var.ip_range_pods
+  ip_range_services      = var.ip_range_services
+  create_service_account = false
+  service_account        = var.compute_engine_service_account
+  istio                  = var.istio
+  cloudrun               = var.cloudrun
 }
 
 data "google_client_config" "default" {

diff --git a/examples/simple_regional_private/main.tf b/examples/simple_regional_private/main.tf
@@ -39,6 +39,7 @@ module "gke" {
   subnetwork              = var.subnetwork
   ip_range_pods           = var.ip_range_pods
   ip_range_services       = var.ip_range_services
+  create_service_account  = false
   service_account         = var.compute_engine_service_account
   enable_private_endpoint = true
   enable_private_nodes    = true

diff --git a/examples/simple_zonal_private/main.tf b/examples/simple_zonal_private/main.tf
@@ -40,6 +40,7 @@ module "gke" {
   subnetwork              = var.subnetwork
   ip_range_pods           = var.ip_range_pods
   ip_range_services       = var.ip_range_services
+  create_service_account  = false
   service_account         = var.compute_engine_service_account
   enable_private_endpoint = true
   enable_private_nodes    = true

diff --git a/examples/stub_domains/main.tf b/examples/stub_domains/main.tf
@@ -29,16 +29,17 @@ provider "google-beta" {
 }
 
 module "gke" {
-  source            = "../../"
-  project_id        = var.project_id
-  name              = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
-  region            = var.region
-  network           = var.network
-  subnetwork        = var.subnetwork
-  ip_range_pods     = var.ip_range_pods
-  ip_range_services = var.ip_range_services
-  network_policy    = true
-  service_account   = var.compute_engine_service_account
+  source                 = "../../"
+  project_id             = var.project_id
+  name                   = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
+  region                 = var.region
+  network                = var.network
+  subnetwork             = var.subnetwork
+  ip_range_pods          = var.ip_range_pods
+  ip_range_services      = var.ip_range_services
+  network_policy         = true
+  service_account        = var.compute_engine_service_account
+  create_service_account = false
 
   configure_ip_masq = true
 

diff --git a/examples/stub_domains_private/main.tf b/examples/stub_domains_private/main.tf
@@ -57,8 +57,9 @@ module "gke" {
 
   master_ipv4_cidr_block = "172.16.0.0/28"
 
-  network_policy  = true
-  service_account = var.compute_engine_service_account
+  network_policy         = true
+  create_service_account = false
+  service_account        = var.compute_engine_service_account
 
   stub_domains = {
     "example.com" = [

diff --git a/examples/stub_domains_upstream_nameservers/main.tf b/examples/stub_domains_upstream_nameservers/main.tf
@@ -29,16 +29,17 @@ provider "google-beta" {
 }
 
 module "gke" {
-  source            = "../../"
-  project_id        = var.project_id
-  name              = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
-  region            = var.region
-  network           = var.network
-  subnetwork        = var.subnetwork
-  ip_range_pods     = var.ip_range_pods
-  ip_range_services = var.ip_range_services
-  network_policy    = true
-  service_account   = var.compute_engine_service_account
+  source                 = "../../"
+  project_id             = var.project_id
+  name                   = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
+  region                 = var.region
+  network                = var.network
+  subnetwork             = var.subnetwork
+  ip_range_pods          = var.ip_range_pods
+  ip_range_services      = var.ip_range_services
+  network_policy         = true
+  create_service_account = false
+  service_account        = var.compute_engine_service_account
 
   configure_ip_masq = true
 

diff --git a/examples/upstream_nameservers/main.tf b/examples/upstream_nameservers/main.tf
@@ -29,16 +29,17 @@ provider "google-beta" {
 }
 
 module "gke" {
-  source            = "../../"
-  project_id        = var.project_id
-  name              = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
-  region            = var.region
-  network           = var.network
-  subnetwork        = var.subnetwork
-  ip_range_pods     = var.ip_range_pods
-  ip_range_services = var.ip_range_services
-  network_policy    = true
-  service_account   = var.compute_engine_service_account
+  source                 = "../../"
+  project_id             = var.project_id
+  name                   = "${local.cluster_type}-cluster${var.cluster_name_suffix}"
+  region                 = var.region
+  network                = var.network
+  subnetwork             = var.subnetwork
+  ip_range_pods          = var.ip_range_pods
+  ip_range_services      = var.ip_range_services
+  network_policy         = true
+  create_service_account = false
+  service_account        = var.compute_engine_service_account
 
   configure_ip_masq    = true
   upstream_nameservers = ["8.8.8.8", "8.8.4.4"]

diff --git a/examples/workload_metadata_config/main.tf b/examples/workload_metadata_config/main.tf
@@ -40,6 +40,7 @@ module "gke" {
   subnetwork              = var.subnetwork
   ip_range_pods           = var.ip_range_pods
   ip_range_services       = var.ip_range_services
+  create_service_account  = false
   service_account         = var.compute_engine_service_account
   enable_private_endpoint = true
   enable_private_nodes    = true

diff --git a/modules/beta-private-cluster/README.md b/modules/beta-private-cluster/README.md
@@ -142,6 +142,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | cluster\_ipv4\_cidr | The IP address range of the kubernetes pods in this cluster. Default is an automatically assigned CIDR. | string | `""` | no |
 | cluster\_resource\_labels | The GCE resource labels (a map of key/value pairs) to be applied to the cluster | map(string) | `<map>` | no |
 | configure\_ip\_masq | Enables the installation of ip masquerading, which is usually no longer required when using aliasied IP addresses. IP masquerading uses a kubectl call, so when you have a private cluster, you will need access to the API server. | string | `"false"` | no |
+| create\_service\_account | Defines if service account specified to run nodes should be created. | 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. | object | `<list>` | no |
 | default\_max\_pods\_per\_node | The maximum number of pods to schedule per node | string | `"110"` | no |
 | deploy\_using\_private\_endpoint | (Beta) A toggle for Terraform and kubectl to connect to the master's internal IP address during deployment. | bool | `"false"` | no |
@@ -187,7 +188,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | region | The region to host the cluster in (required) | string | n/a | yes |
 | regional | Whether is a regional cluster (zonal cluster if set false. WARNING: changing this after cluster creation is destructive!) | bool | `"true"` | no |
 | remove\_default\_node\_pool | Remove default node pool while setting up the cluster | bool | `"false"` | no |
-| service\_account | The service account to run nodes as if not overridden in `node_pools`. The default value will cause a cluster-specific service account to be created. | string | `"create"` | no |
+| service\_account | The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created. | string | `""` | no |
 | stub\_domains | Map of stub domains and their resolvers to forward DNS queries for a certain domain to an external DNS server | map(list(string)) | `<map>` | no |
 | subnetwork | The subnetwork to host the cluster in (required) | string | n/a | yes |
 | upstream\_nameservers | If specified, the values replace the nameservers taken by default from the node’s /etc/resolv.conf | list | `<list>` | no |

diff --git a/modules/beta-private-cluster/outputs.tf b/modules/beta-private-cluster/outputs.tf
@@ -45,7 +45,7 @@ output "endpoint" {
   sensitive   = true
   description = "Cluster endpoint"
   value       = local.cluster_endpoint
-  depends_on  = [
+  depends_on = [
     /* Nominally, the endpoint is populated as soon as it is known to Terraform.
     * However, the cluster may not be in a usable state yet.  Therefore any
     * resources dependent on the cluster being up will fail to deploy.  With