feat: added the grant_services_network_role flag to control network IAM #618

kaariger · 2021-09-22T21:57:28Z

Added a new flag (grant_services_network_role) to control whether service project service accounts are granted the roles/compute.networkUser on the shared VPC host project. The default value for this flag is true for backwards compatibility.

Added the flag
Added integration tests
Updated documentation

fixes #603

comment-bot-dev · 2021-09-22T22:03:40Z

Thanks for the PR! 🚀
✅ Lint checks have passed.

bharathkkb

LGTM with small nit

modules/shared_vpc_access/variables.tf

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

kaariger · 2021-09-22T23:09:06Z

I will update the PR with the changes in the description for various modules and updated docs.

sandy-0007 · 2021-09-23T18:39:23Z

@kaariger / @bharathkkb
Unfortunately, the provider has is not yet accepting this flag. I am getting this error

=========================
Error: Unsupported argument

on main.tf line 16, in module "project-factory":
16: grant_services_network_role = false

An argument named "grant_services_network_role" is not expected here.

=========================

Below is the code:

module "project-factory" {
source = "terraform-google-modules/project-factory/google"
version = "~> 11.1.1"
name = var.project_name
random_project_id = "false"
org_id = var.organization_id
billing_account = var.billing_account_id
svpc_host_project_id = var.vpc_project_id
folder_id = var.folder_id
shared_vpc_subnets = var.shared_vpcs
grant_services_network_role = false
activate_apis = module.user_globals.apis
sa_role = var.sa_role
default_service_account = "disable"
disable_services_on_destroy = "false"
}

bharathkkb · 2021-09-23T18:48:06Z

Hi @sandy-0007 it should be available in our next release. I did notice a bug which I have opened a PR to fix. Once that is merged you can temporarily try pulling from the main branch to validate if the flag is working as expected and switch to a pinned version after release.

sandy-0007 · 2021-09-23T19:56:12Z

Can I know when is the next release planned for? And thanks for fixing the bug. Once the PR #619 is merged, could you please share me the details on how I can use this branch to test my terraform code locally.

bharathkkb · 2021-09-24T04:13:08Z

You can use the main branch of the repo like shown below to test it. #620 can be tracked for release which we can cut after validation. We generally cut a release ~ 2 weeks but in this case we can do it sooner due to bugfix in #619.

module "project-factory" {
  source  = "github.com/terraform-google-modules/terraform-google-project-factory"
  
  name                 = "pf-test-1"
  random_project_id    = true
...
 }

sandy-0007 · 2021-09-24T16:52:04Z

Looks like the code from #619 is working. I have tested "grant_services_network_role", its working as desired.

Now I could see the below changes. Why is it trying to remove the installed API's?

 # module.project-factory.module.project-factory.module.gcloud_disable.null_resource.run_command[0] will be destroyed
- resource "null_resource" "run_command" {
      - id       = "xxxxxxxxxxxxxx" -> null
      - triggers = {
          - "activated_apis"          = "storage-api.googleapis.com,servicemanagement.googleapis.com,servicecontrol.googleapis.com,endpoints.googleapis.com,iam.googleapis.com,containerregistry.googleapis.com,serviceusage.googleapis.com,oslogin.googleapis.com,replicapoolupdater.googleapis.com,resourceviews.googleapis.com,storage-component.googleapis.com,stackdriver.googleapis.com,monitoring.googleapis.com,dataflow.googleapis.com,dataproc.googleapis.com,servicenetworking.googleapis.com,bigtable.googleapis.com,bigtabletableadmin.googleapis.com,bigtableadmin.googleapis.com,spanner.googleapis.com,deploymentmanager.googleapis.com,securitycenter.googleapis.com,billingbudgets.googleapis.com,cloudbilling.googleapis.com,networkmanagement.googleapis.com,iamcredentials.googleapis.com,logging.googleapis.com,cloudapis.googleapis.com,notebooks.googleapis.com,automl.googleapis.com,ml.googleapis.com,artifactregistry.googleapis.com,aiplatform.googleapis.com,datacatalog.googleapis.com,datastudio.googleapis.com"
          - "arguments"               = "e99d93327b72185c253c467ef89675a4"
          - "create_cmd_body"         = <<~EOT
                --project_id='xxxxxxxxxxx' \
                --sa_id='xxxxxx-compute@developer.gserviceaccount.com' \
                --credentials_path='' \
                --impersonate-service-account='' \
                --action='disable'
            EOT
          - "create_cmd_entrypoint"   = ".terraform/modules/project-factory/modules/core_project_factory/scripts/modify-service-account.sh"
          - "default_service_account" = "xxxxx-compute@developer.gserviceaccount.com"
          - "gcloud_bin_abs_path"     = "/google-cloud-sdk/bin"
          - "md5"                     = "xxxxxxxxxxxxxxxxxxx"
          - "project_services"        = "xxxxxxx"
        } -> null
    }

  # module.project-factory.module.project-factory.module.gcloud_disable.null_resource.run_destroy_command[0] will be destroyed
- resource "null_resource" "run_destroy_command" {
      - id       = "3141809779843838021" -> null
      - triggers = {
          - "activated_apis"          = "storage-api.googleapis.com,servicemanagement.googleapis.com,servicecontrol.googleapis.com,endpoints.googleapis.com,iam.googleapis.com,containerregistry.googleapis.com,serviceusage.googleapis.com,oslogin.googleapis.com,replicapoolupdater.googleapis.com,resourceviews.googleapis.com,storage-component.googleapis.com,stackdriver.googleapis.com,monitoring.googleapis.com,dataflow.googleapis.com,dataproc.googleapis.com,servicenetworking.googleapis.com,bigtable.googleapis.com,bigtabletableadmin.googleapis.com,bigtableadmin.googleapis.com,spanner.googleapis.com,deploymentmanager.googleapis.com,securitycenter.googleapis.com,billingbudgets.googleapis.com,cloudbilling.googleapis.com,networkmanagement.googleapis.com,iamcredentials.googleapis.com,logging.googleapis.com,cloudapis.googleapis.com,notebooks.googleapis.com,automl.googleapis.com,ml.googleapis.com,artifactregistry.googleapis.com,aiplatform.googleapis.com,datacatalog.googleapis.com,datastudio.googleapis.com"
          - "default_service_account" = "xxxxxxx-compute@developer.gserviceaccount.com"
          - "destroy_cmd_body"        = "info"
          - "destroy_cmd_entrypoint"  = "gcloud"
          - "gcloud_bin_abs_path"     = "/google-cloud-sdk/bin"
          - "project_services"        = "xxxxxxx"
        } -> null
    }

sandy-0007 · 2021-09-24T16:58:25Z

Below is the code used for this testing...

module "project-factory" {
  source                      = "github.com/terraform-google-modules/terraform-google-project-factory"
  name                        = var.project_name
  random_project_id           = "false"
  org_id                      = var.organization_id
  billing_account             = var.billing_account_id
  svpc_host_project_id        = var.vpc_project_id
  folder_id                   = var.folder_id
  shared_vpc_subnets          = var.shared_vpcs
  grant_services_network_role = false
  activate_apis               = module.user_globals.apis
  sa_role                     = var.sa_role
  default_service_account     = "disable"
  disable_services_on_destroy = "false"
  labels = merge(module.resource_labels.labels, module.gdw_resource_labels.labels)
}

sandy-0007 · 2021-09-24T18:44:21Z

@bharathkkb Since the code #619 is working as desired, you can move forward with merging it to the master. Also it has not removed any API's. So we are good here to move on with merging this.

sandy-0007 · 2021-09-28T03:45:25Z

@bharathkkb, Can #620 be merged? as I'm convinced with the functionality of #619.

feat: added the grant_services_network_role flag to control network IAM

ff0b233

kaariger requested a review from a team as a code owner September 22, 2021 21:57

bharathkkb previously approved these changes Sep 22, 2021

View reviewed changes

modules/shared_vpc_access/variables.tf Outdated Show resolved Hide resolved

Update modules/shared_vpc_access/variables.tf

aef6983

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

kaariger dismissed bharathkkb’s stale review via aef6983 September 22, 2021 23:02

kaariger and others added 2 commits September 22, 2021 17:18

Merge branch 'terraform-google-modules:master' into iambindingflags

c3e6210

updated description for grant_services_network_role flag

e58fa56

bharathkkb approved these changes Sep 23, 2021

View reviewed changes

bharathkkb merged commit f116dd5 into terraform-google-modules:master Sep 23, 2021

bharathkkb mentioned this pull request Sep 23, 2021

fix: plumb grant_services_network_role for root module #619

Merged

morgante mentioned this pull request Oct 18, 2021

Prevent Project service account getting added to the Shared project #630

Closed

This was referenced Aug 18, 2022

chore(master): release 14.0.0 #741

Closed

chore(master): release 13.1.0 #713

Closed

release-please bot mentioned this pull request Nov 17, 2022

chore(master): release 15.0.0 #769

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: added the grant_services_network_role flag to control network IAM #618

feat: added the grant_services_network_role flag to control network IAM #618

kaariger commented Sep 22, 2021

comment-bot-dev commented Sep 22, 2021 •

edited

bharathkkb left a comment

kaariger commented Sep 22, 2021

sandy-0007 commented Sep 23, 2021 •

edited

bharathkkb commented Sep 23, 2021

sandy-0007 commented Sep 23, 2021

bharathkkb commented Sep 24, 2021

sandy-0007 commented Sep 24, 2021 •

edited

sandy-0007 commented Sep 24, 2021

sandy-0007 commented Sep 24, 2021

sandy-0007 commented Sep 28, 2021

feat: added the grant_services_network_role flag to control network IAM #618

feat: added the grant_services_network_role flag to control network IAM #618

Conversation

kaariger commented Sep 22, 2021

comment-bot-dev commented Sep 22, 2021 • edited

bharathkkb left a comment

Choose a reason for hiding this comment

kaariger commented Sep 22, 2021

sandy-0007 commented Sep 23, 2021 • edited

bharathkkb commented Sep 23, 2021

sandy-0007 commented Sep 23, 2021

bharathkkb commented Sep 24, 2021

sandy-0007 commented Sep 24, 2021 • edited

sandy-0007 commented Sep 24, 2021

sandy-0007 commented Sep 24, 2021

sandy-0007 commented Sep 28, 2021

comment-bot-dev commented Sep 22, 2021 •

edited

sandy-0007 commented Sep 23, 2021 •

edited

sandy-0007 commented Sep 24, 2021 •

edited