From c50b732c79eeaebb93beb0d9a1a9cf8273e4754b Mon Sep 17 00:00:00 2001 From: simonebruzzechesse <60114646+simonebruzzechesse@users.noreply.github.com> Date: Fri, 15 Dec 2023 14:39:21 +0100 Subject: [PATCH] Allow granting network user role on host project from project module and factory (#1930) * Update shared vpc config for project factory and project module for more granular Shared VPC configuration --------- Co-authored-by: Ludovico Magnocavallo --- .../factories/project-factory/README.md | 36 +++++++---- .../factories/project-factory/factory.tf | 2 + .../factories/project-factory/variables.tf | 2 + modules/project/README.md | 59 +++++++++++++++--- modules/project/shared-vpc.tf | 35 +++++++++++ modules/project/variables.tf | 9 ++- .../project_factory/examples/example.yaml | 41 +++++++++--- .../examples/shared-vpc-host-project-iam.yaml | 62 +++++++++++++++++++ .../examples/shared-vpc-subnet-grants.yaml | 4 +- 9 files changed, 217 insertions(+), 33 deletions(-) create mode 100644 tests/modules/project/examples/shared-vpc-host-project-iam.yaml diff --git a/blueprints/factories/project-factory/README.md b/blueprints/factories/project-factory/README.md index 231ca8dc03..67081df2e2 100644 --- a/blueprints/factories/project-factory/README.md +++ b/blueprints/factories/project-factory/README.md @@ -57,7 +57,7 @@ module "project-factory" { # location where the yaml files are read from factory_data_path = "data" } -# tftest modules=7 resources=31 files=prj-app-1,prj-app-2,prj-app-3 inventory=example.yaml +# tftest modules=7 resources=33 files=prj-app-1,prj-app-2,prj-app-3 inventory=example.yaml ``` ```yaml @@ -85,9 +85,15 @@ service_accounts: ```yaml labels: - app: app-2 - team: foo + app: app-2 + team: foo parent: folders/12345678 +org_policies: + "compute.restrictSharedVpcSubnetworks": + rules: + - allow: + values: + - projects/foo-host/regions/europe-west1/subnetworks/prod-default-ew1 service_accounts: app-2-be: {} services: @@ -98,13 +104,17 @@ services: shared_vpc_service_config: host_project: foo-host service_identity_iam: - "roles/compute.networkUser": - - cloudservices - - container-engine "roles/vpcaccess.user": - - cloudrun + - cloudrun "roles/container.hostServiceAgentUser": - - container-engine + - container-engine + service_identity_subnet_iam: + europe-west1/prod-default-ew1: + - cloudservices + - container-engine + network_subnet_users: + europe-west1/prod-default-ew1: + - group:team-1@example.com # tftest-file id=prj-app-2 path=data/prj-app-2.yaml ``` @@ -117,15 +127,16 @@ services: # tftest-file id=prj-app-3 path=data/prj-app-3.yaml ``` + ## Variables | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [factory_data_path](variables.tf#L89) | Path to folder with YAML project description data files. | string | ✓ | | -| [data_defaults](variables.tf#L17) | Optional default values used when corresponding project data from files are missing. | object({…}) | | {} | -| [data_merges](variables.tf#L47) | Optional values that will be merged with corresponding data from files. Combines with `data_defaults`, file data, and `data_overrides`. | object({…}) | | {} | -| [data_overrides](variables.tf#L67) | Optional values that override corresponding data from files. Takes precedence over file data and `data_defaults`. | object({…}) | | {} | +| [factory_data_path](variables.tf#L91) | Path to folder with YAML project description data files. | string | ✓ | | +| [data_defaults](variables.tf#L17) | Optional default values used when corresponding project data from files are missing. | object({…}) | | {} | +| [data_merges](variables.tf#L49) | Optional values that will be merged with corresponding data from files. Combines with `data_defaults`, file data, and `data_overrides`. | object({…}) | | {} | +| [data_overrides](variables.tf#L69) | Optional values that override corresponding data from files. Takes precedence over file data and `data_defaults`. | object({…}) | | {} | ## Outputs @@ -134,6 +145,7 @@ services: | [projects](outputs.tf#L17) | Project module outputs. | | | [service_accounts](outputs.tf#L22) | Service account emails. | | + ## Tests These tests validate fixes to the project factory. diff --git a/blueprints/factories/project-factory/factory.tf b/blueprints/factories/project-factory/factory.tf index b8da24a49e..4028186caf 100644 --- a/blueprints/factories/project-factory/factory.tf +++ b/blueprints/factories/project-factory/factory.tf @@ -79,9 +79,11 @@ locals { try(v.shared_vpc_service_config, null) != null ? merge( { + network_users = [] service_identity_iam = {} service_identity_subnet_iam = {} service_iam_grants = [] + network_subnet_users = {} }, v.shared_vpc_service_config ) diff --git a/blueprints/factories/project-factory/variables.tf b/blueprints/factories/project-factory/variables.tf index 18b315d6d8..d37f939928 100644 --- a/blueprints/factories/project-factory/variables.tf +++ b/blueprints/factories/project-factory/variables.tf @@ -29,9 +29,11 @@ variable "data_defaults" { services = optional(list(string), []) shared_vpc_service_config = optional(object({ host_project = string + network_users = optional(list(string), []) service_identity_iam = optional(map(list(string)), {}) service_identity_subnet_iam = optional(map(list(string)), {}) service_iam_grants = optional(list(string), []) + network_subnet_users = optional(map(list(string)), {}) }), { host_project = null }) tag_bindings = optional(map(string), {}) # non-project resources diff --git a/modules/project/README.md b/modules/project/README.md index fb08566244..3f51113a6b 100644 --- a/modules/project/README.md +++ b/modules/project/README.md @@ -233,7 +233,7 @@ The module allows managing Shared VPC status for both hosts and service projects Project service association for VPC host projects can be - authoritatively managed in the host project by enabling Shared VPC and specifying the set of service projects, or -- additively managed in service projects by by enabling Shared VPC in the host project and then "attaching" each service project independently +- additively managed in service projects by enabling Shared VPC in the host project and then "attaching" each service project independently IAM bindings in the host project for API service identities can be managed from service projects in two different ways: @@ -317,9 +317,51 @@ module "service-project" { # tftest modules=2 resources=9 inventory=shared-vpc-auto-grants.yaml e2e ``` -In specific cases it might make sense to selectively grant the `compute.networkUser` role for service identities at the subnet level, and while that is best done via org policies it's also supported by this module. +The `compute.networkUser` role for identities other than API services (e.g. users, groups or service accounts) can be managed via the `network_users` attribute, by specifying the list of identities. Avoid using dynamically generated lists, as this attribute is involved in a `for_each` loop and may result in Terraform errors. -In this example, Compute service identity will be granted compute.networkUser in the `gce` subnet defined in `europe-west1` region. +Note that this configuration grants the role at project level which results in the identities being able to configure resources on all the VPCs and subnets belonging to the host project. The most reliable way to restrict which subnets can be used on the newly created project is via the `compute.restrictSharedVpcSubnetworks` organization policy. For more information on the Org Policy configuration check the corresponding [Organization Policy section](#organization-policies). The following example details this configuration. + +```hcl +module "host-project" { + source = "./fabric/modules/project" + billing_account = var.billing_account_id + name = "host" + parent = var.folder_id + prefix = var.prefix + shared_vpc_host_config = { + enabled = true + } +} + +module "service-project" { + source = "./fabric/modules/project" + billing_account = var.billing_account_id + name = "service" + parent = var.folder_id + prefix = var.prefix + org_policies = { + "compute.restrictSharedVpcSubnetworks" = { + rules = [{ + allow = { + values = ["projects/host/regions/europe-west1/subnetworks/prod-default-ew1"] + } + }] + } + } + services = [ + "container.googleapis.com", + ] + shared_vpc_service_config = { + host_project = module.host-project.project_id + network_users = ["group:team-1@example.com"] + # reuse the list of services from the module's outputs + service_iam_grants = module.service-project.services + } +} +# tftest modules=2 resources=11 inventory=shared-vpc-host-project-iam.yaml e2e +``` + +In specific cases it might make sense to selectively grant the `compute.networkUser` role for service identities at the subnet level, and while that is best done via org policies it's also supported by this module. In this example, Compute service identity and `team-1@example.com` Google Group will be granted compute.networkUser in the `gce` subnet defined in `europe-west1` region via the `service_identity_subnet_iam` and `network_subnet_users` attributes. ```hcl module "host-project" { @@ -347,9 +389,12 @@ module "service-project" { service_identity_subnet_iam = { "europe-west1/gce" = ["compute"] } + network_subnet_users = { + "europe-west1/gce" = ["group:team-1@example.com"] + } } } -# tftest modules=2 resources=6 inventory=shared-vpc-subnet-grants.yaml +# tftest modules=2 resources=7 inventory=shared-vpc-subnet-grants.yaml ``` ## Organization Policies @@ -929,9 +974,9 @@ module "bucket" { | [service_perimeter_standard](variables.tf#L280) | Name of VPC-SC Standard perimeter to add project into. See comment in the variables file for format. | string | | null | | [services](variables.tf#L286) | Service APIs to enable. | list(string) | | [] | | [shared_vpc_host_config](variables.tf#L292) | Configures this project as a Shared VPC host project (mutually exclusive with shared_vpc_service_project). | object({…}) | | null | -| [shared_vpc_service_config](variables.tf#L301) | Configures this project as a Shared VPC service project (mutually exclusive with shared_vpc_host_config). | object({…}) | | {…} | -| [skip_delete](variables.tf#L324) | Allows the underlying resources to be destroyed without destroying the project itself. | bool | | false | -| [tag_bindings](variables.tf#L330) | Tag bindings for this project, in key => tag value id format. | map(string) | | null | +| [shared_vpc_service_config](variables.tf#L301) | Configures this project as a Shared VPC service project (mutually exclusive with shared_vpc_host_config). | object({…}) | | {…} | +| [skip_delete](variables.tf#L329) | Allows the underlying resources to be destroyed without destroying the project itself. | bool | | false | +| [tag_bindings](variables.tf#L335) | Tag bindings for this project, in key => tag value id format. | map(string) | | null | ## Outputs diff --git a/modules/project/shared-vpc.tf b/modules/project/shared-vpc.tf index 91a56a90dc..925b5aebb8 100644 --- a/modules/project/shared-vpc.tf +++ b/modules/project/shared-vpc.tf @@ -68,6 +68,24 @@ locals { for v in local._svpc_service_subnet_iam : "${v.region}:${v.subnet}:${v.service}" => v } + # normalize the network user subnet IAM binding + _svpc_network_user_subnet_iam = ( + local._svpc.network_subnet_users == null || local._svpc.host_project == null + ? [] + : flatten([ + for subnet, members in local._svpc.network_subnet_users : [ + for member in members : { + region = split("/", subnet)[0] + subnet = split("/", subnet)[1] + member = member + } + ] + ]) + ) + svpc_network_user_subnet_iam = { + for v in local._svpc_network_user_subnet_iam : + "${v.region}:${v.subnet}:${v.member}" => v + } } resource "google_compute_shared_vpc_host_project" "shared_vpc_host" { @@ -111,6 +129,14 @@ resource "google_project_iam_member" "shared_vpc_host_robots" { ] } +resource "google_project_iam_member" "shared_vpc_host_iam" { + for_each = toset(var.shared_vpc_service_config.network_users) + project = var.shared_vpc_service_config.host_project + role = "roles/compute.networkUser" + member = each.value + depends_on = [] +} + resource "google_compute_subnetwork_iam_member" "shared_vpc_host_robots" { for_each = local.svpc_service_subnet_iam project = var.shared_vpc_service_config.host_project @@ -131,3 +157,12 @@ resource "google_compute_subnetwork_iam_member" "shared_vpc_host_robots" { data.google_storage_project_service_account.gcs_sa, ] } + +resource "google_compute_subnetwork_iam_member" "shared_vpc_host_subnets_iam" { + for_each = local.svpc_network_user_subnet_iam + project = var.shared_vpc_service_config.host_project + region = each.value.region + subnetwork = each.value.subnet + role = "roles/compute.networkUser" + member = each.value.member +} diff --git a/modules/project/variables.tf b/modules/project/variables.tf index 7ca6003606..064dd275d0 100644 --- a/modules/project/variables.tf +++ b/modules/project/variables.tf @@ -303,9 +303,11 @@ variable "shared_vpc_service_config" { # the list of valid service identities is in service-agents.yaml type = object({ host_project = string + network_users = optional(list(string), []) service_identity_iam = optional(map(list(string)), {}) service_identity_subnet_iam = optional(map(list(string)), {}) service_iam_grants = optional(list(string), []) + network_subnet_users = optional(map(list(string)), {}) }) default = { host_project = null @@ -314,10 +316,13 @@ variable "shared_vpc_service_config" { validation { condition = var.shared_vpc_service_config.host_project != null || ( var.shared_vpc_service_config.host_project == null && + length(var.shared_vpc_service_config.network_users) == 0 && length(var.shared_vpc_service_config.service_iam_grants) == 0 && - length(var.shared_vpc_service_config.service_iam_grants) == 0 + length(var.shared_vpc_service_config.service_identity_iam) == 0 && + length(var.shared_vpc_service_config.service_identity_subnet_iam) == 0 && + length(var.shared_vpc_service_config.network_subnet_users) == 0 ) - error_message = "You need to provide host_project when providing service_identity_iam or service_iam_grants" + error_message = "You need to provide host_project when providing Shared VPC host and subnet IAM permissions." } } diff --git a/tests/blueprints/factories/project_factory/examples/example.yaml b/tests/blueprints/factories/project_factory/examples/example.yaml index 4e636946eb..5595a729f4 100644 --- a/tests/blueprints/factories/project_factory/examples/example.yaml +++ b/tests/blueprints/factories/project_factory/examples/example.yaml @@ -102,14 +102,6 @@ values: environment: test team: foo timeouts: null - ? module.project-factory.module.projects["prj-app-2"].google_project_iam_member.shared_vpc_host_robots["roles/compute.networkUser:cloudservices"] - : condition: [] - project: foo-host - role: roles/compute.networkUser - ? module.project-factory.module.projects["prj-app-2"].google_project_iam_member.shared_vpc_host_robots["roles/compute.networkUser:container-engine"] - : condition: [] - project: foo-host - role: roles/compute.networkUser ? module.project-factory.module.projects["prj-app-2"].google_project_iam_member.shared_vpc_host_robots["roles/container.hostServiceAgentUser:container-engine"] : condition: [] project: foo-host @@ -118,6 +110,18 @@ values: : condition: [] project: foo-host role: roles/vpcaccess.user + ? module.project-factory.module.projects["prj-app-2"].google_compute_subnetwork_iam_member.shared_vpc_host_robots["europe-west1:prod-default-ew1:cloudservices"] + : condition: [ ] + project: foo-host + role: roles/compute.networkUser + ? module.project-factory.module.projects["prj-app-2"].google_compute_subnetwork_iam_member.shared_vpc_host_robots["europe-west1:prod-default-ew1:container-engine"] + : condition: [ ] + project: foo-host + role: roles/compute.networkUser + ? module.project-factory.module.projects["prj-app-2"].google_compute_subnetwork_iam_member.shared_vpc_host_subnets_iam["europe-west1:prod-default-ew1:group:team-1@example.com"] + : condition: [ ] + project: foo-host + role: roles/compute.networkUser module.project-factory.module.projects["prj-app-2"].google_project_service.project_services["compute.googleapis.com"]: disable_dependent_services: false disable_on_destroy: false @@ -148,6 +152,21 @@ values: project: test-pf-prj-app-2 service: storage.googleapis.com timeouts: null + module.project-factory.module.projects["prj-app-2"].google_org_policy_policy.default["compute.restrictSharedVpcSubnetworks"]: + name: projects/test-pf-prj-app-2/policies/compute.restrictSharedVpcSubnetworks + parent: projects/test-pf-prj-app-2 + spec: + - inherit_from_parent: null + reset: null + rules: + - allow_all: null + condition: [ ] + deny_all: null + enforce: null + values: + - allowed_values: + - projects/foo-host/regions/europe-west1/subnetworks/prod-default-ew1 + denied_values: null module.project-factory.module.projects["prj-app-3"].data.google_storage_project_service_account.gcs_sa[0]: project: test-pf-prj-app-3 user_project: null @@ -223,14 +242,16 @@ values: counts: google_compute_shared_vpc_service_project: 1 + google_compute_subnetwork_iam_member: 3 google_essential_contacts_contact: 3 google_kms_crypto_key_iam_member: 1 google_project: 3 - google_project_iam_member: 6 + google_project_iam_member: 4 google_project_service: 11 google_service_account: 3 google_storage_project_service_account: 3 + google_org_policy_policy: 1 modules: 7 - resources: 31 + resources: 33 outputs: {} diff --git a/tests/modules/project/examples/shared-vpc-host-project-iam.yaml b/tests/modules/project/examples/shared-vpc-host-project-iam.yaml new file mode 100644 index 0000000000..7a864d9442 --- /dev/null +++ b/tests/modules/project/examples/shared-vpc-host-project-iam.yaml @@ -0,0 +1,62 @@ +# Copyright 2023 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +values: + module.host-project.google_compute_shared_vpc_host_project.shared_vpc_host[0]: + project: test-host + module.host-project.google_project.project[0]: + project_id: test-host + module.service-project.google_compute_shared_vpc_service_project.shared_vpc_service[0]: + host_project: test-host + service_project: test-service + module.service-project.google_project.project[0]: + project_id: test-service + module.service-project.google_project_iam_member.shared_vpc_host_robots["roles/compute.networkUser:cloudservices"]: + condition: [] + project: test-host + role: roles/compute.networkUser + module.service-project.google_project_iam_member.shared_vpc_host_robots["roles/compute.networkUser:container"]: + condition: [] + project: test-host + role: roles/compute.networkUser + module.service-project.google_project_iam_member.shared_vpc_host_robots["roles/container.hostServiceAgentUser:container"]: + condition: [] + project: test-host + role: roles/container.hostServiceAgentUser + module.service-project.google_project_iam_member.shared_vpc_host_iam["group:team-1@example.com"]: + condition: [ ] + project: test-host + role: roles/compute.networkUser + module.service-project.google_org_policy_policy.default["compute.restrictSharedVpcSubnetworks"]: + name: projects/test-service/policies/compute.restrictSharedVpcSubnetworks + parent: projects/test-service + spec: + - inherit_from_parent: null + reset: null + rules: + - allow_all: null + condition: [ ] + deny_all: null + enforce: null + values: + - allowed_values: + - projects/host/regions/europe-west1/subnetworks/prod-default-ew1 + denied_values: null + +counts: + google_compute_shared_vpc_host_project: 1 + google_compute_shared_vpc_service_project: 1 + google_project: 2 + google_project_iam_member: 5 + google_org_policy_policy: 1 diff --git a/tests/modules/project/examples/shared-vpc-subnet-grants.yaml b/tests/modules/project/examples/shared-vpc-subnet-grants.yaml index f245615fe3..077abf1a0e 100644 --- a/tests/modules/project/examples/shared-vpc-subnet-grants.yaml +++ b/tests/modules/project/examples/shared-vpc-subnet-grants.yaml @@ -57,10 +57,10 @@ values: counts: google_compute_shared_vpc_host_project: 1 google_compute_shared_vpc_service_project: 1 - google_compute_subnetwork_iam_member: 1 + google_compute_subnetwork_iam_member: 2 google_project: 2 google_project_service: 1 modules: 2 - resources: 6 + resources: 7 outputs: {}