feat!: Use user defined SA for cb triggers (#148)

* Add service_account parameter usege when creating cb trigger * Fixes service account path * Bump dev-tools version to from 1.2.3 to 1.3 Code review suggestions Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com> * Adds service account user role * configure cross-project set up * add new required role: orgpolicy.policyAdmin * add permissions and logging bucket and move org policy to main module * fix integration test * add test to check terraform service account in cb trigger configuration * code review fixes * add time sleep for impersonation propagation for cloud build service agent Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com> Co-authored-by: Daniel da Silva Andrade <dandrade@ciandt.com>
terraform-google-modules · Jun 7, 2022 · 5a925f8 · 5a925f8
1 parent e1d3952
commit 5a925f8
Show file tree

Hide file tree

Showing 15 changed files with 172 additions and 37 deletions.
diff --git a/.gitignore b/.gitignore
@@ -22,7 +22,7 @@ Session.vim
 ### https://raw.github.com/github/gitignore/90f149de451a5433aebd94d02d11b0e28843a1af/Terraform.gitignore
 
 # Local .terraform directories
-**/.terraform/*
+**/.terraform*
 
 # .tfstate files
 *.tfstate

diff --git a/README.md b/README.md
@@ -95,6 +95,7 @@ For the cloudbuild submodule, see the README [cloudbuild](./modules/cloudbuild).
 ### Permissions
 
 - `roles/resourcemanager.organizationAdmin` on GCP Organization
+- `roles/orgpolicy.policyAdmin` on GCP Organization
 - `roles/billing.admin` on supplied billing account
 - Account running terraform should be a member of group provided in `group_org_admins` variable, otherwise they will loose `roles/resourcemanager.projectCreator` access. Additional members can be added by using the `org_project_creators` variable.
 

diff --git a/examples/cloudbuild_enabled/README.md b/examples/cloudbuild_enabled/README.md
@@ -21,7 +21,8 @@ This example combines the Organization bootstrap module with the Cloud Build sub
 |------|-------------|
 | cloudbuild\_project\_id | Project where CloudBuild configuration and terraform container image will reside. |
 | csr\_repos | List of Cloud Source Repos created by the module, linked to Cloud Build triggers. |
-| gcs\_bucket\_cloudbuild\_artifacts | Bucket used to store Cloud/Build artefacts in CloudBuild project. |
+| gcs\_bucket\_cloudbuild\_artifacts | Bucket used to store Cloud/Build artifacts in CloudBuild project. |
+| gcs\_bucket\_cloudbuild\_logs | Bucket used to store Cloud/Build logs in CloudBuild project. |
 | gcs\_bucket\_tfstate | Bucket used for storing terraform state for foundations pipelines in seed project. |
 | seed\_project\_id | Project where service accounts and core APIs will be enabled. |
 | terraform\_sa\_email | Email for privileged service account for Terraform. |

diff --git a/examples/cloudbuild_enabled/outputs.tf b/examples/cloudbuild_enabled/outputs.tf
@@ -40,10 +40,15 @@ output "cloudbuild_project_id" {
 }
 
 output "gcs_bucket_cloudbuild_artifacts" {
-  description = "Bucket used to store Cloud/Build artefacts in CloudBuild project."
+  description = "Bucket used to store Cloud/Build artifacts in CloudBuild project."
   value       = module.cloudbuild_bootstrap.gcs_bucket_cloudbuild_artifacts
 }
 
+output "gcs_bucket_cloudbuild_logs" {
+  description = "Bucket used to store Cloud/Build logs in CloudBuild project."
+  value       = module.cloudbuild_bootstrap.gcs_bucket_cloudbuild_logs
+}
+
 output "csr_repos" {
   description = "List of Cloud Source Repos created by the module, linked to Cloud Build triggers."
   value       = module.cloudbuild_bootstrap.csr_repos

diff --git a/main.tf b/main.tf
@@ -69,6 +69,18 @@ module "seed_project" {
   lien                        = true
 }
 
+module "enable_cross_project_service_account_usage" {
+  source  = "terraform-google-modules/org-policy/google"
+  version = "~> 5.1"
+
+  project_id  = module.seed_project.project_id
+  policy_for  = "project"
+  policy_type = "boolean"
+  enforce     = "false"
+  constraint  = "constraints/iam.disableCrossProjectServiceAccountUsage"
+}
+
+
 /******************************************
   Service Account - Terraform for Org
 *******************************************/
@@ -210,6 +222,14 @@ resource "google_storage_bucket_iam_member" "org_terraform_state_iam" {
   as org admin.
  ***********************************************/
 
+resource "google_service_account_iam_member" "org_admin_sa_user" {
+  count = local.impersonation_enabled_count
+
+  service_account_id = google_service_account.org_terraform.name
+  role               = "roles/iam.serviceAccountUser"
+  member             = "group:${var.group_org_admins}"
+}
+
 resource "google_service_account_iam_member" "org_admin_sa_impersonate_permissions" {
   count = local.impersonation_enabled_count
 
@@ -226,6 +246,14 @@ resource "google_organization_iam_member" "org_admin_serviceusage_consumer" {
   member = "group:${var.group_org_admins}"
 }
 
+resource "google_folder_iam_member" "org_admin_service_account_user" {
+  count = var.sa_enable_impersonation && !local.is_organization ? 1 : 0
+
+  folder = local.parent_id
+  role   = "roles/iam.serviceAccountUser"
+  member = "group:${var.group_org_admins}"
+}
+
 resource "google_folder_iam_member" "org_admin_serviceusage_consumer" {
   count = var.sa_enable_impersonation && !local.is_organization ? 1 : 0
 

diff --git a/modules/cloudbuild/README.md b/modules/cloudbuild/README.md
@@ -83,7 +83,8 @@ Functional examples and sample Cloud Build definitions are included in the [exam
 |------|-------------|
 | cloudbuild\_project\_id | Project where CloudBuild configuration and terraform container image will reside. |
 | csr\_repos | List of Cloud Source Repos created by the module, linked to Cloud Build triggers. |
-| gcs\_bucket\_cloudbuild\_artifacts | Bucket used to store Cloud/Build artefacts in CloudBuild project. |
+| gcs\_bucket\_cloudbuild\_artifacts | Bucket used to store Cloud/Build artifacts in CloudBuild project. |
+| gcs\_bucket\_cloudbuild\_logs | Bucket used to store Cloud/Build logs in CloudBuild project. |
 | tf\_runner\_artifact\_repo | GAR Repo created to store runner images |
 
 <!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->

diff --git a/modules/cloudbuild/main.tf b/modules/cloudbuild/main.tf
@@ -33,7 +33,6 @@ data "google_organization" "org" {
   organization = var.org_id
 }
 
-
 /******************************************
   Cloudbuild project
 *******************************************/
@@ -51,6 +50,66 @@ module "cloudbuild_project" {
   labels                      = var.project_labels
 }
 
+/******************************************
+  Cloudbuild IAM for terraform SA
+*******************************************/
+// See https://cloud.google.com/build/docs/securing-builds/configure-user-specified-service-accounts
+// for details regarding the configuration of the Terraform service account to run Cloud Build
+
+resource "google_project_iam_member" "terraform_sa_log_writer" {
+  project = module.cloudbuild_project.project_id
+  role    = "roles/logging.logWriter"
+  member  = "serviceAccount:${var.terraform_sa_email}"
+}
+
+resource "google_artifact_registry_repository_iam_member" "terraform_sa_artifact_registry_reader" {
+  provider = google-beta
+
+  project    = module.cloudbuild_project.project_id
+  location   = google_artifact_registry_repository.tf-image-repo.location
+  repository = google_artifact_registry_repository.tf-image-repo.name
+  role       = "roles/artifactregistry.reader"
+  member     = "serviceAccount:${var.terraform_sa_email}"
+}
+
+resource "google_service_account_iam_member" "terraform_sa_self_impersonate" {
+  service_account_id = var.terraform_sa_name
+  role               = "roles/iam.serviceAccountUser"
+  member             = "serviceAccount:${var.terraform_sa_email}"
+}
+
+resource "google_service_account_iam_member" "terraform_sa_self_impersonate_token" {
+  service_account_id = var.terraform_sa_name
+  role               = "roles/iam.serviceAccountTokenCreator"
+  member             = "serviceAccount:${var.terraform_sa_email}"
+}
+
+resource "google_storage_bucket_iam_member" "terraform_sa_artifacts_iam" {
+  bucket = google_storage_bucket.cloudbuild_artifacts.name
+  role   = "roles/storage.objectCreator"
+  member = "serviceAccount:${var.terraform_sa_email}"
+}
+
+resource "google_storage_bucket_iam_member" "terraform_sa_logs_iam" {
+  bucket = google_storage_bucket.cloudbuild_logs.name
+  role   = "roles/storage.admin"
+  member = "serviceAccount:${var.terraform_sa_email}"
+}
+
+resource "google_service_account_iam_member" "cloud_build_service_agent_sa_impersonate" {
+  service_account_id = var.terraform_sa_name
+  role               = "roles/iam.serviceAccountTokenCreator"
+  member             = "serviceAccount:service-${module.cloudbuild_project.project_number}@gcp-sa-cloudbuild.iam.gserviceaccount.com"
+}
+
+resource "time_sleep" "impersonate_propagation" {
+  create_duration = "30s"
+
+  depends_on = [
+    google_service_account_iam_member.cloud_build_service_agent_sa_impersonate
+  ]
+}
+
 /******************************************
   Cloudbuild IAM for admins
 *******************************************/
@@ -67,6 +126,18 @@ resource "google_project_iam_member" "org_admins_cloudbuild_viewer" {
   member  = "group:${var.group_org_admins}"
 }
 
+/******************************************
+  Cloudbuild Logs bucket
+*******************************************/
+
+resource "google_storage_bucket" "cloudbuild_logs" {
+  project                     = module.cloudbuild_project.project_id
+  name                        = format("%s-%s-%s", module.cloudbuild_project.project_id, "cloudbuild-logs", random_id.suffix.hex)
+  location                    = var.default_region
+  labels                      = var.storage_bucket_labels
+  uniform_bucket_level_access = true
+}
+
 /******************************************
   Cloudbuild Artifact bucket
 *******************************************/
@@ -108,9 +179,10 @@ resource "google_project_iam_member" "org_admins_source_repo_admin" {
  ***********************************************/
 
 resource "google_cloudbuild_trigger" "main_trigger" {
-  for_each    = var.create_cloud_source_repos ? toset(var.cloud_source_repos) : []
-  project     = module.cloudbuild_project.project_id
-  description = "${each.value} - terraform apply."
+  for_each        = var.create_cloud_source_repos ? toset(var.cloud_source_repos) : []
+  project         = module.cloudbuild_project.project_id
+  description     = "${each.value} - terraform apply."
+  service_account = var.terraform_sa_name
 
   trigger_template {
     branch_name = local.apply_branches_regex
@@ -125,12 +197,15 @@ resource "google_cloudbuild_trigger" "main_trigger" {
     _TF_SA_EMAIL          = var.terraform_sa_email
     _STATE_BUCKET_NAME    = var.terraform_state_bucket
     _ARTIFACT_BUCKET_NAME = google_storage_bucket.cloudbuild_artifacts.name
+    _LOGS_BUCKET_NAME     = google_storage_bucket.cloudbuild_logs.name
     _TF_ACTION            = "apply"
   }
 
   filename = var.cloudbuild_apply_filename
   depends_on = [
     google_sourcerepo_repository.gcp_repo,
+    google_service_account_iam_member.org_admin_terraform_sa_impersonate,
+    time_sleep.impersonate_propagation
   ]
 }
 
@@ -139,9 +214,10 @@ resource "google_cloudbuild_trigger" "main_trigger" {
  ***********************************************/
 
 resource "google_cloudbuild_trigger" "non_main_trigger" {
-  for_each    = var.create_cloud_source_repos ? toset(var.cloud_source_repos) : []
-  project     = module.cloudbuild_project.project_id
-  description = "${each.value} - terraform plan."
+  for_each        = var.create_cloud_source_repos ? toset(var.cloud_source_repos) : []
+  project         = module.cloudbuild_project.project_id
+  description     = "${each.value} - terraform plan."
+  service_account = var.terraform_sa_name
 
   trigger_template {
     invert_regex = true
@@ -157,12 +233,15 @@ resource "google_cloudbuild_trigger" "non_main_trigger" {
     _TF_SA_EMAIL          = var.terraform_sa_email
     _STATE_BUCKET_NAME    = var.terraform_state_bucket
     _ARTIFACT_BUCKET_NAME = google_storage_bucket.cloudbuild_artifacts.name
+    _LOGS_BUCKET_NAME     = google_storage_bucket.cloudbuild_logs.name
     _TF_ACTION            = "plan"
   }
 
   filename = var.cloudbuild_plan_filename
   depends_on = [
     google_sourcerepo_repository.gcp_repo,
+    google_service_account_iam_member.org_admin_terraform_sa_impersonate,
+    time_sleep.impersonate_propagation
   ]
 }
 
@@ -222,27 +301,10 @@ resource "google_artifact_registry_repository_iam_member" "terraform-image-iam"
   member     = "serviceAccount:${module.cloudbuild_project.project_number}@cloudbuild.gserviceaccount.com"
 }
 
-resource "google_service_account_iam_member" "cloudbuild_terraform_sa_impersonate_permissions" {
+resource "google_service_account_iam_member" "org_admin_terraform_sa_impersonate" {
   count = local.impersonation_enabled_count
 
   service_account_id = var.terraform_sa_name
-  role               = "roles/iam.serviceAccountTokenCreator"
-  member             = "serviceAccount:${module.cloudbuild_project.project_number}@cloudbuild.gserviceaccount.com"
-}
-
-resource "google_organization_iam_member" "cloudbuild_serviceusage_consumer" {
-  count = local.impersonation_enabled_count
-
-  org_id = var.org_id
-  role   = "roles/serviceusage.serviceUsageConsumer"
-  member = "serviceAccount:${module.cloudbuild_project.project_number}@cloudbuild.gserviceaccount.com"
-}
-
-# Required to allow cloud build to access state with impersonation.
-resource "google_storage_bucket_iam_member" "cloudbuild_state_iam" {
-  count = local.impersonation_enabled_count
-
-  bucket = var.terraform_state_bucket
-  role   = "roles/storage.admin"
-  member = "serviceAccount:${module.cloudbuild_project.project_number}@cloudbuild.gserviceaccount.com"
+  role               = "roles/iam.serviceAccountUser"
+  member             = "group:${var.group_org_admins}"
 }
diff --git a/modules/cloudbuild/outputs.tf b/modules/cloudbuild/outputs.tf
@@ -20,10 +20,15 @@ output "cloudbuild_project_id" {
 }
 
 output "gcs_bucket_cloudbuild_artifacts" {
-  description = "Bucket used to store Cloud/Build artefacts in CloudBuild project."
+  description = "Bucket used to store Cloud/Build artifacts in CloudBuild project."
   value       = google_storage_bucket.cloudbuild_artifacts.name
 }
 
+output "gcs_bucket_cloudbuild_logs" {
+  description = "Bucket used to store Cloud/Build logs in CloudBuild project."
+  value       = google_storage_bucket.cloudbuild_logs.name
+}
+
 output "csr_repos" {
   description = "List of Cloud Source Repos created by the module, linked to Cloud Build triggers."
   value       = google_sourcerepo_repository.gcp_repo

diff --git a/test/fixtures/cloudbuild_enabled/outputs.tf b/test/fixtures/cloudbuild_enabled/outputs.tf
@@ -45,10 +45,15 @@ output "cloudbuild_project_id" {
 }
 
 output "gcs_bucket_cloudbuild_artifacts" {
-  description = "Bucket used to store Cloud/Build artefacts in CloudBuild project."
+  description = "Bucket used to store Cloud/Build artifacts in CloudBuild project."
   value       = module.cloudbuild_enabled.gcs_bucket_cloudbuild_artifacts
 }
 
+output "gcs_bucket_cloudbuild_logs" {
+  description = "Bucket used to store Cloud/Build logs in CloudBuild project."
+  value       = module.cloudbuild_enabled.gcs_bucket_cloudbuild_logs
+}
+
 output "csr_repos" {
   description = "List of Cloud Source Repos created by the module, linked to Cloud Build triggers."
   value       = module.cloudbuild_enabled.csr_repos

diff --git a/test/integration/cloudbuild_enabled/controls/gcloud.rb b/test/integration/cloudbuild_enabled/controls/gcloud.rb
@@ -46,4 +46,24 @@
         end
       end
     end
+
+    title "Terraform SA in Trigger"
+    describe command("gcloud beta --project=#{attribute("cloudbuild_project_id")} builds triggers list --format=json") do
+      its(:exit_status) { should eq 0 }
+      let!(:data) do
+        if subject.exit_status == 0
+          JSON.parse(subject.stdout)
+        else
+          {}
+        end
+      end
+      describe "Terraform SA" do
+        it "exists" do
+          expect(data[0]['serviceAccount']).to include "#{attribute("terraform_sa_email")}"
+        end
+        it "exists" do
+          expect(data[1]['serviceAccount']).to include "#{attribute("terraform_sa_email")}"
+        end
+      end
+    end
   end
diff --git a/test/integration/cloudbuild_enabled/controls/gcp.rb b/test/integration/cloudbuild_enabled/controls/gcp.rb
@@ -76,6 +76,10 @@
     it { should exist }
   end
 
+  describe google_storage_bucket(name: attribute("gcs_bucket_cloudbuild_logs")) do
+    it { should exist }
+  end
+
   default_apis.each do |api|
     describe google_project_service(project: attribute("cloudbuild_project_id"), name: api) do
       it { should exist }
@@ -95,7 +99,6 @@
       it { should exist }
       its('members') {should include 'group:' + attribute("group_org_admins")}
       its('members') {should include 'serviceAccount:' + attribute("terraform_sa_email")}
-      its('members') {should include 'serviceAccount:' + project_number.to_s + '@cloudbuild.gserviceaccount.com'}
     end
   end
 

diff --git a/test/integration/cloudbuild_enabled/inspec.yml b/test/integration/cloudbuild_enabled/inspec.yml
@@ -38,6 +38,9 @@ attributes:
   - name: gcs_bucket_cloudbuild_artifacts
     required: true
     type: string
+  - name: gcs_bucket_cloudbuild_logs
+    required: true
+    type: string
   - name: csr_repos
     required: true
     type: hash

diff --git a/test/setup/iam.tf b/test/setup/iam.tf
@@ -21,6 +21,7 @@ locals {
   int_org_required_roles = [
     "roles/billing.user",
     "roles/resourcemanager.organizationAdmin",
+    "roles/orgpolicy.policyAdmin",
     "roles/resourcemanager.projectCreator"
   ]
 }

diff --git a/test/setup/main.tf b/test/setup/main.tf
@@ -16,7 +16,7 @@
 
 module "project" {
   source  = "terraform-google-modules/project-factory/google"
-  version = "~> 10.0.1"
+  version = "~> 11.3.1"
 
   name              = "ci-bootstrap"
   random_project_id = true