fix service account create (#923)

GoogleCloudPlatform · Oct 27, 2022 · e20de3b · e20de3b
1 parent b021d84
commit e20de3b
Show file tree

Hide file tree

Showing 10 changed files with 125 additions and 64 deletions.
diff --git a/blueprints/gke/binauthz/main.tf b/blueprints/gke/binauthz/main.tf
@@ -99,13 +99,15 @@ module "cluster" {
 }
 
 module "cluster_nodepool" {
-  source          = "../../../modules/gke-nodepool"
-  project_id      = module.project.project_id
-  cluster_name    = module.cluster.name
-  location        = var.zone
-  name            = "nodepool"
-  service_account = {}
-  node_count      = { initial = 3 }
+  source       = "../../../modules/gke-nodepool"
+  project_id   = module.project.project_id
+  cluster_name = module.cluster.name
+  location     = var.zone
+  name         = "nodepool"
+  service_account = {
+    create = true
+  }
+  node_count = { initial = 3 }
 }
 
 module "kms" {

diff --git a/blueprints/gke/multi-cluster-mesh-gke-fleet-api/gke.tf b/blueprints/gke/multi-cluster-mesh-gke-fleet-api/gke.tf
@@ -44,15 +44,17 @@ module "clusters" {
 }
 
 module "cluster_nodepools" {
-  for_each        = var.clusters_config
-  source          = "../../../modules/gke-nodepool"
-  project_id      = module.fleet_project.project_id
-  cluster_name    = module.clusters[each.key].name
-  location        = var.region
-  name            = "nodepool-${each.key}"
-  node_count      = { initial = 1 }
-  service_account = {}
-  tags            = ["${each.key}-node"]
+  for_each     = var.clusters_config
+  source       = "../../../modules/gke-nodepool"
+  project_id   = module.fleet_project.project_id
+  cluster_name = module.clusters[each.key].name
+  location     = var.region
+  name         = "nodepool-${each.key}"
+  node_count   = { initial = 1 }
+  service_account = {
+    create = true
+  }
+  tags = ["${each.key}-node"]
 }
 
 module "hub" {

diff --git a/blueprints/networking/shared-vpc-gke/main.tf b/blueprints/networking/shared-vpc-gke/main.tf
@@ -219,11 +219,13 @@ module "cluster-1" {
 }
 
 module "cluster-1-nodepool-1" {
-  source          = "../../../modules/gke-nodepool"
-  count           = var.cluster_create ? 1 : 0
-  name            = "nodepool-1"
-  project_id      = module.project-svc-gke.project_id
-  location        = module.cluster-1.0.location
-  cluster_name    = module.cluster-1.0.name
-  service_account = {}
+  source       = "../../../modules/gke-nodepool"
+  count        = var.cluster_create ? 1 : 0
+  name         = "nodepool-1"
+  project_id   = module.project-svc-gke.project_id
+  location     = module.cluster-1.0.location
+  cluster_name = module.cluster-1.0.name
+  service_account = {
+    create = true
+  }
 }
diff --git a/modules/gke-hub/README.md b/modules/gke-hub/README.md
@@ -257,7 +257,7 @@ module "cluster_1_nodepool" {
   location        = "europe-west1"
   name            = "nodepool"
   node_count      = { initial = 1 }
-  service_account = {}
+  service_account = { create = true }
   tags            = ["cluster-1-node"]
 }
 
@@ -292,7 +292,7 @@ module "cluster_2_nodepool" {
   location                    = "europe-west4"
   name                        = "nodepool"
   node_count      = { initial = 1 }
-  service_account = {}
+  service_account = { create = true }
   tags            = ["cluster-2-node"]
 }
 

diff --git a/modules/gke-nodepool/README.md b/modules/gke-nodepool/README.md
@@ -21,7 +21,46 @@ module "cluster-1-nodepool-1" {
 
 ### Internally managed service account
 
-To have the module auto-create a service account for the nodes, define the `service_account` variable without setting its `email` attribute. You can then specify service account scopes, or use the default. The service account resource and email (in both plain and IAM formats) are then available in outputs to assign IAM roles from your own code.
+There are three different approaches to defining the nodes service account, all depending on the `service_account` variable where the `create` attribute controls creation of a new service account by this module, and the `email` attribute controls the actual service account to use.
+
+If you create a new service account, its resource and email (in both plain and IAM formats) are then available in outputs to reference it in other modules or resources.
+
+#### GCE default service account
+
+To use the GCE default service account, you can ignore the variable which is equivalent to `{ create = null, email = null }`.
+
+```hcl
+module "cluster-1-nodepool-1" {
+  source          = "./fabric/modules/gke-nodepool"
+  project_id      = "myproject"
+  cluster_name    = "cluster-1"
+  location        = "europe-west1-b"
+  name            = "nodepool-1"
+}
+# tftest modules=1 resources=1
+```
+
+#### Externally defined service account
+
+To use an existing service account, pass in just the `email` attribute.
+
+```hcl
+module "cluster-1-nodepool-1" {
+  source          = "./fabric/modules/gke-nodepool"
+  project_id      = "myproject"
+  cluster_name    = "cluster-1"
+  location        = "europe-west1-b"
+  name            = "nodepool-1"
+  service_account = {
+    email = "[email protected]"
+  }
+}
+# tftest modules=1 resources=1
+```
+
+#### Auto-created service account
+
+To have the module create a service account, set the `create` attribute to `true` and optionally pass the desired account id in `email`.
 
 ```hcl
 module "cluster-1-nodepool-1" {
@@ -30,7 +69,11 @@ module "cluster-1-nodepool-1" {
   cluster_name    = "cluster-1"
   location        = "europe-west1-b"
   name            = "nodepool-1"
-  service_account = {}
+  service_account = {
+    create = true
+    # optional
+    email = "spam-eggs"
+  }
 }
 # tftest modules=1 resources=2
 ```
@@ -53,10 +96,10 @@ module "cluster-1-nodepool-1" {
 | [nodepool_config](variables.tf#L109) | Nodepool-level configuration. | <code title="object&#40;&#123;&#10;  autoscaling &#61; optional&#40;object&#40;&#123;&#10;    location_policy &#61; optional&#40;string&#41;&#10;    max_node_count  &#61; optional&#40;number&#41;&#10;    min_node_count  &#61; optional&#40;number&#41;&#10;    use_total_nodes &#61; optional&#40;bool, false&#41;&#10;  &#125;&#41;&#41;&#10;  management &#61; optional&#40;object&#40;&#123;&#10;    auto_repair  &#61; optional&#40;bool&#41;&#10;    auto_upgrade &#61; optional&#40;bool&#41;&#10;  &#125;&#41;&#41;&#10;  upgrade_settings &#61; optional&#40;object&#40;&#123;&#10;    max_surge       &#61; number&#10;    max_unavailable &#61; number&#10;  &#125;&#41;&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
 | [pod_range](variables.tf#L131) | Pod secondary range configuration. | <code title="object&#40;&#123;&#10;  secondary_pod_range &#61; object&#40;&#123;&#10;    cidr   &#61; optional&#40;string&#41;&#10;    create &#61; optional&#40;bool&#41;&#10;    name   &#61; string&#10;  &#125;&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
 | [reservation_affinity](variables.tf#L148) | Configuration of the desired reservation which instances could take capacity from. | <code title="object&#40;&#123;&#10;  consume_reservation_type &#61; string&#10;  key                      &#61; optional&#40;string&#41;&#10;  values                   &#61; optional&#40;list&#40;string&#41;&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
-| [service_account](variables.tf#L158) | Nodepool service account. If this variable is set to null, the default GCE service account will be used. If set and email is null, a service account will be created. If scopes are null a default will be used. | <code title="object&#40;&#123;&#10;  email        &#61; optional&#40;string&#41;&#10;  oauth_scopes &#61; optional&#40;list&#40;string&#41;&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>null</code> |
-| [sole_tenant_nodegroup](variables.tf#L167) | Sole tenant node group. | <code>string</code> |  | <code>null</code> |
-| [tags](variables.tf#L173) | Network tags applied to nodes. | <code>list&#40;string&#41;</code> |  | <code>null</code> |
-| [taints](variables.tf#L179) | Kubernetes taints applied to all nodes. | <code title="list&#40;object&#40;&#123;&#10;  key    &#61; string&#10;  value  &#61; string&#10;  effect &#61; string&#10;&#125;&#41;&#41;">list&#40;object&#40;&#123;&#8230;&#125;&#41;&#41;</code> |  | <code>null</code> |
+| [service_account](variables.tf#L158) | Nodepool service account. If this variable is set to null, the default GCE service account will be used. If set and email is null, a service account will be created. If scopes are null a default will be used. | <code title="object&#40;&#123;&#10;  create       &#61; optional&#40;bool, false&#41;&#10;  email        &#61; optional&#40;string, null&#41;&#10;  oauth_scopes &#61; optional&#40;list&#40;string&#41;, null&#41;&#10;&#125;&#41;">object&#40;&#123;&#8230;&#125;&#41;</code> |  | <code>&#123;&#125;</code> |
+| [sole_tenant_nodegroup](variables.tf#L169) | Sole tenant node group. | <code>string</code> |  | <code>null</code> |
+| [tags](variables.tf#L175) | Network tags applied to nodes. | <code>list&#40;string&#41;</code> |  | <code>null</code> |
+| [taints](variables.tf#L181) | Kubernetes taints applied to all nodes. | <code title="list&#40;object&#40;&#123;&#10;  key    &#61; string&#10;  value  &#61; string&#10;  effect &#61; string&#10;&#125;&#41;&#41;">list&#40;object&#40;&#123;&#8230;&#125;&#41;&#41;</code> |  | <code>null</code> |
 
 ## Outputs
 

diff --git a/modules/gke-nodepool/main.tf b/modules/gke-nodepool/main.tf
@@ -31,17 +31,14 @@ locals {
   )
   # if no attributes passed for service account, use the GCE default
   # if no email specified, create service account
-  service_account_create = (
-    var.service_account != null && try(var.service_account.email, null) == null
-  )
   service_account_email = (
-    local.service_account_create
+    var.service_account.create
     ? google_service_account.service_account[0].email
-    : try(var.service_account.email, null)
+    : var.service_account.email
   )
   service_account_scopes = (
-    try(var.service_account.scopes, null) != null
-    ? var.service_account.scopes
+    var.service_account.oauth_scopes != null
+    ? var.service_account.oauth_scopes
     : [
       "https://www.googleapis.com/auth/devstorage.read_only",
       "https://www.googleapis.com/auth/logging.write",
@@ -60,9 +57,13 @@ locals {
 }
 
 resource "google_service_account" "service_account" {
-  count        = local.service_account_create ? 1 : 0
-  project      = var.project_id
-  account_id   = "tf-gke-${var.name}"
+  count   = var.service_account.create ? 1 : 0
+  project = var.project_id
+  account_id = (
+    var.service_account.email != null
+    ? split("@", var.service_account.email)[0]
+    : "tf-gke-${var.name}"
+  )
   display_name = "Terraform GKE ${var.cluster_name} ${var.name}."
 }
 

diff --git a/modules/gke-nodepool/variables.tf b/modules/gke-nodepool/variables.tf
@@ -158,10 +158,12 @@ variable "reservation_affinity" {
 variable "service_account" {
   description = "Nodepool service account. If this variable is set to null, the default GCE service account will be used. If set and email is null, a service account will be created. If scopes are null a default will be used."
   type = object({
-    email        = optional(string)
-    oauth_scopes = optional(list(string))
+    create       = optional(bool, false)
+    email        = optional(string, null)
+    oauth_scopes = optional(list(string), null)
   })
-  default = null
+  default  = {}
+  nullable = false
 }
 
 variable "sole_tenant_nodegroup" {

diff --git a/tests/modules/gke_nodepool/fixture/main.tf b/tests/modules/gke_nodepool/fixture/main.tf
@@ -14,22 +14,31 @@
  * limitations under the License.
  */
 
+resource "google_service_account" "test" {
+  project      = "my-project"
+  account_id   = "gke-nodepool-test"
+  display_name = "Test Service Account"
+}
+
 module "test" {
-  source                = "../../../../modules/gke-nodepool"
-  project_id            = "my-project"
-  cluster_name          = "cluster-1"
-  location              = "europe-west1-b"
-  name                  = "nodepool-1"
-  gke_version           = var.gke_version
-  labels                = var.labels
-  max_pods_per_node     = var.max_pods_per_node
-  node_config           = var.node_config
-  node_count            = var.node_count
-  node_locations        = var.node_locations
-  nodepool_config       = var.nodepool_config
-  pod_range             = var.pod_range
-  reservation_affinity  = var.reservation_affinity
-  service_account       = var.service_account
+  source               = "../../../../modules/gke-nodepool"
+  project_id           = "my-project"
+  cluster_name         = "cluster-1"
+  location             = "europe-west1-b"
+  name                 = "nodepool-1"
+  gke_version          = var.gke_version
+  labels               = var.labels
+  max_pods_per_node    = var.max_pods_per_node
+  node_config          = var.node_config
+  node_count           = var.node_count
+  node_locations       = var.node_locations
+  nodepool_config      = var.nodepool_config
+  pod_range            = var.pod_range
+  reservation_affinity = var.reservation_affinity
+  service_account = {
+    create = var.service_account_create
+    email  = google_service_account.test.email
+  }
   sole_tenant_nodegroup = var.sole_tenant_nodegroup
   tags                  = var.tags
   taints                = var.taints

diff --git a/tests/modules/gke_nodepool/fixture/variables.tf b/tests/modules/gke_nodepool/fixture/variables.tf
@@ -65,9 +65,9 @@ variable "reservation_affinity" {
   default = null
 }
 
-variable "service_account" {
-  type    = any
-  default = null
+variable "service_account_create" {
+  type    = bool
+  default = false
 }
 
 variable "sole_tenant_nodegroup" {

diff --git a/tests/modules/gke_nodepool/test_plan.py b/tests/modules/gke_nodepool/test_plan.py
@@ -21,9 +21,9 @@ def test_defaults(plan_runner):
 
 
 def test_service_account(plan_runner):
-  _, resources = plan_runner(service_account='{email="[email protected]"}')
+  _, resources = plan_runner()
   assert len(resources) == 1
-  _, resources = plan_runner(service_account='{}')
+  _, resources = plan_runner(service_account_create='true')
   assert len(resources) == 2
   assert 'google_service_account' in [r['type'] for r in resources]