From d3aaaa07835ba6974056a37252b767571913d9c5 Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Tue, 5 Apr 2022 15:23:28 -0400 Subject: [PATCH 01/12] variablize networkUser role assignment --- modules/core_project_factory/main.tf | 8 ++++---- modules/core_project_factory/variables.tf | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/modules/core_project_factory/main.tf b/modules/core_project_factory/main.tf index 095592af..7e253d28 100644 --- a/modules/core_project_factory/main.tf +++ b/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_services_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_services_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_services_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_services_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( diff --git a/modules/core_project_factory/variables.tf b/modules/core_project_factory/variables.tf index 5f7b0e2b..55c0a601 100644 --- a/modules/core_project_factory/variables.tf +++ b/modules/core_project_factory/variables.tf @@ -233,3 +233,9 @@ variable "default_network_tier" { type = string default = "" } + +variable "grant_services_network_role" { + description = "Whether or not to grant networkUser role on the host project/subnets" + type = bool + default = true +} \ No newline at end of file From 372beb02fd4d5caa1a0e24f4f0a30a803ed58adb Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Tue, 5 Apr 2022 15:55:21 -0400 Subject: [PATCH 02/12] pass grant_services_network_role to core_project_factory --- main.tf | 1 + 1 file changed, 1 insertion(+) diff --git a/main.tf b/main.tf index a7c04a15..e8422a10 100644 --- a/main.tf +++ b/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_services_network_role = var.grant_services_network_role billing_account = var.billing_account folder_id = var.folder_id create_project_sa = var.create_project_sa From 91de32761844510642902ac4ad1923cf59c85d23 Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Tue, 5 Apr 2022 16:27:31 -0400 Subject: [PATCH 03/12] add a new line at the end of variables.tf file --- modules/core_project_factory/variables.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core_project_factory/variables.tf b/modules/core_project_factory/variables.tf index 55c0a601..f431f040 100644 --- a/modules/core_project_factory/variables.tf +++ b/modules/core_project_factory/variables.tf @@ -238,4 +238,4 @@ variable "grant_services_network_role" { description = "Whether or not to grant networkUser role on the host project/subnets" type = bool default = true -} \ No newline at end of file +} From 3333c317396e1dc17a8bbbcfec2fad0407e31fa2 Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Wed, 6 Apr 2022 14:32:40 -0400 Subject: [PATCH 04/12] rename grant_services_network_role to grant_network_role --- main.tf | 2 +- modules/core_project_factory/main.tf | 8 ++++---- modules/core_project_factory/variables.tf | 2 +- variables.tf | 6 ++++++ 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/main.tf b/main.tf index e8422a10..8f9ff8c8 100644 --- a/main.tf +++ b/main.tf @@ -39,7 +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_services_network_role = var.grant_services_network_role + grant_network_role = var.grant_network_role billing_account = var.billing_account folder_id = var.folder_id create_project_sa = var.create_project_sa diff --git a/modules/core_project_factory/main.tf b/modules/core_project_factory/main.tf index 7e253d28..30882590 100644 --- a/modules/core_project_factory/main.tf +++ b/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.grant_services_network_role && 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.grant_services_network_role && 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.grant_services_network_role && 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.grant_services_network_role && 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( diff --git a/modules/core_project_factory/variables.tf b/modules/core_project_factory/variables.tf index f431f040..381d3cda 100644 --- a/modules/core_project_factory/variables.tf +++ b/modules/core_project_factory/variables.tf @@ -234,7 +234,7 @@ variable "default_network_tier" { default = "" } -variable "grant_services_network_role" { +variable "grant_network_role" { description = "Whether or not to grant networkUser role on the host project/subnets" type = bool default = true diff --git a/variables.tf b/variables.tf index 242f23c6..60e9278b 100644 --- a/variables.tf +++ b/variables.tf @@ -309,3 +309,9 @@ variable "language_tag" { type = string default = "en-US" } + +variable "grant_network_role" { + description = "Whether or not to grant networkUser role on the host project/subnets" + type = bool + default = true +} From bfaec2836afcd102326d41504f4422b34d60d028 Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Wed, 6 Apr 2022 14:47:43 -0400 Subject: [PATCH 05/12] README update with new input variable --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 665019bb..c69ed701 100644 --- a/README.md +++ b/README.md @@ -138,6 +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\_network\_role | Whether or not to grant networkUser role on the host project/subnets | `bool` | `true` | no | | grant\_services\_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 | From 227936cf3ad2b23d5ec8dc3c3c927d6dd130b6b5 Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Wed, 6 Apr 2022 21:57:01 -0400 Subject: [PATCH 06/12] updates and testcases for grant_network_role --- README.md | 1 - examples/shared_vpc/main.tf | 9 +++-- main.tf | 2 +- modules/shared_vpc_access/README.md | 2 +- modules/shared_vpc_access/main.tf | 8 ++--- modules/shared_vpc_access/variables.tf | 2 +- modules/svpc_service_project/README.md | 2 +- modules/svpc_service_project/main.tf | 3 +- modules/svpc_service_project/variables.tf | 2 +- .../dynamic_shared_vpc/controls/svpc.rb | 33 +++++++++++++++++-- variables.tf | 10 ++---- 11 files changed, 49 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index c69ed701..9b7a15fc 100644 --- a/README.md +++ b/README.md @@ -139,7 +139,6 @@ determining that location is as follows: | 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\_network\_role | Whether or not to grant networkUser role on the host project/subnets | `bool` | `true` | no | -| grant\_services\_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 | diff --git a/examples/shared_vpc/main.tf b/examples/shared_vpc/main.tf index 0060e4f6..02955131 100644 --- a/examples/shared_vpc/main.tf +++ b/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 } /****************************************** diff --git a/main.tf b/main.tf index 8f9ff8c8..9411d108 100644 --- a/main.tf +++ b/main.tf @@ -80,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 } /****************************************** diff --git a/modules/shared_vpc_access/README.md b/modules/shared_vpc_access/README.md index 7fd89dde..d68a989b 100644 --- a/modules/shared_vpc_access/README.md +++ b/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 | diff --git a/modules/shared_vpc_access/main.tf b/modules/shared_vpc_access/main.tf index 47473f22..a22135aa 100644 --- a/modules/shared_vpc_access/main.tf +++ b/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"]) diff --git a/modules/shared_vpc_access/variables.tf b/modules/shared_vpc_access/variables.tf index e2e274c4..d798f632 100644 --- a/modules/shared_vpc_access/variables.tf +++ b/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 diff --git a/modules/svpc_service_project/README.md b/modules/svpc_service_project/README.md index 80b86947..88db4cfa 100644 --- a/modules/svpc_service_project/README.md +++ b/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 | diff --git a/modules/svpc_service_project/main.tf b/modules/svpc_service_project/main.tf index aca02503..c00ad9d9 100755 --- a/modules/svpc_service_project/main.tf +++ b/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 } /****************************************** diff --git a/modules/svpc_service_project/variables.tf b/modules/svpc_service_project/variables.tf index b8884eb6..ada6e4f5 100755 --- a/modules/svpc_service_project/variables.tf +++ b/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 diff --git a/test/integration/dynamic_shared_vpc/controls/svpc.rb b/test/integration/dynamic_shared_vpc/controls/svpc.rb index 09ad2dce..f02c3eb3 100644 --- a/test/integration/dynamic_shared_vpc/controls/svpc.rb +++ b/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 without 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 without 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", @@ -157,6 +157,33 @@ ) end 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 diff --git a/variables.tf b/variables.tf index 60e9278b..8e420e12 100644 --- a/variables.tf +++ b/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 } @@ -309,9 +309,3 @@ variable "language_tag" { type = string default = "en-US" } - -variable "grant_network_role" { - description = "Whether or not to grant networkUser role on the host project/subnets" - type = bool - default = true -} From 41c33420d997177e14912d415efd260bddd0b1ac Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Thu, 7 Apr 2022 10:14:48 -0400 Subject: [PATCH 07/12] add v13 upgrade guide and intigration test case --- docs/upgrading_to_project_factory_v13.0.md | 32 +++++++++++++++++++ .../dynamic_shared_vpc/controls/svpc.rb | 9 ++++++ 2 files changed, 41 insertions(+) create mode 100644 docs/upgrading_to_project_factory_v13.0.md diff --git a/docs/upgrading_to_project_factory_v13.0.md b/docs/upgrading_to_project_factory_v13.0.md new file mode 100644 index 00000000..8e6f4df7 --- /dev/null +++ b/docs/upgrading_to_project_factory_v13.0.md @@ -0,0 +1,32 @@ +# Upgrading to Project Factory v11.0 + +The v11.0 release of Project Factory is a backwards incompatible release. + +## Migration Instructions + +### Unused variables have been removed + +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](https://github.com/terraform-google-modules/terraform-google-project-factory/blob/master/docs/upgrading_to_project_factory_v13.0.md). + +```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). diff --git a/test/integration/dynamic_shared_vpc/controls/svpc.rb b/test/integration/dynamic_shared_vpc/controls/svpc.rb index f02c3eb3..e19ecd3a 100644 --- a/test/integration/dynamic_shared_vpc/controls/svpc.rb +++ b/test/integration/dynamic_shared_vpc/controls/svpc.rb @@ -158,6 +158,15 @@ 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( From 2d80d11916f07c5f977a6569f51c4edc0e77ce69 Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Thu, 7 Apr 2022 12:48:44 -0400 Subject: [PATCH 08/12] upgrade doc updates --- docs/upgrading_to_project_factory_v13.0.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/upgrading_to_project_factory_v13.0.md b/docs/upgrading_to_project_factory_v13.0.md index 8e6f4df7..a1afcbea 100644 --- a/docs/upgrading_to_project_factory_v13.0.md +++ b/docs/upgrading_to_project_factory_v13.0.md @@ -1,12 +1,12 @@ -# Upgrading to Project Factory v11.0 +# Upgrading to Project Factory v13.0 -The v11.0 release of Project Factory is a backwards incompatible release. +The v13.0 release of Project Factory is a backwards incompatible release. ## Migration Instructions ### Unused variables have been removed -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](https://github.com/terraform-google-modules/terraform-google-project-factory/blob/master/docs/upgrading_to_project_factory_v13.0.md). +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" { @@ -30,3 +30,7 @@ Service accounts principles on which networkUser can be managed through `grant_n - [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" From e429a871c94f81e3262b6ef06d3c3e1bb1a8254c Mon Sep 17 00:00:00 2001 From: vponnam Date: Thu, 7 Apr 2022 14:37:22 -0400 Subject: [PATCH 09/12] Update docs/upgrading_to_project_factory_v13.0.md Co-authored-by: Bharath KKB --- docs/upgrading_to_project_factory_v13.0.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/upgrading_to_project_factory_v13.0.md b/docs/upgrading_to_project_factory_v13.0.md index a1afcbea..06260395 100644 --- a/docs/upgrading_to_project_factory_v13.0.md +++ b/docs/upgrading_to_project_factory_v13.0.md @@ -4,7 +4,7 @@ The v13.0 release of Project Factory is a backwards incompatible release. ## Migration Instructions -### Unused variables have been removed +### `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 From d0d7bff66fbec5354c2d61d739a8dc52df6f9d13 Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Thu, 7 Apr 2022 14:44:23 -0400 Subject: [PATCH 10/12] update test cases scenario context --- test/integration/dynamic_shared_vpc/controls/svpc.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/dynamic_shared_vpc/controls/svpc.rb b/test/integration/dynamic_shared_vpc/controls/svpc.rb index e19ecd3a..80d7c36a 100644 --- a/test/integration/dynamic_shared_vpc/controls/svpc.rb +++ b/test/integration/dynamic_shared_vpc/controls/svpc.rb @@ -96,7 +96,7 @@ ) end - it "service project c without 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 + 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_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", From 817469080fdf9202985862f7caa31e44c0b9f0c8 Mon Sep 17 00:00:00 2001 From: Venkata Ponnam Date: Thu, 7 Apr 2022 15:06:44 -0400 Subject: [PATCH 11/12] intigration test case for default project sa --- test/integration/dynamic_shared_vpc/controls/svpc.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/integration/dynamic_shared_vpc/controls/svpc.rb b/test/integration/dynamic_shared_vpc/controls/svpc.rb index 80d7c36a..2b6358f8 100644 --- a/test/integration/dynamic_shared_vpc/controls/svpc.rb +++ b/test/integration/dynamic_shared_vpc/controls/svpc.rb @@ -148,6 +148,15 @@ 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: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( From f0ff8e842a13496edcb245593e3d8336a5b04c26 Mon Sep 17 00:00:00 2001 From: vponnam Date: Thu, 7 Apr 2022 16:23:32 -0400 Subject: [PATCH 12/12] Update test/integration/dynamic_shared_vpc/controls/svpc.rb Co-authored-by: Bharath KKB --- test/integration/dynamic_shared_vpc/controls/svpc.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/dynamic_shared_vpc/controls/svpc.rb b/test/integration/dynamic_shared_vpc/controls/svpc.rb index 2b6358f8..fa0cd28c 100644 --- a/test/integration/dynamic_shared_vpc/controls/svpc.rb +++ b/test/integration/dynamic_shared_vpc/controls/svpc.rb @@ -149,7 +149,7 @@ 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 + 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",