fix: Fix regression in shared VPC service account submodule (#438)

Makefile

@@ -18,7 +18,7 @@
 # Make will use bash instead of sh
 SHELL := /usr/bin/env bash
 
-DOCKER_TAG_VERSION_DEVELOPER_TOOLS := 0
+DOCKER_TAG_VERSION_DEVELOPER_TOOLS := 0.12.0
 DOCKER_IMAGE_DEVELOPER_TOOLS := cft/developer-tools
 REGISTRY_URL := gcr.io/cloud-foundation-cicd
 

build/int.cloudbuild.yaml

@@ -46,4 +46,4 @@ tags:
 - 'integration'
 substitutions:
   _DOCKER_IMAGE_DEVELOPER_TOOLS: 'cft/developer-tools'
-  _DOCKER_TAG_VERSION_DEVELOPER_TOOLS: '0'
+  _DOCKER_TAG_VERSION_DEVELOPER_TOOLS: '0.12.0'

build/lint.cloudbuild.yaml

@@ -21,4 +21,4 @@ tags:
 - 'lint'
 substitutions:
   _DOCKER_IMAGE_DEVELOPER_TOOLS: 'cft/developer-tools'
-  _DOCKER_TAG_VERSION_DEVELOPER_TOOLS: '0'
+  _DOCKER_TAG_VERSION_DEVELOPER_TOOLS: '0.12.0'

modules/shared_vpc_access/main.tf

@@ -19,40 +19,48 @@ data "google_project" "service_project" {
 }
 
 locals {
+  apis = {
+    "container.googleapis.com" : format("service-%[email protected]", data.google_project.service_project.number),
+    "dataproc.googleapis.com" : format("service-%[email protected]", data.google_project.service_project.number),
+  }
   gke_shared_vpc_enabled = contains(var.active_apis, "container.googleapis.com")
-  gke_s_account = local.gke_shared_vpc_enabled ? format(
-    "service-%[email protected]",
-    data.google_project.service_project.number,
-  ) : ""
-  dataproc_shared_vpc_enabled = contains(var.active_apis, "dataproc.googleapis.com")
-  dataproc_s_account = local.dataproc_shared_vpc_enabled ? format(
-    "service-%[email protected]",
-    data.google_project.service_project.number
-  ) : ""
-  active_api_s_accounts = compact([local.gke_s_account, local.dataproc_s_account])
+  active_apis            = setintersection(keys(local.apis), var.active_apis)
+  subnetwork_api         = length(var.shared_vpc_subnets) != 0 ? tolist(setproduct(local.active_apis, var.shared_vpc_subnets)) : []
 }
 
 /******************************************
-  compute.networkUser role granted to GKE service account for GKE on shared VPC subnets
+  if "container.googleapis.com" compute.networkUser role granted to GKE service account for GKE on shared VPC subnets
+  if "dataproc.googleapis.com" compute.networkUser role granted to dataproc service account for dataproc on shared VPC subnets
   See: https://cloud.google.com/kubernetes-engine/docs/how-to/cluster-shared-vpc
  *****************************************/
-resource "google_compute_subnetwork_iam_member" "gke_shared_vpc_subnets" {
+resource "google_compute_subnetwork_iam_member" "service_shared_vpc_subnet_users" {
   provider = google-beta
-  count    = local.gke_shared_vpc_enabled && length(var.shared_vpc_subnets) != 0 ? length(var.shared_vpc_subnets) : 0
+  count    = length(local.subnetwork_api)
   subnetwork = element(
-    split("/", var.shared_vpc_subnets[count.index]),
+    split("/", local.subnetwork_api[count.index][1]),
     index(
-      split("/", var.shared_vpc_subnets[count.index]),
+      split("/", local.subnetwork_api[count.index][1]),
       "subnetworks",
     ) + 1,
   )
   role = "roles/compute.networkUser"
   region = element(
-    split("/", var.shared_vpc_subnets[count.index]),
-    index(split("/", var.shared_vpc_subnets[count.index]), "regions") + 1,
+    split("/", local.subnetwork_api[count.index][1]),
+    index(split("/", local.subnetwork_api[count.index][1]), "regions") + 1,
   )
   project = var.host_project_id
-  member  = format("serviceAccount:%s", local.gke_s_account)
+  member  = format("serviceAccount:%s", local.apis[local.subnetwork_api[count.index][0]])
+}
+
+/******************************************
+ if "container.googleapis.com" compute.networkUser role granted to GKE service account for GKE on shared VPC Project if no subnets defined
+ if "dataproc.googleapis.com" compute.networkUser role granted to dataproc service account for dataproc on shared VPC Project if no subnets defined
+ *****************************************/
+resource "google_project_iam_member" "service_shared_vpc_user" {
+  for_each = length(var.shared_vpc_subnets) == 0 ? local.active_apis : []
+  project  = var.host_project_id
+  role     = "roles/compute.networkUser"
+  member   = format("serviceAccount:%s", local.apis[each.value])
 }
 
 /******************************************
@@ -63,16 +71,5 @@ resource "google_project_iam_member" "gke_host_agent" {
   count   = local.gke_shared_vpc_enabled ? 1 : 0
   project = var.host_project_id
   role    = "roles/container.hostServiceAgentUser"
-  member  = format("serviceAccount:%s", local.gke_s_account)
-}
-
-/******************************************
-  compute.networkUser role granted to dataproc service account for dataproc on shared VPC subnets
-  See: https://cloud.google.com/dataproc/docs/concepts/configuring-clusters/network#creating_a_cluster_that_uses_a_vpc_network_in_another_project
- *****************************************/
-resource "google_project_iam_member" "dataproc_shared_vpc_network_user" {
-  count   = local.dataproc_shared_vpc_enabled ? 1 : 0
-  project = var.host_project_id
-  role    = "roles/compute.networkUser"
-  member  = format("serviceAccount:%s", local.dataproc_s_account)
+  member  = format("serviceAccount:%s", local.apis["container.googleapis.com"])
 }

modules/shared_vpc_access/outputs.tf

@@ -16,15 +16,15 @@
 
 output "active_api_service_accounts" {
   description = "List of active API service accounts in the service project."
-  value       = local.active_api_s_accounts
+  value       = local.active_apis
 }
 
 output "project_id" {
   description = "Service project ID."
   value       = var.service_project_id
   depends_on = [
-    google_compute_subnetwork_iam_member.gke_shared_vpc_subnets,
+    google_compute_subnetwork_iam_member.gke_dataproc_shared_vpc_subnets,
     google_project_iam_member.gke_host_agent,
-    google_project_iam_member.dataproc_shared_vpc_network_user,
+    google_project_iam_member.gke_dataproc_shared_vpc_network_user,
   ]
 }

test/fixtures/dynamic_shared_vpc/outputs.tf

@@ -37,6 +37,11 @@ output "service_project_number" {
   description = "The service project number"
 }
 
+output "service_project_b_number" {
+  value       = module.example.service_project_b.project_number
+  description = "The service project b number"
+}
+
 output "service_account_email" {
   value       = module.example.service_project.service_account_email
   description = "The service account email"

test/integration/dynamic_shared_vpc/controls/svpc.rb

@@ -15,6 +15,7 @@
 service_project_id          = attribute('service_project_id')
 service_project_ids         = attribute('service_project_ids')
 service_project_number      = attribute('service_project_number')
+service_project_b_number    = attribute('service_project_b_number')
 service_account_email       = attribute('service_account_email')
 shared_vpc                  = attribute('shared_vpc')
 shared_vpc_subnet_name_01   = attribute('shared_vpc_subnet_name_01')
@@ -53,7 +54,22 @@
         )
       end
 
-      it "does not include the GKE service account in the roles/compute.networkUser IAM binding" do
+
+    it "service project with explicit subnets includes the GKE service account in the roles/container.hostServiceAgentUser IAM binding" do
+      expect(bindings).to include(
+        members: including("serviceAccount:service-#{service_project_number}@container-engine-robot.iam.gserviceaccount.com"),
+        role: "roles/container.hostServiceAgentUser",
+      )
+    end
+
+    it "service project b without explicit subnets includes the GKE service account in the roles/container.hostServiceAgentUser IAM binding" do
+      expect(bindings).to include(
+        members: including("serviceAccount:service-#{service_project_b_number}@container-engine-robot.iam.gserviceaccount.com"),
+        role: "roles/container.hostServiceAgentUser",
+      )
+    end
+
+      it "service project with explicit subnets does not include the GKE service account in the roles/compute.networkUser IAM binding" do
         expect(bindings).not_to include(
           members: including(
             "serviceAccount:service-#{service_project_number}@container-engine-robot.iam.gserviceaccount.com"
@@ -63,20 +79,20 @@
       end
     end
 
-    it "includes the GKE service account in the roles/container.hostServiceAgentUser IAM binding" do
-      expect(bindings).to include(
-        members: including("serviceAccount:service-#{service_project_number}@container-engine-robot.iam.gserviceaccount.com"),
-        role: "roles/container.hostServiceAgentUser",
-      )
-    end
-
-    it "includes the dataproc service account in the roles/compute.networkUser IAM binding" do
+    it "service project b without explicit subnets includes the GKE service account in the roles/compute.networkUser IAM binding" do
       expect(bindings).to include(
-        members: including("serviceAccount:service-#{service_project_number}@dataproc-accounts.iam.gserviceaccount.com"),
+        members: including("serviceAccount:service-#{service_project_b_number}@container-engine-robot.iam.gserviceaccount.com"),
         role: "roles/compute.networkUser",
       )
     end
+
+  it "service project b without explicit subnets includes the dataproc service account in the roles/compute.networkUser IAM binding" do
+    expect(bindings).to include(
+      members: including("serviceAccount:service-#{service_project_b_number}@dataproc-accounts.iam.gserviceaccount.com"),
+      role: "roles/compute.networkUser",
+    )
   end
+end
 
   describe command("gcloud beta compute networks subnets get-iam-policy #{shared_vpc_subnet_name_01} --region #{shared_vpc_subnet_region_01} --project #{shared_vpc} --format=json") do
     its('exit_status') { should eq 0 }
@@ -98,6 +114,16 @@
         )
       end
     end
+
+    describe "roles/compute.networkUser" do
+      it "service project with explicit subnets includes the GKE service account in the roles/compute.networkUser IAM binding" do
+        expect(bindings).to include(
+          members: including("serviceAccount:service-#{service_project_number}@container-engine-robot.iam.gserviceaccount.com"),
+          role: "roles/compute.networkUser",
+        )
+      end
+    end
+
   end
 
   describe command("gcloud beta compute networks subnets get-iam-policy #{shared_vpc_subnet_name_02} --region #{shared_vpc_subnet_region_02} --project #{shared_vpc} --format=json") do
@@ -120,5 +146,15 @@
         )
       end
     end
+
+    describe "roles/compute.networkUser" do
+      it "service project b without explicit subnets does not include the GKE service account in the roles/compute.networkUser IAM binding" do
+        expect(bindings).not_to include(
+          members: including("serviceAccount:service-#{service_project_b_number}@container-engine-robot.iam.gserviceaccount.com"),
+          role: "roles/compute.networkUser",
+        )
+      end
+    end
+
   end
 end

test/integration/dynamic_shared_vpc/inspec.yml

@@ -16,6 +16,9 @@ attributes:
   - name: service_project_number
     required: true
     type: string
+  - name: service_project_b_number
+    required: true
+    type: string
   - name: service_account_email
     required: true
     type: string

test/setup/main.tf

@@ -43,11 +43,12 @@ module "pfactory_project" {
   source  = "terraform-google-modules/project-factory/google"
   version = "~> 8.0"
 
-  name              = "ci-pfactory-tests"
-  random_project_id = true
-  org_id            = var.org_id
-  folder_id         = google_folder.ci_pfactory_folder.id
-  billing_account   = var.billing_account
+  name                 = "ci-pfactory-tests"
+  random_project_id    = true
+  org_id               = var.org_id
+  folder_id            = google_folder.ci_pfactory_folder.id
+  billing_account      = var.billing_account
+  skip_gcloud_download = true
 
   activate_apis = [
     "admin.googleapis.com",