From ff8eef6a6f2c5b397b4e40c4b3f36503380660be Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Tue, 22 Aug 2023 09:23:49 +0200 Subject: [PATCH] use cloud run bindings for cf v2 invoker role, refactor iam handling in cf v2 and cloud run (#1609) --- modules/cloud-function-v2/README.md | 19 +++- modules/cloud-function-v2/main.tf | 66 +++++++++----- modules/cloud-function-v2/outputs.tf | 4 +- modules/cloud-run/main.tf | 57 ++++++------ .../cloud_function_v2/examples/iam.yaml | 26 +++++- .../trigger-service-account-external.yaml | 55 +++++++++++- .../examples/trigger-service-account.yaml | 88 +++++++++++++++++-- 7 files changed, 250 insertions(+), 65 deletions(-) diff --git a/modules/cloud-function-v2/README.md b/modules/cloud-function-v2/README.md index ca08b884..fa56fa10 100644 --- a/modules/cloud-function-v2/README.md +++ b/modules/cloud-function-v2/README.md @@ -4,6 +4,21 @@ Cloud Function management, with support for IAM roles and optional bucket creati The GCS object used for deployment uses a hash of the bundle zip contents in its name, which ensures change tracking and avoids recreating the function if the GCS object is deleted and needs recreating. + +- [TODO](#todo) +- [Examples](#examples) + - [HTTP trigger](#http-trigger) + - [PubSub and non-HTTP triggers](#pubsub-and-non-http-triggers) + - [Controlling HTTP access](#controlling-http-access) + - [GCS bucket creation](#gcs-bucket-creation) + - [Service account management](#service-account-management) + - [Custom bundle config](#custom-bundle-config) + - [Private Cloud Build Pool](#private-cloud-build-pool) + - [Multiple Cloud Functions within project](#multiple-cloud-functions-within-project) +- [Variables](#variables) +- [Outputs](#outputs) + + ## TODO - [ ] add support for `source_repository` @@ -67,7 +82,7 @@ as documented [here](https://cloud.google.com/eventarc/docs/roles-permissions#pu ### Controlling HTTP access -To allow anonymous access to the function, grant the `roles/cloudfunctions.invoker` role to the special `allUsers` identifier. Use specific identities (service accounts, groups, etc.) instead of `allUsers` to only allow selective access. +To allow anonymous access to the function, grant the `roles/run.invoker` role to the special `allUsers` identifier. Use specific identities (service accounts, groups, etc.) instead of `allUsers` to only allow selective access. The Cloud Run role needs to be used as explained in the [gcloud documentation](https://cloud.google.com/sdk/gcloud/reference/functions/add-invoker-policy-binding#DESCRIPTION). ```hcl module "cf-http" { @@ -80,7 +95,7 @@ module "cf-http" { output_path = "bundle.zip" } iam = { - "roles/cloudfunctions.invoker" = ["allUsers"] + "roles/run.invoker" = ["allUsers"] } } # tftest modules=1 resources=3 inventory=iam.yaml diff --git a/modules/cloud-function-v2/main.tf b/modules/cloud-function-v2/main.tf index 71979ee3..5b506f29 100644 --- a/modules/cloud-function-v2/main.tf +++ b/modules/cloud-function-v2/main.tf @@ -24,28 +24,17 @@ locals { : null ) ) - _iam_run_invoker_members = concat( - lookup(var.iam, "roles/run.invoker", []), - var.trigger_config == null ? [] : - var.trigger_config.service_account_create ? ["serviceAccount:${local.trigger_service_account_email}"] : [] - ) - iam = merge( - var.iam, - length(local._iam_run_invoker_members) == 0 ? {} : - { - "roles/run.invoker" : local._iam_run_invoker_members - }, - ) prefix = var.prefix == null ? "" : "${var.prefix}-" service_account_email = ( var.service_account_create ? google_service_account.service_account[0].email : var.service_account ) - trigger_service_account_email = ( - try(var.trigger_config.service_account_create, false) - ? google_service_account.trigger_service_account[0].email - : try(var.trigger_config.service_account_email, null) + trigger_sa_create = ( + try(var.trigger_config.service_account_create, false) == true + ) + trigger_sa_email = try( + google_service_account.trigger_service_account[0].email, null ) vpc_connector = ( var.vpc_connector == null @@ -104,7 +93,7 @@ resource "google_cloudfunctions2_function" "function" { operator = event_filter.value.operator } } - service_account_email = local.trigger_service_account_email + service_account_email = local.trigger_sa_email retry_policy = var.trigger_config.retry_policy } } @@ -154,8 +143,10 @@ resource "google_cloudfunctions2_function" "function" { labels = var.labels } -resource "google_cloudfunctions2_function_iam_binding" "default" { - for_each = local.iam +resource "google_cloudfunctions2_function_iam_binding" "binding" { + for_each = { + for k, v in var.iam : k => v if k != "roles/run.invoker" + } project = var.project_id location = google_cloudfunctions2_function.function.location cloud_function = google_cloudfunctions2_function.function.name @@ -163,6 +154,39 @@ resource "google_cloudfunctions2_function_iam_binding" "default" { members = each.value } +resource "google_cloud_run_service_iam_binding" "invoker" { + # cloud run resources are needed for invoker role to the underlying service + count = ( + lookup(var.iam, "roles/run.invoker", null) != null + ) ? 1 : 0 + project = var.project_id + location = google_cloudfunctions2_function.function.location + service = google_cloudfunctions2_function.function.name + role = "roles/run.invoker" + members = distinct(compact(concat( + lookup(var.iam, "roles/run.invoker", []), + ( + !local.trigger_sa_create + ? [] + : ["serviceAccount:${local.trigger_sa_email}"] + ) + ))) +} + +resource "google_cloud_run_service_iam_member" "invoker" { + # if authoritative invoker role is not present and we create trigger sa + # use additive binding to grant it the role + count = ( + lookup(var.iam, "roles/run.invoker", null) == null && + local.trigger_sa_create + ) ? 1 : 0 + project = var.project_id + location = google_cloudfunctions2_function.function.location + service = google_cloudfunctions2_function.function.name + role = "roles/run.invoker" + member = "serviceAccount:${local.trigger_sa_email}" +} + resource "google_storage_bucket" "bucket" { count = var.bucket_config == null ? 0 : 1 project = var.project_id @@ -216,9 +240,7 @@ resource "google_service_account" "service_account" { } resource "google_service_account" "trigger_service_account" { - count = ( - try(var.trigger_config.service_account_create, false) == true ? 1 : 0 - ) + count = local.trigger_sa_create ? 1 : 0 project = var.project_id account_id = "tf-cf-trigger-${var.name}" display_name = "Terraform trigger for Cloud Function ${var.name}." diff --git a/modules/cloud-function-v2/outputs.tf b/modules/cloud-function-v2/outputs.tf index 780f5c1e..4e42a002 100644 --- a/modules/cloud-function-v2/outputs.tf +++ b/modules/cloud-function-v2/outputs.tf @@ -66,14 +66,14 @@ output "trigger_service_account" { output "trigger_service_account_email" { description = "Service account email." - value = local.trigger_service_account_email + value = local.trigger_sa_email } output "trigger_service_account_iam_email" { description = "Service account email." value = join("", [ "serviceAccount:", - local.trigger_service_account_email == null ? "" : local.trigger_service_account_email + local.trigger_sa_email == null ? "" : local.trigger_sa_email ]) } diff --git a/modules/cloud-run/main.tf b/modules/cloud-run/main.tf index 527e1fd1..46f6e3a8 100644 --- a/modules/cloud-run/main.tf +++ b/modules/cloud-run/main.tf @@ -35,17 +35,6 @@ locals { "run.googleapis.com/ingress" = var.ingress_settings }, ) - _iam_run_invoker_members = concat( - lookup(var.iam, "roles/run.invoker", []), - var.eventarc_triggers.service_account_create ? ["serviceAccount:${local.trigger_service_account_email}"] : [] - ) - iam = merge( - var.iam, - length(local._iam_run_invoker_members) == 0 ? {} : - { - "roles/run.invoker" : local._iam_run_invoker_members - }, - ) prefix = var.prefix == null ? "" : "${var.prefix}-" revision_annotations = merge( try(var.revision_annotations.autoscaling, null) == null ? {} : { @@ -90,17 +79,12 @@ locals { ) : var.service_account ) - trigger_service_account_email = ( - var.eventarc_triggers.service_account_create - ? ( - length(google_service_account.trigger_service_account) > 0 - ? google_service_account.trigger_service_account[0].email - # : google_service_account.trigger_service_account[0].email # : null - : null - ) - : var.eventarc_triggers.service_account_email + trigger_sa_create = try( + var.eventarc_triggers.service_account_create, false + ) + trigger_sa_email = try( + google_service_account.trigger_service_account[0].email, null ) - vpc_connector_create = var.vpc_connector_create != null } @@ -317,12 +301,33 @@ resource "google_cloud_run_service" "service" { } resource "google_cloud_run_service_iam_binding" "binding" { - for_each = local.iam + for_each = var.iam project = google_cloud_run_service.service.project location = google_cloud_run_service.service.location service = google_cloud_run_service.service.name role = each.key - members = each.value + members = ( + each.key != "roles/run.invoker" || !local.trigger_sa_create + ? each.value + # if invoker role is present and we create trigger sa, add it as member + : concat( + each.value, ["serviceAccount:${local.trigger_sa_email}"] + ) + ) +} + +resource "google_cloud_run_service_iam_member" "default" { + # if authoritative invoker role is not present and we create trigger sa + # use additive binding to grant it the role + count = ( + lookup(var.iam, "roles/run.invoker", null) == null && + local.trigger_sa_create + ) ? 1 : 0 + project = google_cloud_run_service.service.project + location = google_cloud_run_service.service.location + service = google_cloud_run_service.service.name + role = "roles/run.invoker" + member = "serviceAccount:${local.trigger_sa_email}" } resource "google_service_account" "service_account" { @@ -355,7 +360,7 @@ resource "google_eventarc_trigger" "audit_log_triggers" { region = google_cloud_run_service.service.location } } - service_account = local.trigger_service_account_email + service_account = local.trigger_sa_email } resource "google_eventarc_trigger" "pubsub_triggers" { @@ -378,11 +383,11 @@ resource "google_eventarc_trigger" "pubsub_triggers" { region = google_cloud_run_service.service.location } } - service_account = local.trigger_service_account_email + service_account = local.trigger_sa_email } resource "google_service_account" "trigger_service_account" { - count = var.eventarc_triggers.service_account_create ? 1 : 0 # coalesce(try(var.eventarc_triggers.service_account_create, false), false) ? 1 : 0 + count = local.trigger_sa_create ? 1 : 0 project = var.project_id account_id = "tf-cr-trigger-${var.name}" display_name = "Terraform trigger for Cloud Run ${var.name}." diff --git a/tests/modules/cloud_function_v2/examples/iam.yaml b/tests/modules/cloud_function_v2/examples/iam.yaml index 6353b626..4bbd6653 100644 --- a/tests/modules/cloud_function_v2/examples/iam.yaml +++ b/tests/modules/cloud_function_v2/examples/iam.yaml @@ -13,17 +13,37 @@ # limitations under the License. values: - module.cf-http.google_cloudfunctions2_function_iam_binding.default["roles/cloudfunctions.invoker"]: - cloud_function: test-cf-http + module.cf-http.google_cloud_run_service_iam_binding.invoker[0]: condition: [] location: europe-west1 members: - allUsers project: my-project - role: roles/cloudfunctions.invoker + role: roles/run.invoker + service: test-cf-http + module.cf-http.google_cloudfunctions2_function.function: {} + module.cf-http.google_storage_bucket_object.bundle: + bucket: test-cf-bundles + cache_control: null + content: null + content_disposition: null + content_encoding: null + content_language: null + customer_encryption: [] + detect_md5hash: different hash + event_based_hold: null + metadata: null + name: bundle-6f1ece136848fee658e335b05fe2d79d.zip + source: bundle.zip + temporary_hold: null + timeouts: null counts: + google_cloud_run_service_iam_binding: 1 google_cloudfunctions2_function: 1 google_storage_bucket_object: 1 modules: 1 resources: 3 + +outputs: {} + diff --git a/tests/modules/cloud_run/examples/trigger-service-account-external.yaml b/tests/modules/cloud_run/examples/trigger-service-account-external.yaml index def9c89a..45d15ea4 100644 --- a/tests/modules/cloud_run/examples/trigger-service-account-external.yaml +++ b/tests/modules/cloud_run/examples/trigger-service-account-external.yaml @@ -13,10 +13,61 @@ # limitations under the License. values: - module.cloud_run.google_cloud_run_service.service: {} + module.cloud_run.google_cloud_run_service.service: + autogenerate_revision_name: false + location: europe-west1 + metadata: + - {} + name: hello + project: my-project + template: + - metadata: + - {} + spec: + - containers: + - args: null + command: null + env: [] + env_from: [] + image: us-docker.pkg.dev/cloudrun/container/hello + liveness_probe: [] + volume_mounts: [] + working_dir: null + volumes: [] + timeouts: null module.cloud_run.google_eventarc_trigger.audit_log_triggers["setiampolicy"]: - service_account: cloud-run-trigger@my-project.iam.gserviceaccount.com + channel: null + destination: + - cloud_function: null + cloud_run_service: + - path: null + region: europe-west1 + service: hello + gke: [] + workflow: null + event_data_content_type: null + labels: null + location: europe-west1 + matching_criteria: + - attribute: methodName + operator: '' + value: SetIamPolicy + - attribute: serviceName + operator: '' + value: cloudresourcemanager.googleapis.com + - attribute: type + operator: '' + value: google.cloud.audit.log.v1.written + name: audit-log-setiampolicy + project: my-project + service_account: null + timeouts: null counts: google_cloud_run_service: 1 google_eventarc_trigger: 1 + modules: 1 + resources: 2 + +outputs: {} + diff --git a/tests/modules/cloud_run/examples/trigger-service-account.yaml b/tests/modules/cloud_run/examples/trigger-service-account.yaml index 86b1e1af..92b8fce8 100644 --- a/tests/modules/cloud_run/examples/trigger-service-account.yaml +++ b/tests/modules/cloud_run/examples/trigger-service-account.yaml @@ -13,23 +13,95 @@ # limitations under the License. values: - module.cloud_run.google_cloud_run_service.service: {} - module.cloud_run.google_cloud_run_service_iam_binding.binding["roles/run.invoker"]: + module.cloud_run.google_cloud_run_service.service: + autogenerate_revision_name: false + location: europe-west1 + metadata: + - {} + name: hello + project: my-project + template: + - metadata: + - {} + spec: + - containers: + - args: null + command: null + env: [] + env_from: [] + image: us-docker.pkg.dev/cloudrun/container/hello + liveness_probe: [] + volume_mounts: [] + working_dir: null + volumes: [] + timeouts: null + module.cloud_run.google_cloud_run_service_iam_member.default[0]: + condition: [] + location: europe-west1 project: my-project role: roles/run.invoker service: hello - # members: ["known after apply"] - module.cloud_run.google_eventarc_trigger.pubsub_triggers["topic-1"]: {} - # service_account: known after apply - module.cloud_run.google_eventarc_trigger.pubsub_triggers["topic-2"]: {} - # service_account: known after apply + module.cloud_run.google_eventarc_trigger.pubsub_triggers["topic-1"]: + channel: null + destination: + - cloud_function: null + cloud_run_service: + - path: null + region: europe-west1 + service: hello + gke: [] + workflow: null + event_data_content_type: null + labels: null + location: europe-west1 + matching_criteria: + - attribute: type + operator: '' + value: google.cloud.pubsub.topic.v1.messagePublished + name: pubsub-topic-1 + project: my-project + timeouts: null + transport: + - pubsub: + - topic: topic1 + module.cloud_run.google_eventarc_trigger.pubsub_triggers["topic-2"]: + channel: null + destination: + - cloud_function: null + cloud_run_service: + - path: null + region: europe-west1 + service: hello + gke: [] + workflow: null + event_data_content_type: null + labels: null + location: europe-west1 + matching_criteria: + - attribute: type + operator: '' + value: google.cloud.pubsub.topic.v1.messagePublished + name: pubsub-topic-2 + project: my-project + timeouts: null + transport: + - pubsub: + - topic: topic2 module.cloud_run.google_service_account.trigger_service_account[0]: account_id: tf-cr-trigger-hello + description: null + disabled: false display_name: Terraform trigger for Cloud Run hello. project: my-project + timeouts: null counts: google_cloud_run_service: 1 - google_cloud_run_service_iam_binding: 1 + google_cloud_run_service_iam_member: 1 google_eventarc_trigger: 2 google_service_account: 1 + modules: 1 + resources: 5 + +outputs: {} +