From b021d84633d6ae798d1daa15dc45a76d793cb403 Mon Sep 17 00:00:00 2001 From: Ayman Farhat <823713+aymanfarhat@users.noreply.github.com> Date: Thu, 27 Oct 2022 16:07:24 +0200 Subject: [PATCH 1/3] Fix formatting for gcloud dataflow job launch command (#924) This fix is to prevent errors similar to "ERROR: (gcloud.dataflow.jobs.run) unrecognized arguments: df-loading@tf-pso-workshop-test.iam.gserviceaccount.com" when copy pasting the example code. Line 8 misses a space between the statement and line break, leading to missing a space between parameters when evaluated by the gcloud command. --- .../data-solutions/gcs-to-bq-with-least-privileges/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/blueprints/data-solutions/gcs-to-bq-with-least-privileges/README.md b/blueprints/data-solutions/gcs-to-bq-with-least-privileges/README.md index 1d3f9397..915ada21 100644 --- a/blueprints/data-solutions/gcs-to-bq-with-least-privileges/README.md +++ b/blueprints/data-solutions/gcs-to-bq-with-least-privileges/README.md @@ -145,13 +145,13 @@ Once this is done, the 3 files necessary to run the Dataflow Job will have been Run the following command to start the dataflow job: - gcloud --impersonate-service-account=orchestrator@$SERVICE_PROJECT_ID.iam.gserviceaccount.com dataflow jobs run test_batch_01 \ + gcloud --impersonate-service-account=orchestrator@$SERVICE_PROJECT_ID.iam.gserviceaccount.com dataflow jobs run test_batch_01 \ --gcs-location gs://dataflow-templates/latest/GCS_Text_to_BigQuery \ --project $SERVICE_PROJECT_ID \ --region europe-west1 \ --disable-public-ips \ --subnetwork https://www.googleapis.com/compute/v1/projects/$SERVICE_PROJECT_ID/regions/europe-west1/subnetworks/subnet \ - --staging-location gs://$PREFIX-df-tmp\ + --staging-location gs://$PREFIX-df-tmp \ --service-account-email df-loading@$SERVICE_PROJECT_ID.iam.gserviceaccount.com \ --parameters \ javascriptTextTransformFunctionName=transform,\ From e20de3b86a5eea42ecbf4452b499e1264444905c Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Thu, 27 Oct 2022 17:12:04 +0200 Subject: [PATCH 2/3] fix service account create (#923) --- blueprints/gke/binauthz/main.tf | 16 +++--- .../multi-cluster-mesh-gke-fleet-api/gke.tf | 20 ++++--- blueprints/networking/shared-vpc-gke/main.tf | 16 +++--- modules/gke-hub/README.md | 4 +- modules/gke-nodepool/README.md | 55 +++++++++++++++++-- modules/gke-nodepool/main.tf | 21 +++---- modules/gke-nodepool/variables.tf | 8 ++- tests/modules/gke_nodepool/fixture/main.tf | 39 ++++++++----- .../modules/gke_nodepool/fixture/variables.tf | 6 +- tests/modules/gke_nodepool/test_plan.py | 4 +- 10 files changed, 125 insertions(+), 64 deletions(-) diff --git a/blueprints/gke/binauthz/main.tf b/blueprints/gke/binauthz/main.tf index 79323943..0c3655e4 100644 --- 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 index 73ab19b1..6c769d92 100644 --- 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 index 9d141acc..59d07d2d 100644 --- 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 index 2573ac9d..1a3c547c 100644 --- 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 index d464656f..4c471c60 100644 --- a/modules/gke-nodepool/README.md +++ b/modules/gke-nodepool/README.md @@ -21,7 +21,13 @@ 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" { @@ -30,7 +36,44 @@ module "cluster-1-nodepool-1" { cluster_name = "cluster-1" location = "europe-west1-b" name = "nodepool-1" - service_account = {} +} +# 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 = "foo-bar@myproject.iam.gserviceaccount.com" + } +} +# 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" { + source = "./fabric/modules/gke-nodepool" + project_id = "myproject" + cluster_name = "cluster-1" + location = "europe-west1-b" + name = "nodepool-1" + 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. | object({…}) | | null | | [pod_range](variables.tf#L131) | Pod secondary range configuration. | object({…}) | | null | | [reservation_affinity](variables.tf#L148) | Configuration of the desired reservation which instances could take capacity from. | object({…}) | | null | -| [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. | object({…}) | | null | -| [sole_tenant_nodegroup](variables.tf#L167) | Sole tenant node group. | string | | null | -| [tags](variables.tf#L173) | Network tags applied to nodes. | list(string) | | null | -| [taints](variables.tf#L179) | Kubernetes taints applied to all nodes. | list(object({…})) | | null | +| [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. | object({…}) | | {} | +| [sole_tenant_nodegroup](variables.tf#L169) | Sole tenant node group. | string | | null | +| [tags](variables.tf#L175) | Network tags applied to nodes. | list(string) | | null | +| [taints](variables.tf#L181) | Kubernetes taints applied to all nodes. | list(object({…})) | | null | ## Outputs diff --git a/modules/gke-nodepool/main.tf b/modules/gke-nodepool/main.tf index 6a3714f0..0c35c8d0 100644 --- 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 index dec5b823..15c8a151 100644 --- 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 index aaa030b9..4ee27482 100644 --- 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 index 420b9eb0..18376ec5 100644 --- 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 index fd63f332..75d1cc14 100644 --- 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="foo@example.org"}') + _, 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] From 3dc7b5dcdf5f080b69f610a75e42eeca51a72c17 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Thu, 27 Oct 2022 18:03:24 +0200 Subject: [PATCH 3/3] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c43d3fda..fb3484be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file. ### BLUEPRINTS +- [[#924](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/924)] Fix formatting for gcloud dataflow job launch command ([aymanfarhat](https://github.com/aymanfarhat)) +- [[#921](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/921)] Align documentation, move glb blueprint ([ludoo](https://github.com/ludoo)) - [[#915](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/915)] TFE OIDC with GCP WIF blueprint added ([averbuks](https://github.com/averbuks)) - [[#899](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/899)] Static routes monitoring metrics added to network dashboard BP ([maunope](https://github.com/maunope)) - [[#909](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/909)] GCS2BQ: Move images and templates in sub-folders ([lcaggio](https://github.com/lcaggio)) @@ -41,6 +43,7 @@ All notable changes to this project will be documented in this file. ### DOCUMENTATION +- [[#921](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/921)] Align documentation, move glb blueprint ([ludoo](https://github.com/ludoo)) - [[#898](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/898)] Update FAST bootstrap README.md ([juliocc](https://github.com/juliocc)) - [[#878](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/878)] chore: update cft and fabric ([bharathkkb](https://github.com/bharathkkb)) - [[#863](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/863)] Fabric vs CFT doc ([ludoo](https://github.com/ludoo)) @@ -68,6 +71,7 @@ All notable changes to this project will be documented in this file. ### MODULES +- [[#923](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/923)] Fix service account creation error in gke nodepool module ([ludoo](https://github.com/ludoo)) - [[#908](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/908)] GKE module: autopilot fixes ([ludoo](https://github.com/ludoo)) - [[#906](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/906)] GKE module: add managed_prometheus to features ([apichick](https://github.com/apichick)) - [[#916](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/pull/916)] Add support for DNS routing policies ([juliocc](https://github.com/juliocc))