feat!: replace grant_services_network_role with grant_network_role fo…

…r networkUser role management (#697)

* variablize networkUser role assignment

* pass grant_services_network_role to core_project_factory

* add a new line at the end of variables.tf file

* rename grant_services_network_role to grant_network_role

* README update with new input variable

* updates and testcases for grant_network_role

* add v13 upgrade guide and intigration test case

* upgrade doc updates

* Update docs/upgrading_to_project_factory_v13.0.md

Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>

* update test cases scenario context

* intigration test case for default project sa

* Update test/integration/dynamic_shared_vpc/controls/svpc.rb

Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>

Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>

README.md

@@ -138,7 +138,7 @@ determining that location is as follows:
 | enable\_shared\_vpc\_host\_project | If this project is a shared VPC host project. If true, you must *not* set svpc\_host\_project\_id variable. Default is false. | `bool` | `false` | no |
 | essential\_contacts | A mapping of users or groups to be assigned as Essential Contacts to the project, specifying a notification category | `map(list(string))` | `{}` | no |
 | folder\_id | The ID of a folder to host this project | `string` | `""` | no |
-| grant\_services\_network\_role | Whether or not to grant service agents the network roles on the host project | `bool` | `true` | no |
+| grant\_network\_role | Whether or not to grant networkUser role on the host project/subnets | `bool` | `true` | no |
 | grant\_services\_security\_admin\_role | Whether or not to grant Kubernetes Engine Service Agent the Security Admin role on the host project so it can manage firewall rules | `bool` | `false` | no |
 | group\_name | A group to control the project by being assigned group\_role (defaults to project editor) | `string` | `""` | no |
 | group\_role | The role to give the controlling group (group\_name) over the project (defaults to project editor) | `string` | `"roles/editor"` | no |

docs/upgrading_to_project_factory_v13.0.md

@@ -0,0 +1,36 @@
+# Upgrading to Project Factory v13.0
+
+The v13.0 release of Project Factory is a backwards incompatible release.
+
+## Migration Instructions
+
+### `grant_services_network_role` renamed to `grant_network_role`
+
+Variable `grant_services_network_role` is renamed to `grant_network_role` to provide the ability to not manage networkUser role through project factory module v13.0
+
+```diff
+ module "project-factory" {
+   source  = "terraform-google-modules/project-factory/google"
+-  version = "~> 12.0"
++  version = "~> 13.0"
+
+   name                            = "pf-test-1"
+   random_project_id               = "true"
+   org_id                          = "1234567890"
+   usage_bucket_name               = "pf-test-1-usage-report-bucket"
+   usage_bucket_prefix             = "pf/test/1/integration"
+   billing_account                 = "ABCDEF-ABCDEF-ABCDEF"
+-  grant_services_network_role     = "..."
++  grant_network_role              = "..."
+ }
+```
+
+Service accounts principles on which networkUser can be managed through `grant_network_role` variable.
+- Project default service account
+- [Google APIs service agent](https://cloud.google.com/compute/docs/access/service-accounts#google_apis_service_agent)
+- group_email
+- dataflow, dataproc, composer, container, and vpcaccess API [agent accounts](https://github.com/terraform-google-modules/terraform-google-project-factory/blob/616ede9456cc8f86ef7995192af3473d17ee7946/modules/shared_vpc_access/main.tf#L24-L30).
+
+Additional roles that are managed through `grant_network_role` variable.
+- roles/container.hostServiceAgentUser on "serviceAccount:service-{PROJECT-NUMBER}@container-engine-robot.iam.gserviceaccount.com
+- roles/composer.sharedVpcAgent on "service-{PROJECT-NUMBER}@cloudcomposer-accounts.iam.gserviceaccount.com"

examples/shared_vpc/main.tf

@@ -140,7 +140,7 @@ module "service-project-b" {
 
 /******************************************
   Third Service Project Creation
-  To test the grant_services_network_role
+  To test the grant_network_role
  *****************************************/
 module "service-project-c" {
   source = "../../modules/svpc_service_project"
@@ -152,12 +152,15 @@ module "service-project-c" {
   folder_id       = var.folder_id
   billing_account = var.billing_account
 
-  shared_vpc = module.host-project.project_id
+  shared_vpc         = module.host-project.project_id
+  shared_vpc_subnets = module.vpc.subnets_self_links
 
   activate_apis = [
     "compute.googleapis.com",
     "container.googleapis.com",
     "dataproc.googleapis.com",
+    "composer.googleapis.com",
+    "dataflow.googleapis.com"
   ]
 
   activate_api_identities = [{
@@ -169,7 +172,7 @@ module "service-project-c" {
   }]
 
   disable_services_on_destroy = false
-  grant_services_network_role = false
+  grant_network_role          = false
 }
 
 /******************************************

main.tf

@@ -39,6 +39,7 @@ module "project-factory" {
   shared_vpc                         = var.svpc_host_project_id
   enable_shared_vpc_service_project  = var.svpc_host_project_id != ""
   enable_shared_vpc_host_project     = var.enable_shared_vpc_host_project
+  grant_network_role                 = var.grant_network_role
   billing_account                    = var.billing_account
   folder_id                          = var.folder_id
   create_project_sa                  = var.create_project_sa
@@ -79,7 +80,7 @@ module "shared_vpc_access" {
   service_project_number             = module.project-factory.project_number
   lookup_project_numbers             = false
   grant_services_security_admin_role = var.grant_services_security_admin_role
-  grant_services_network_role        = var.grant_services_network_role
+  grant_network_role                 = var.grant_network_role
 }
 
 /******************************************

modules/core_project_factory/main.tf

@@ -179,7 +179,7 @@ resource "google_service_account_iam_member" "service_account_grant_to_group" {
   compute.networkUser role granted to G Suite group, APIs Service account, and Project Service Account
  *****************************************************************************************************************/
 resource "google_project_iam_member" "controlling_group_vpc_membership" {
-  count = var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) == 0 ? local.shared_vpc_users_length : 0
+  count = var.grant_network_role && var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) == 0 ? local.shared_vpc_users_length : 0
 
   project = var.shared_vpc
   role    = "roles/compute.networkUser"
@@ -195,7 +195,7 @@ resource "google_project_iam_member" "controlling_group_vpc_membership" {
  *************************************************************************************/
 resource "google_compute_subnetwork_iam_member" "service_account_role_to_vpc_subnets" {
   provider = google-beta
-  count    = var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) > 0 && var.create_project_sa ? length(var.shared_vpc_subnets) : 0
+  count    = var.grant_network_role && var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) > 0 && var.create_project_sa ? length(var.shared_vpc_subnets) : 0
 
   subnetwork = element(
     split("/", var.shared_vpc_subnets[count.index]),
@@ -219,7 +219,7 @@ resource "google_compute_subnetwork_iam_member" "service_account_role_to_vpc_sub
 resource "google_compute_subnetwork_iam_member" "group_role_to_vpc_subnets" {
   provider = google-beta
 
-  count = var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) > 0 && var.manage_group ? length(var.shared_vpc_subnets) : 0
+  count = var.grant_network_role && var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) > 0 && var.manage_group ? length(var.shared_vpc_subnets) : 0
   subnetwork = element(
     split("/", var.shared_vpc_subnets[count.index]),
     index(
@@ -242,7 +242,7 @@ resource "google_compute_subnetwork_iam_member" "group_role_to_vpc_subnets" {
 resource "google_compute_subnetwork_iam_member" "apis_service_account_role_to_vpc_subnets" {
   provider = google-beta
 
-  count = var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) > 0 ? length(var.shared_vpc_subnets) : 0
+  count = var.grant_network_role && var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) > 0 ? length(var.shared_vpc_subnets) : 0
   subnetwork = element(
     split("/", var.shared_vpc_subnets[count.index]),
     index(

modules/core_project_factory/variables.tf

@@ -233,3 +233,9 @@ variable "default_network_tier" {
   type        = string
   default     = ""
 }
+
+variable "grant_network_role" {
+  description = "Whether or not to grant networkUser role on the host project/subnets"
+  type        = bool
+  default     = true
+}

modules/shared_vpc_access/README.md

@@ -29,7 +29,7 @@ module "shared_vpc_access" {
 |------|-------------|------|---------|:--------:|
 | active\_apis | The list of active apis on the service project. If api is not active this module will not try to activate it | `list(string)` | `[]` | no |
 | enable\_shared\_vpc\_service\_project | Flag set if SVPC enabled | `bool` | n/a | yes |
-| grant\_services\_network\_role | Whether or not to grant service agents the network roles on the host project | `bool` | `true` | no |
+| grant\_network\_role | Whether or not to grant service agents the network roles on the host project | `bool` | `true` | no |
 | grant\_services\_security\_admin\_role | Whether or not to grant Kubernetes Engine Service Agent the Security Admin role on the host project so it can manage firewall rules | `bool` | `false` | no |
 | host\_project\_id | The ID of the host project which hosts the shared VPC | `string` | n/a | yes |
 | lookup\_project\_numbers | Whether to look up the project numbers from data sources. If false, `service_project_number` will be used instead. | `bool` | `true` | no |

modules/shared_vpc_access/main.tf

@@ -48,7 +48,7 @@ locals {
  *****************************************/
 resource "google_compute_subnetwork_iam_member" "service_shared_vpc_subnet_users" {
   provider = google-beta
-  count    = var.grant_services_network_role ? length(local.subnetwork_api) : 0
+  count    = var.grant_network_role ? length(local.subnetwork_api) : 0
   subnetwork = element(
     split("/", split(",", local.subnetwork_api[count.index])[1]),
     index(
@@ -71,7 +71,7 @@ resource "google_compute_subnetwork_iam_member" "service_shared_vpc_subnet_users
  if "dataflow.googleapis.com" compute.networkUser role granted to dataflow service account for Dataflow 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) && var.enable_shared_vpc_service_project && var.grant_services_network_role ? toset(local.active_apis) : []
+  for_each = (length(var.shared_vpc_subnets) == 0) && var.enable_shared_vpc_service_project && var.grant_network_role ? toset(local.active_apis) : []
   project  = var.host_project_id
   role     = "roles/compute.networkUser"
   member   = format("serviceAccount:%s", local.apis[each.value])
@@ -82,7 +82,7 @@ resource "google_project_iam_member" "service_shared_vpc_user" {
   See: https://cloud.google.com/composer/docs/how-to/managing/configuring-shared-vpc
  *****************************************/
 resource "google_project_iam_member" "composer_host_agent" {
-  count   = local.composer_shared_vpc_enabled && var.enable_shared_vpc_service_project && var.grant_services_network_role ? 1 : 0
+  count   = local.composer_shared_vpc_enabled && var.enable_shared_vpc_service_project && var.grant_network_role ? 1 : 0
   project = var.host_project_id
   role    = "roles/composer.sharedVpcAgent"
   member  = format("serviceAccount:%s", local.apis["composer.googleapis.com"])
@@ -93,7 +93,7 @@ resource "google_project_iam_member" "composer_host_agent" {
   See: https://cloud.google.com/kubernetes-engine/docs/how-to/cluster-shared-vpc
  *****************************************/
 resource "google_project_iam_member" "gke_host_agent" {
-  count   = local.gke_shared_vpc_enabled && var.enable_shared_vpc_service_project && var.grant_services_network_role ? 1 : 0
+  count   = local.gke_shared_vpc_enabled && var.enable_shared_vpc_service_project && var.grant_network_role ? 1 : 0
   project = var.host_project_id
   role    = "roles/container.hostServiceAgentUser"
   member  = format("serviceAccount:%s", local.apis["container.googleapis.com"])

modules/shared_vpc_access/variables.tf

@@ -59,7 +59,7 @@ variable "grant_services_security_admin_role" {
   default     = false
 }
 
-variable "grant_services_network_role" {
+variable "grant_network_role" {
   description = "Whether or not to grant service agents the network roles on the host project"
   type        = bool
   default     = true

modules/svpc_service_project/README.md

@@ -50,7 +50,7 @@ module "service-project" {
 | disable\_services\_on\_destroy | Whether project services will be disabled when the resources are destroyed | `bool` | `true` | no |
 | domain | The domain name (optional). | `string` | `""` | no |
 | folder\_id | The ID of a folder to host this project | `string` | `""` | no |
-| grant\_services\_network\_role | Whether or not to grant service agents the network roles on the host project | `bool` | `true` | no |
+| grant\_network\_role | Whether or not to grant service agents the network roles on the host project | `bool` | `true` | no |
 | grant\_services\_security\_admin\_role | Whether or not to grant Kubernetes Engine Service Agent the Security Admin role on the host project so it can manage firewall rules | `bool` | `false` | no |
 | group\_name | A group to control the project by being assigned group\_role (defaults to project editor) | `string` | `""` | no |
 | group\_role | The role to give the controlling group (group\_name) over the project (defaults to project editor) | `string` | `"roles/editor"` | no |

modules/svpc_service_project/main.tf

@@ -38,6 +38,7 @@ module "project-factory" {
   project_id                        = var.project_id
   shared_vpc                        = var.shared_vpc
   enable_shared_vpc_service_project = true
+  grant_network_role                = var.grant_network_role
   billing_account                   = var.billing_account
   folder_id                         = var.folder_id
   create_project_sa                 = var.create_project_sa
@@ -73,7 +74,7 @@ module "shared_vpc_access" {
   service_project_number             = module.project-factory.project_number
   lookup_project_numbers             = false
   grant_services_security_admin_role = var.grant_services_security_admin_role
-  grant_services_network_role        = var.grant_services_network_role
+  grant_network_role                 = var.grant_network_role
 }
 
 /******************************************

modules/svpc_service_project/variables.tf

@@ -216,7 +216,7 @@ variable "grant_services_security_admin_role" {
   default     = false
 }
 
-variable "grant_services_network_role" {
+variable "grant_network_role" {
   description = "Whether or not to grant service agents the network roles on the host project"
   type        = bool
   default     = true

test/integration/dynamic_shared_vpc/controls/svpc.rb

@@ -79,7 +79,7 @@
         )
       end
 
-      it "service project c with explicit subnets and grant_services_network_role flag set to false does not include the GKE service account in the roles/compute.networkUser IAM binding" do
+      it "service project c with explicit subnets and grant_network_role flag set to false 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_c_number}@container-engine-robot.iam.gserviceaccount.com"
@@ -96,7 +96,7 @@
       )
     end
 
-    it "service project c without explicit subnets and grant_services_network_role flag set to false does not include the GKE service account in the roles/compute.networkUser IAM binding" do
+    it "service project c with explicit subnets and grant_network_role flag set to false 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_c_number}@container-engine-robot.iam.gserviceaccount.com"),
         role: "roles/compute.networkUser",
@@ -110,7 +110,7 @@
     )
   end
 
-  it "service project c without explicit subnets and grant_services_network_role flag set to false does not include the dataproc service account in the roles/compute.networkUser IAM binding" do
+  it "service project c with explicit subnets and grant_network_role flag set to false does not include the dataproc service account in the roles/compute.networkUser IAM binding" do
     expect(bindings).not_to include(
       members: including("serviceAccount:service-#{service_project_c_number}@dataproc-accounts.iam.gserviceaccount.com"),
       role: "roles/compute.networkUser",
@@ -148,6 +148,15 @@
       end
     end
 
+    describe "roles/compute.networkUser" do
+      it "service project with explicit subnets includes project default service account in the roles/compute.networkUser IAM binding" do
+        expect(bindings).to include(
+          members: including("serviceAccount:project-service-account@#{service_project_ids[0]}.iam.gserviceaccount.com"),
+          role: "roles/compute.networkUser",
+        )
+      end
+    end
+
     describe "roles/compute.networkUser" do
       it "service project with explicit subnets includes the dataflow service account in the roles/compute.networkUser IAM binding" do
         expect(bindings).to include(
@@ -157,6 +166,42 @@
         )
       end
     end
+
+    it "service project c with explicit subnets and grant_network_role flag set to false does not include project default service account in the roles/compute.networkUser IAM binding" do
+      expect(bindings).not_to include(
+        members: including(
+          "serviceAccount:project-service-account@#{service_project_ids[2]}.iam.gserviceaccount.com"
+        ),
+        role: "roles/compute.networkUser",
+      )
+    end
+
+    it "service project c with explicit subnets and grant_network_role flag set to false does not include the GCP Compute agent service account in the roles/compute.networkUser IAM binding" do
+      expect(bindings).not_to include(
+        members: including(
+          "serviceAccount:#{service_project_c_number}@cloudservices.gserviceaccount.com"
+        ),
+        role: "roles/compute.networkUser",
+      )
+    end
+
+    it "service project c with explicit subnets and grant_network_role flag set to false does not include the GCP Dataflow agent service account in the roles/compute.networkUser IAM binding" do
+      expect(bindings).not_to include(
+        members: including(
+          "serviceAccount:service-#{service_project_c_number}@dataflow-service-producer-prod.iam.gserviceaccount.com"
+        ),
+        role: "roles/compute.networkUser",
+      )
+    end
+
+    it "service project c with explicit subnets and grant_network_role flag set to false does not include the GCP Dataproc agent service account in the roles/compute.networkUser IAM binding" do
+      expect(bindings).not_to include(
+        members: including(
+          "serviceAccount:service-#{service_project_c_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_02} --region #{shared_vpc_subnet_region_02} --project #{shared_vpc} --format=json") do

variables.tf

@@ -275,8 +275,8 @@ variable "grant_services_security_admin_role" {
   default     = false
 }
 
-variable "grant_services_network_role" {
-  description = "Whether or not to grant service agents the network roles on the host project"
+variable "grant_network_role" {
+  description = "Whether or not to grant networkUser role on the host project/subnets"
   type        = bool
   default     = true
 }