From 44693fb6dc788dac4d873268b13f72d643389d48 Mon Sep 17 00:00:00 2001 From: David Gleich Date: Tue, 12 Apr 2022 14:10:14 +0200 Subject: [PATCH 01/15] Allow to use an existing monitoring project and remove some unused TF variables --- .../network-dashboard/main.tf | 22 ++++++++------- .../network-dashboard/variables.tf | 28 ++++++------------- 2 files changed, 21 insertions(+), 29 deletions(-) diff --git a/examples/cloud-operations/network-dashboard/main.tf b/examples/cloud-operations/network-dashboard/main.tf index 6378c95d..23ef3e2f 100644 --- a/examples/cloud-operations/network-dashboard/main.tf +++ b/examples/cloud-operations/network-dashboard/main.tf @@ -15,8 +15,9 @@ */ locals { - project_id_list = toset(var.monitored_projects_list) - projects = join(",", local.project_id_list) + project_id_list = toset(var.monitored_projects_list) + projects = join(",", local.project_id_list) + monitoring_project = var.monitoring_project_id == "" ? module.project-monitoring.project_id : var.monitoring_project_id } ################################################ @@ -24,6 +25,7 @@ locals { ################################################ module "project-monitoring" { + count = var.monitoring_project_id == "" ? 1 : 0 source = "../../../modules/project" name = "monitoring" parent = "organizations/${var.organization_id}" @@ -38,7 +40,7 @@ module "project-monitoring" { module "service-account-function" { source = "../../../modules/iam-service-account" - project_id = module.project-monitoring.project_id + project_id = local.monitoring_project name = "sa-dash" generate_key = false @@ -54,7 +56,7 @@ module "service-account-function" { } iam_project_roles = { - "${module.project-monitoring.project_id}" = [ + "${local.monitoring_project}" = [ "roles/monitoring.metricWriter" ] } @@ -66,7 +68,7 @@ module "service-account-function" { module "pubsub" { source = "../../../modules/pubsub" - project_id = module.project-monitoring.project_id + project_id = local.monitoring_project name = "network-dashboard-pubsub" subscriptions = { "network-dashboard-pubsub-default" = null @@ -76,7 +78,7 @@ module "pubsub" { } resource "google_cloud_scheduler_job" "job" { - project = module.project-monitoring.project_id + project = local.monitoring_project region = var.region name = "network-dashboard-scheduler" schedule = var.schedule_cron @@ -90,9 +92,9 @@ resource "google_cloud_scheduler_job" "job" { module "cloud-function" { source = "../../../modules/cloud-function" - project_id = module.project-monitoring.project_id + project_id = local.monitoring_project name = "network-dashboard-cloud-function" - bucket_name = "network-dashboard-bucket" + bucket_name = "${local.monitoring_project}-network-dashboard-bucket" bucket_config = { location = var.region lifecycle_delete_age = null @@ -114,7 +116,7 @@ module "cloud-function" { environment_variables = { MONITORED_PROJECTS_LIST = local.projects - MONITORING_PROJECT_ID = module.project-monitoring.project_id + MONITORING_PROJECT_ID = local.monitoring_project ORGANIZATION_ID = var.organization_id } @@ -133,5 +135,5 @@ module "cloud-function" { resource "google_monitoring_dashboard" "dashboard" { dashboard_json = file("${path.module}/dashboards/quotas-utilization.json") - project = module.project-monitoring.project_id + project = local.monitoring_project } \ No newline at end of file diff --git a/examples/cloud-operations/network-dashboard/variables.tf b/examples/cloud-operations/network-dashboard/variables.tf index 7e4237ca..d7c5eda5 100644 --- a/examples/cloud-operations/network-dashboard/variables.tf +++ b/examples/cloud-operations/network-dashboard/variables.tf @@ -22,8 +22,14 @@ variable "billing_account" { description = "The ID of the billing account to associate this project with" } +variable "monitoring_project_id" { + description = "Monitoring project where the dashboard will be created and the solution deployed; a project will be created if set to empty string" + default = "" +} + variable "prefix" { - description = "Customer name to use as prefix for resources' naming" + description = "Customer name to use as prefix for monitoring project" + default = "" } # TODO: support folder instead of a list of projects? @@ -38,7 +44,7 @@ variable "schedule_cron" { } variable "project_monitoring_services" { - description = "Service APIs enabled by default in new projects." + description = "Service APIs enabled in the monitoring project if it will be created." default = [ "cloudbilling.googleapis.com", "cloudbuild.googleapis.com", @@ -56,23 +62,7 @@ variable "project_monitoring_services" { ] } -variable "project_vm_services" { - description = "Service APIs enabled by default in new projects." - default = [ - "cloudbilling.googleapis.com", - "compute.googleapis.com", - "logging.googleapis.com", - "monitoring.googleapis.com", - "servicenetworking.googleapis.com", - ] -} - variable "region" { - description = "Region used to deploy subnets" + description = "Region used to deploy the cloud functions and scheduler" default = "europe-west1" -} - -variable "zone" { - description = "Zone used to deploy vms" - default = "europe-west1-b" } \ No newline at end of file From 2a6a890058fd56a4d56405e372a5f515edbcd2ad Mon Sep 17 00:00:00 2001 From: David Gleich Date: Tue, 12 Apr 2022 14:13:05 +0200 Subject: [PATCH 02/15] Catch errors due to permission denied for Peering Groups --- .../network-dashboard/cloud-function/main.py | 92 ++++++++++++++----- 1 file changed, 70 insertions(+), 22 deletions(-) diff --git a/examples/cloud-operations/network-dashboard/cloud-function/main.py b/examples/cloud-operations/network-dashboard/cloud-function/main.py index e5763b37..cc3d172c 100644 --- a/examples/cloud-operations/network-dashboard/cloud-function/main.py +++ b/examples/cloud-operations/network-dashboard/cloud-function/main.py @@ -18,13 +18,14 @@ from code import interact import os from pickletools import int4 import time +import http import yaml from collections import defaultdict from google.api import metric_pb2 as ga_metric -from google.api_core import protobuf_helpers +from google.api_core import exceptions, protobuf_helpers from google.cloud import monitoring_v3, asset_v1 from google.protobuf import field_mask_pb2 -from googleapiclient import discovery +from googleapiclient import discovery, errors # Organization ID containing the projects to be monitored ORGANIZATION_ID = os.environ.get("ORGANIZATION_ID") @@ -366,6 +367,9 @@ def get_gce_instances_data(metrics_dict, gce_instance_dict, limit_dict): current_quota_limit = get_quota_current_limit(f"projects/{project}", metric_instances_limit) + if current_quota_limit is None: + print(f"Could not write number of instances for projects/{project} due to missing quotas") + current_quota_limit_view = customize_quota_view(current_quota_limit) for net in network_dict: @@ -519,6 +523,10 @@ def get_l4_forwarding_rules_data(metrics_dict, forwarding_rules_dict, current_quota_limit = get_quota_current_limit( f"projects/{project}", L4_FORWARDING_RULES_LIMIT_METRIC) + if current_quota_limit is None: + print(f"Could not write L4 forwarding rules to metric for projects/{project} due to missing quotas") + continue + current_quota_limit_view = customize_quota_view(current_quota_limit) for net in network_dict: @@ -564,9 +572,21 @@ def get_pgg_data(metric_dict, usage_dict, limit_metric, limit_dict): # project_id, network_name, network_id, usage, limit, peerings (list of peered networks) # peerings is a list of dictionary (one for each peered network) and contains: # project_id, network_name, network_id + current_quota_limit = get_quota_current_limit(f"projects/{project}", + limit_metric) + if current_quota_limit is None: + print(f"Could not write number of L7 forwarding rules to metric for projects/{project} due to missing quotas") + continue + + current_quota_limit_view = customize_quota_view(current_quota_limit) # For each network in this GCP project for network_dict in network_dict_list: + if network_dict['network_id'] == 0: + print( + f"Could not write {metric_dict['usage']['name']} for peering group {network_dict['network_name']} in {project} due to missing permissions." + ) + continue network_link = f"https://www.googleapis.com/compute/v1/projects/{project}/global/networks/{network_dict['network_name']}" current_quota_limit = get_quota_current_limit(f"projects/{project}", @@ -590,9 +610,13 @@ def get_pgg_data(metric_dict, usage_dict, limit_metric, limit_dict): if peered_network_link in usage_dict: peered_usage = usage_dict[peered_network_link] - peering_project_limit = customize_quota_view( - get_quota_current_limit( - f"projects/{peered_network_dict['project_id']}", limit_metric)) + current_peered_quota_limit = get_quota_current_limit( + f"projects/{peered_network_dict['project_id']}", limit_metric) + if current_peered_quota_limit is None: + print(f"Could not write metrics for peering to projects/{peered_network_dict['project_id']} due to missing quotas") + continue + + peering_project_limit = customize_quota_view(current_peered_quota_limit) peered_limit = get_limit_network(peered_network_dict, peered_network_link, @@ -681,6 +705,9 @@ def count_effective_limit(project_id, network_dict, usage_metric_name, # Get usage: Sums usage for current network + all peered networks peering_group_usage = network_dict['usage'] for peered_network in network_dict['peerings']: + if 'usage' not in peered_network: + print(f"Can not add metrics for peered network in projects/{project_id} as no usage metrics exist due to missing permissions") + continue peering_group_usage += peered_network['usage'] network_link = f"https://www.googleapis.com/compute/v1/projects/{project_id}/global/networks/{network_dict['network_name']}" @@ -694,12 +721,18 @@ def count_effective_limit(project_id, network_dict, usage_metric_name, for peered_network in network_dict['peerings']: peered_network_link = f"https://www.googleapis.com/compute/v1/projects/{peered_network['project_id']}/global/networks/{peered_network['network_name']}" - limit_step2.append( - max(peered_network['limit'], - get_limit_ppg(peered_network_link, limit_dict))) + if 'limit' in peered_network: + limit_step2.append( + max(peered_network['limit'], + get_limit_ppg(peered_network_link, limit_dict))) + else: + print(f"Ignoring projects/{peered_network['project_id']} for limits in peering group of project {project_id} as no limits are available." + + "This can happen due to the project belonging to a different organization") # Calculates effective limit: Step 3: Find minimum from the list created by Step 2 - limit_step3 = min(limit_step2) + limit_step3 = 0 + if len(limit_step2) > 0: + limit_step3 = min(limit_step2) # Calculates effective limit: Step 4: Find maximum from step 1 and step 3 effective_limit = max(limit_step1, limit_step3) @@ -837,8 +870,9 @@ def get_routes_for_router(project_id, router_region, router_name): sum_routes = 0 if 'result' in response: - for peer in response['result']['bgpPeerStatus']: - sum_routes += peer['numLearnedRoutes'] + if 'bgpPeerStatus' in response['result']: + for peer in response['result']['bgpPeerStatus']: + sum_routes += peer['numLearnedRoutes'] return sum_routes @@ -939,7 +973,17 @@ def get_network_id(project_id, network_name): network_id (int): Network ID. ''' request = service.networks().list(project=project_id) - response = request.execute() + try: + response = request.execute() + except errors.HttpError as err: + # TODO: log proper warning + if err.resp.status == http.HTTPStatus.FORBIDDEN: + print(f"Warning: error reading networks for {project_id}. " + + f"This can happen if this project is not belonging to you organization") + else: + print(f"Warning: error reading networks for {project_id}: {err}") + return 0 + network_id = 0 @@ -967,16 +1011,20 @@ def get_quota_current_limit(project_link, metric_name): ''' client, interval = create_client() - results = client.list_time_series( - request={ - "name": project_link, - "filter": f'metric.type = "{metric_name}"', - "interval": interval, - "view": monitoring_v3.ListTimeSeriesRequest.TimeSeriesView.FULL - }) - results_list = list(results) - return results_list - + try: + results = client.list_time_series( + request={ + "name": project_link, + "filter": f'metric.type = "{metric_name}"', + "interval": interval, + "view": monitoring_v3.ListTimeSeriesRequest.TimeSeriesView.FULL + }) + results_list = list(results) + return results_list + except exceptions.PermissionDenied as err: + print(f"Warning: error reading quotas for {project_link}. " + + f"This can happen if this project is not belonging to you organization: {err}") + return None def customize_quota_view(quota_results): ''' From 8743d6101be6316ae658eff946da73f1f34e5ea3 Mon Sep 17 00:00:00 2001 From: David Gleich Date: Tue, 12 Apr 2022 14:21:14 +0200 Subject: [PATCH 03/15] Add metrics for L7 forwarding groups --- .../network-dashboard/cloud-function/main.py | 53 +++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/examples/cloud-operations/network-dashboard/cloud-function/main.py b/examples/cloud-operations/network-dashboard/cloud-function/main.py index cc3d172c..da6bdf0b 100644 --- a/examples/cloud-operations/network-dashboard/cloud-function/main.py +++ b/examples/cloud-operations/network-dashboard/cloud-function/main.py @@ -67,6 +67,9 @@ def main(event, context): get_l4_forwarding_rules_data( metrics_dict, l4_forwarding_rules_dict, limits_dict['internal_forwarding_rules_l4_limit']) + get_l7_forwarding_rules_data( + metrics_dict, l7_forwarding_rules_dict, + limits_dict['internal_forwarding_rules_l7_limit']) get_vpc_peering_data(metrics_dict, limits_dict['number_of_vpc_peerings_limit']) dynamic_routes_dict = get_dynamic_routes( @@ -552,6 +555,53 @@ def get_l4_forwarding_rules_data(metrics_dict, forwarding_rules_dict, print( f"Wrote number of L4 forwarding rules to metric for projects/{project}") +def get_l7_forwarding_rules_data(metrics_dict, forwarding_rules_dict, + limit_dict): + ''' + Gets the data for L7 Internal Forwarding Rules per VPC Network and writes it to the metric defined in forwarding_rules_metric. + + Parameters: + metrics_dict (dictionary of dictionary of string: string): metrics names and descriptions. + forwarding_rules_dict (dictionary of string: int): Keys are the network links and values are the number of Forwarding Rules per network. + limit_dict (dictionary of string:int): Dictionary with the network link as key and the limit as value. + Returns: + None + ''' + for project in MONITORED_PROJECTS_LIST: + network_dict = get_networks(project) + + current_quota_limit = get_quota_current_limit( + f"projects/{project}", L4_FORWARDING_RULES_LIMIT_METRIC) + if current_quota_limit is None: + print(f"Could not write number of L7 forwarding rules to metric for projects/{project} due to missing quotas") + continue + + + current_quota_limit_view = customize_quota_view(current_quota_limit) + + for net in network_dict: + set_limits(net, current_quota_limit_view, limit_dict) + + usage = 0 + if net['self_link'] in forwarding_rules_dict: + usage = forwarding_rules_dict[net['self_link']] + + write_data_to_metric( + project, usage, metrics_dict["metrics_per_network"] + ["l7_forwarding_rules_per_network"]["usage"]["name"], + net['network_name']) + write_data_to_metric( + project, net['limit'], metrics_dict["metrics_per_network"] + ["l7_forwarding_rules_per_network"]["limit"]["name"], + net['network_name']) + write_data_to_metric( + project, usage / net['limit'], metrics_dict["metrics_per_network"] + ["l7_forwarding_rules_per_network"]["utilization"]["name"], + net['network_name']) + + print( + f"Wrote number of L7 forwarding rules to metric for projects/{project}") + def get_pgg_data(metric_dict, usage_dict, limit_metric, limit_dict): ''' @@ -589,9 +639,6 @@ def get_pgg_data(metric_dict, usage_dict, limit_metric, limit_dict): continue network_link = f"https://www.googleapis.com/compute/v1/projects/{project}/global/networks/{network_dict['network_name']}" - current_quota_limit = get_quota_current_limit(f"projects/{project}", - limit_metric) - current_quota_limit_view = customize_quota_view(current_quota_limit) limit = get_limit_network(network_dict, network_link, current_quota_limit_view, limit_dict) From ea6f31e0249b9b71e76edb3846be568d8a60d69b Mon Sep 17 00:00:00 2001 From: David Gleich Date: Tue, 12 Apr 2022 15:31:42 +0200 Subject: [PATCH 04/15] Findings from code review --- .../network-dashboard/cloud-function/main.py | 8 ++++---- examples/cloud-operations/network-dashboard/main.tf | 2 +- examples/cloud-operations/network-dashboard/variables.tf | 3 +-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/examples/cloud-operations/network-dashboard/cloud-function/main.py b/examples/cloud-operations/network-dashboard/cloud-function/main.py index da6bdf0b..593d7b60 100644 --- a/examples/cloud-operations/network-dashboard/cloud-function/main.py +++ b/examples/cloud-operations/network-dashboard/cloud-function/main.py @@ -571,7 +571,7 @@ def get_l7_forwarding_rules_data(metrics_dict, forwarding_rules_dict, network_dict = get_networks(project) current_quota_limit = get_quota_current_limit( - f"projects/{project}", L4_FORWARDING_RULES_LIMIT_METRIC) + f"projects/{project}", L7_FORWARDING_RULES_LIMIT_METRIC) if current_quota_limit is None: print(f"Could not write number of L7 forwarding rules to metric for projects/{project} due to missing quotas") continue @@ -774,7 +774,7 @@ def count_effective_limit(project_id, network_dict, usage_metric_name, get_limit_ppg(peered_network_link, limit_dict))) else: print(f"Ignoring projects/{peered_network['project_id']} for limits in peering group of project {project_id} as no limits are available." + - "This can happen due to the project belonging to a different organization") + "This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project") # Calculates effective limit: Step 3: Find minimum from the list created by Step 2 limit_step3 = 0 @@ -1026,7 +1026,7 @@ def get_network_id(project_id, network_name): # TODO: log proper warning if err.resp.status == http.HTTPStatus.FORBIDDEN: print(f"Warning: error reading networks for {project_id}. " + - f"This can happen if this project is not belonging to you organization") + f"This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project") else: print(f"Warning: error reading networks for {project_id}: {err}") return 0 @@ -1070,7 +1070,7 @@ def get_quota_current_limit(project_link, metric_name): return results_list except exceptions.PermissionDenied as err: print(f"Warning: error reading quotas for {project_link}. " + - f"This can happen if this project is not belonging to you organization: {err}") + f"This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project") return None def customize_quota_view(quota_results): diff --git a/examples/cloud-operations/network-dashboard/main.tf b/examples/cloud-operations/network-dashboard/main.tf index 23ef3e2f..2102b370 100644 --- a/examples/cloud-operations/network-dashboard/main.tf +++ b/examples/cloud-operations/network-dashboard/main.tf @@ -17,7 +17,7 @@ locals { project_id_list = toset(var.monitored_projects_list) projects = join(",", local.project_id_list) - monitoring_project = var.monitoring_project_id == "" ? module.project-monitoring.project_id : var.monitoring_project_id + monitoring_project = var.monitoring_project_id == "" ? module.project-monitoring[0].project_id : var.monitoring_project_id } ################################################ diff --git a/examples/cloud-operations/network-dashboard/variables.tf b/examples/cloud-operations/network-dashboard/variables.tf index d7c5eda5..9d69469e 100644 --- a/examples/cloud-operations/network-dashboard/variables.tf +++ b/examples/cloud-operations/network-dashboard/variables.tf @@ -46,6 +46,7 @@ variable "schedule_cron" { variable "project_monitoring_services" { description = "Service APIs enabled in the monitoring project if it will be created." default = [ + "cloudasset.googleapis.com", "cloudbilling.googleapis.com", "cloudbuild.googleapis.com", "cloudresourcemanager.googleapis.com", @@ -56,8 +57,6 @@ variable "project_monitoring_services" { "iamcredentials.googleapis.com", "logging.googleapis.com", "monitoring.googleapis.com", - "oslogin.googleapis.com", - "servicenetworking.googleapis.com", "serviceusage.googleapis.com", ] } From c7c77541fbcb3a405ec785f545d935b87f69f089 Mon Sep 17 00:00:00 2001 From: Lorenzo Caggioni Date: Tue, 12 Apr 2022 19:01:34 +0200 Subject: [PATCH 05/15] Add KMS on CloudSQL module --- modules/cloudsql-instance/README.md | 53 ++++++++++++++++++++++++++ modules/cloudsql-instance/main.tf | 16 +++++--- modules/cloudsql-instance/outputs.tf | 13 +++++++ modules/cloudsql-instance/variables.tf | 15 ++++++-- modules/project/service-accounts.tf | 6 ++- 5 files changed, 92 insertions(+), 11 deletions(-) diff --git a/modules/cloudsql-instance/README.md b/modules/cloudsql-instance/README.md index 213398d7..d5c050c8 100644 --- a/modules/cloudsql-instance/README.md +++ b/modules/cloudsql-instance/README.md @@ -93,6 +93,59 @@ module "db" { } # tftest modules=1 resources=6 ``` + +### CMEK encryption pippo +```hcl + +module "project" { + source = "./modules/project" + billing_account = var.billing_account_id + parent = var.organization_id + name = "my-db-project" + services = [ + "servicenetworking.googleapis.com" + ] +} + +resource "google_project_service_identity" "jit_si" { + provider = google-beta + project = module.project.project_id + service = "sqladmin.googleapis.com" +} + +module "kms" { + source = "./modules/kms" + project_id = module.project.project_id + keyring = { + name = "keyring" + location = var.region + } + keys = { + key-sql = null + } + key_iam = { + key-sql = { + "roles/cloudkms.cryptoKeyEncrypterDecrypter" = [ + "serviceAccount:${google_project_service_identity.jit_si.email}" + ] + } + } +} + +module "db" { + source = "./modules/cloudsql-instance" + project_id = module.project.project_id + encryption_key_name = module.kms.keys["key-sql"].id + network = var.vpc.self_link + name = "db" + region = var.region + database_version = "POSTGRES_13" + tier = "db-g1-small" +} + +# tftest modules=3 resources=8 +``` + ## Variables diff --git a/modules/cloudsql-instance/main.tf b/modules/cloudsql-instance/main.tf index f50121ab..d9c86378 100644 --- a/modules/cloudsql-instance/main.tf +++ b/modules/cloudsql-instance/main.tf @@ -39,10 +39,12 @@ locals { } resource "google_sql_database_instance" "primary" { - project = var.project_id - name = "${local.prefix}${var.name}" - region = var.region - database_version = var.database_version + provider = google-beta + project = var.project_id + name = "${local.prefix}${var.name}" + region = var.region + database_version = var.database_version + encryption_key_name = var.encryption_key_name settings { tier = var.tier @@ -99,11 +101,13 @@ resource "google_sql_database_instance" "primary" { } resource "google_sql_database_instance" "replicas" { - for_each = local.has_replicas ? var.replicas : {} + provider = google-beta + for_each = length(var.replicas) > 0 ? var.replicas : {} project = var.project_id name = "${local.prefix}${each.key}" - region = each.value + region = each.value.region database_version = var.database_version + encryption_key_name = each.value.encryption_key_name master_instance_name = google_sql_database_instance.primary.name settings { diff --git a/modules/cloudsql-instance/outputs.tf b/modules/cloudsql-instance/outputs.tf index e2ca316d..38bfc951 100644 --- a/modules/cloudsql-instance/outputs.tf +++ b/modules/cloudsql-instance/outputs.tf @@ -66,6 +66,19 @@ output "ips" { } } +output "name" { + description = "Name of the primary instance." + value = google_sql_database_instance.primary.name +} + +output "names" { + description = "Names of all instances." + value = { + for id, instance in local._all_intances : + id => instance.name + } +} + output "self_link" { description = "Self link of the primary instance." value = google_sql_database_instance.primary.self_link diff --git a/modules/cloudsql-instance/variables.tf b/modules/cloudsql-instance/variables.tf index e59736ba..00130045 100644 --- a/modules/cloudsql-instance/variables.tf +++ b/modules/cloudsql-instance/variables.tf @@ -76,6 +76,12 @@ variable "disk_type" { default = "PD_SSD" } +variable "encryption_key_name" { + description = "The full path to the encryption key used for the CMEK disk encryption." + type = string + default = null +} + variable "flags" { description = "Map FLAG_NAME=>VALUE for database-specific tuning." type = map(string) @@ -115,9 +121,12 @@ variable "region" { } variable "replicas" { - description = "Map of NAME=>REGION for additional read replicas. Set to null to disable replica creation." - type = map(any) - default = null + description = "Map of NAME=> {REGION, KMS_KEY} for additional read replicas. Set to null to disable replica creation." + type = map(object({ + region = string + encryption_key_name = string + })) + default = {} } variable "tier" { diff --git a/modules/project/service-accounts.tf b/modules/project/service-accounts.tf index eae98e23..f5cffed4 100644 --- a/modules/project/service-accounts.tf +++ b/modules/project/service-accounts.tf @@ -42,6 +42,7 @@ locals { gcf = "service-%s@gcf-admin-robot" pubsub = "service-%s@gcp-sa-pubsub" secretmanager = "service-%s@gcp-sa-secretmanager" + sql = "service-%s@gcp-sa-cloud-sql" storage = "service-%s@gs-project-accounts" } service_accounts_default = { @@ -56,9 +57,10 @@ locals { k => "${format(v, local.project.number)}.iam.gserviceaccount.com" } service_accounts_jit_services = [ - "secretmanager.googleapis.com", + "cloudasset.googleapis.com", "pubsub.googleapis.com", - "cloudasset.googleapis.com" + "secretmanager.googleapis.com", + "sqladmin.googleapis.com" ] service_accounts_cmek_service_keys = distinct(flatten([ for s in keys(var.service_encryption_key_ids) : [ From 0ea0fa622bb6762b2e80689cbfbb8dd8dfd1442c Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Tue, 12 Apr 2022 21:03:20 +0200 Subject: [PATCH 06/15] Move FAST tests to main tests workflow --- .github/workflows/fast-tests.yml | 55 -------------------------------- .github/workflows/tests.yml | 45 ++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 61 deletions(-) delete mode 100644 .github/workflows/fast-tests.yml diff --git a/.github/workflows/fast-tests.yml b/.github/workflows/fast-tests.yml deleted file mode 100644 index f8524766..00000000 --- a/.github/workflows/fast-tests.yml +++ /dev/null @@ -1,55 +0,0 @@ -# Copyright 2022 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. - -name: "FAST Tests" -on: - pull_request: - branches: - - fast-dev - - fast-dev-gke - - master - # paths: - # - 'modules/**' - # - 'fast/stages/**' - # - 'tests/fast/**' - tags: - - ci - - test - -env: - TF_PLUGIN_CACHE_DIR: "/home/runner/.terraform.d/plugin-cache" - GOOGLE_APPLICATION_CREDENTIALS: "/home/runner/credentials.json" - PYTEST_ADDOPTS: "--color=yes" - -jobs: - all-fast-tests: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - - name: Config auth - run: | - echo '{"type": "service_account", "project_id": "test-only"}' \ - | tee -a $GOOGLE_APPLICATION_CREDENTIALS - - - name: Set up Python - uses: actions/setup-python@v2 - with: - python-version: "3.9" - - - name: Run tests on FAST stages - run: | - mkdir -p ${{ env.TF_PLUGIN_CACHE_DIR }} - pip install -r tests/requirements.txt - pytest -vv tests/fast diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5375865f..9057b9f3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -26,9 +26,11 @@ on: - test env: - TF_PLUGIN_CACHE_DIR: "/home/runner/.terraform.d/plugin-cache" GOOGLE_APPLICATION_CREDENTIALS: "/home/runner/credentials.json" PYTEST_ADDOPTS: "--color=yes" + PYTHON_VERSION: 3.9 + TF_PLUGIN_CACHE_DIR: "/home/runner/.terraform.d/plugin-cache" + TF_VERSION: 1.1.8 jobs: doc-examples: @@ -44,9 +46,10 @@ jobs: - name: Set up Python uses: actions/setup-python@v2 with: - python-version: "3.9" + python-version: ${{ env.PYTHON_VERSION }} - name: Run tests on documentation examples + id: pytest run: | mkdir -p ${{ env.TF_PLUGIN_CACHE_DIR }} pip install -r tests/requirements.txt @@ -65,15 +68,16 @@ jobs: - name: Set up Python uses: actions/setup-python@v2 with: - python-version: "3.9" + python-version: ${{ env.PYTHON_VERSION }} - name: Set up Terraform uses: hashicorp/setup-terraform@v1 with: - terraform_version: 1.1.4 + terraform_version: ${{ env.TF_VERSION }} terraform_wrapper: false - name: Run tests environments + id: pytest run: | mkdir -p ${{ env.TF_PLUGIN_CACHE_DIR }} pip install -r tests/requirements.txt @@ -92,16 +96,45 @@ jobs: - name: Set up Python uses: actions/setup-python@v2 with: - python-version: "3.9" + python-version: ${{ env.PYTHON_VERSION }} - name: Set up Terraform uses: hashicorp/setup-terraform@v1 with: - terraform_version: 1.1.4 + terraform_version: ${{ env.TF_VERSION }} terraform_wrapper: false - name: Run tests modules + id: pytest run: | mkdir -p ${{ env.TF_PLUGIN_CACHE_DIR }} pip install -r tests/requirements.txt pytest -vv tests/modules + + fast: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - name: Config auth + run: | + echo '{"type": "service_account", "project_id": "test-only"}' \ + | tee -a $GOOGLE_APPLICATION_CREDENTIALS + + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Set up Terraform + uses: hashicorp/setup-terraform@v1 + with: + terraform_version: ${{ env.TF_VERSION }} + terraform_wrapper: false + + - name: Run tests on FAST stages + id: pytest + run: | + mkdir -p ${{ env.TF_PLUGIN_CACHE_DIR }} + pip install -r tests/requirements.txt + pytest -vv tests/fast From 2644627837b40073c44930120780551da1ecc10c Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Tue, 12 Apr 2022 21:28:52 +0200 Subject: [PATCH 07/15] Remove broken link and ignore globals in fast stages --- .gitignore | 1 + fast/stages/01-resman/globals.auto.tfvars.json | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 120000 fast/stages/01-resman/globals.auto.tfvars.json diff --git a/.gitignore b/.gitignore index af6a2d21..373948e7 100644 --- a/.gitignore +++ b/.gitignore @@ -27,3 +27,4 @@ fast/stages/**/terraform.tfvars.json fast/stages/**/terraform-*.auto.tfvars.json fast/stages/**/0*.auto.tfvars* **/node_modules +fast/stages/**/globals.auto.tfvars.json diff --git a/fast/stages/01-resman/globals.auto.tfvars.json b/fast/stages/01-resman/globals.auto.tfvars.json deleted file mode 120000 index 29e127f3..00000000 --- a/fast/stages/01-resman/globals.auto.tfvars.json +++ /dev/null @@ -1 +0,0 @@ -/home/ludomagno/Desktop/dev/tf-playground/config/tfvars/globals.auto.tfvars.json \ No newline at end of file From 1f254880b961d8cdd49e4249b15dce87ca5f7937 Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Wed, 13 Apr 2022 00:22:54 +0200 Subject: [PATCH 08/15] Add sqladmin to project jit_si and fix some documentation --- modules/cloudsql-instance/README.md | 47 +++++++++---------- modules/cloudsql-instance/main.tf | 2 +- modules/cloudsql-instance/variables.tf | 6 +-- modules/project/service-accounts.tf | 1 + .../cloudsql_instance/fixture/variables.tf | 2 +- tests/modules/cloudsql_instance/test_plan.py | 13 +++-- 6 files changed, 34 insertions(+), 37 deletions(-) diff --git a/modules/cloudsql-instance/README.md b/modules/cloudsql-instance/README.md index d5c050c8..75a7af0b 100644 --- a/modules/cloudsql-instance/README.md +++ b/modules/cloudsql-instance/README.md @@ -56,8 +56,8 @@ module "db" { tier = "db-g1-small" replicas = { - replica1 = "europe-west3" - replica2 = "us-central1" + replica1 = { region = "europe-west3", encryption_key_name = null } + replica2 = { region = "us-central1", encryption_key_name = null } } } # tftest modules=1 resources=3 @@ -103,16 +103,11 @@ module "project" { parent = var.organization_id name = "my-db-project" services = [ - "servicenetworking.googleapis.com" + "servicenetworking.googleapis.com", + "sqladmin.googleapis.com", ] } -resource "google_project_service_identity" "jit_si" { - provider = google-beta - project = module.project.project_id - service = "sqladmin.googleapis.com" -} - module "kms" { source = "./modules/kms" project_id = module.project.project_id @@ -126,7 +121,7 @@ module "kms" { key_iam = { key-sql = { "roles/cloudkms.cryptoKeyEncrypterDecrypter" = [ - "serviceAccount:${google_project_service_identity.jit_si.email}" + "serviceAccount:${module.project.service_accounts.robots.sqladmin}" ] } } @@ -143,9 +138,8 @@ module "db" { tier = "db-g1-small" } -# tftest modules=3 resources=8 +# tftest modules=3 resources=10 ``` - ## Variables @@ -153,11 +147,11 @@ module "db" { | name | description | type | required | default | |---|---|:---:|:---:|:---:| | [database_version](variables.tf#L50) | Database type and version to create. | string | ✓ | | -| [name](variables.tf#L91) | Name of primary replica. | string | ✓ | | -| [network](variables.tf#L96) | VPC self link where the instances will be deployed. Private Service Networking must be enabled and configured in this VPC. | string | ✓ | | -| [project_id](variables.tf#L107) | The ID of the project where this instances will be created. | string | ✓ | | -| [region](variables.tf#L112) | Region of the primary replica. | string | ✓ | | -| [tier](variables.tf#L123) | The machine type to use for the instances. | string | ✓ | | +| [name](variables.tf#L97) | Name of primary instance. | string | ✓ | | +| [network](variables.tf#L102) | VPC self link where the instances will be deployed. Private Service Networking must be enabled and configured in this VPC. | string | ✓ | | +| [project_id](variables.tf#L113) | The ID of the project where this instances will be created. | string | ✓ | | +| [region](variables.tf#L118) | Region of the primary instance. | string | ✓ | | +| [tier](variables.tf#L132) | The machine type to use for the instances. | string | ✓ | | | [authorized_networks](variables.tf#L17) | Map of NAME=>CIDR_RANGE to allow to connect to the database(s). | map(string) | | null | | [availability_type](variables.tf#L23) | Availability type for the primary replica. Either `ZONAL` or `REGIONAL`. | string | | "ZONAL" | | [backup_configuration](variables.tf#L29) | Backup settings for primary instance. Will be automatically enabled if using MySQL with one or more replicas. | object({…}) | | {…} | @@ -165,11 +159,12 @@ module "db" { | [deletion_protection](variables.tf#L61) | Allow terraform to delete instances. | bool | | false | | [disk_size](variables.tf#L67) | Disk size in GB. Set to null to enable autoresize. | number | | null | | [disk_type](variables.tf#L73) | The type of data disk: `PD_SSD` or `PD_HDD`. | string | | "PD_SSD" | -| [flags](variables.tf#L79) | Map FLAG_NAME=>VALUE for database-specific tuning. | map(string) | | null | -| [labels](variables.tf#L85) | Labels to be attached to all instances. | map(string) | | null | -| [prefix](variables.tf#L101) | Prefix used to generate instance names. | string | | null | -| [replicas](variables.tf#L117) | Map of NAME=>REGION for additional read replicas. Set to null to disable replica creation. | map(any) | | null | -| [users](variables.tf#L128) | Map of users to create in the primary instance (and replicated to other replicas) in the format USER=>PASSWORD. For MySQL, anything afterr the first `@` (if persent) will be used as the user's host. Set PASSWORD to null if you want to get an autogenerated password. | map(string) | | null | +| [encryption_key_name](variables.tf#L79) | The full path to the encryption key used for the CMEK disk encryption of the primary instance. | string | | null | +| [flags](variables.tf#L85) | Map FLAG_NAME=>VALUE for database-specific tuning. | map(string) | | null | +| [labels](variables.tf#L91) | Labels to be attached to all instances. | map(string) | | null | +| [prefix](variables.tf#L107) | Prefix used to generate instance names. | string | | null | +| [replicas](variables.tf#L123) | Map of NAME=> {REGION, KMS_KEY} for additional read replicas. Set to null to disable replica creation. | map(object({…})) | | {} | +| [users](variables.tf#L137) | Map of users to create in the primary instance (and replicated to other replicas) in the format USER=>PASSWORD. For MySQL, anything afterr the first `@` (if persent) will be used as the user's host. Set PASSWORD to null if you want to get an autogenerated password. | map(string) | | null | ## Outputs @@ -182,8 +177,10 @@ module "db" { | [instances](outputs.tf#L50) | Cloud SQL instance resources. | ✓ | | [ip](outputs.tf#L56) | IP address of the primary instance. | | | [ips](outputs.tf#L61) | IP addresses of all instances. | | -| [self_link](outputs.tf#L69) | Self link of the primary instance. | | -| [self_links](outputs.tf#L74) | Self links of all instances. | | -| [user_passwords](outputs.tf#L82) | Map of containing the password of all users created through terraform. | ✓ | +| [name](outputs.tf#L69) | Name of the primary instance. | | +| [names](outputs.tf#L74) | Names of all instances. | | +| [self_link](outputs.tf#L82) | Self link of the primary instance. | | +| [self_links](outputs.tf#L87) | Self links of all instances. | | +| [user_passwords](outputs.tf#L95) | Map of containing the password of all users created through terraform. | ✓ | diff --git a/modules/cloudsql-instance/main.tf b/modules/cloudsql-instance/main.tf index d9c86378..be46b59c 100644 --- a/modules/cloudsql-instance/main.tf +++ b/modules/cloudsql-instance/main.tf @@ -102,7 +102,7 @@ resource "google_sql_database_instance" "primary" { resource "google_sql_database_instance" "replicas" { provider = google-beta - for_each = length(var.replicas) > 0 ? var.replicas : {} + for_each = local.has_replicas ? var.replicas : {} project = var.project_id name = "${local.prefix}${each.key}" region = each.value.region diff --git a/modules/cloudsql-instance/variables.tf b/modules/cloudsql-instance/variables.tf index 00130045..30acd915 100644 --- a/modules/cloudsql-instance/variables.tf +++ b/modules/cloudsql-instance/variables.tf @@ -77,7 +77,7 @@ variable "disk_type" { } variable "encryption_key_name" { - description = "The full path to the encryption key used for the CMEK disk encryption." + description = "The full path to the encryption key used for the CMEK disk encryption of the primary instance." type = string default = null } @@ -95,7 +95,7 @@ variable "labels" { } variable "name" { - description = "Name of primary replica." + description = "Name of primary instance." type = string } @@ -116,7 +116,7 @@ variable "project_id" { } variable "region" { - description = "Region of the primary replica." + description = "Region of the primary instance." type = string } diff --git a/modules/project/service-accounts.tf b/modules/project/service-accounts.tf index f5cffed4..5ba40fc4 100644 --- a/modules/project/service-accounts.tf +++ b/modules/project/service-accounts.tf @@ -44,6 +44,7 @@ locals { secretmanager = "service-%s@gcp-sa-secretmanager" sql = "service-%s@gcp-sa-cloud-sql" storage = "service-%s@gs-project-accounts" + sqladmin = "service-%s@gcp-sa-cloud-sql" } service_accounts_default = { compute = "${local.project.number}-compute@developer.gserviceaccount.com" diff --git a/tests/modules/cloudsql_instance/fixture/variables.tf b/tests/modules/cloudsql_instance/fixture/variables.tf index 62ff27a3..d6cc7d83 100644 --- a/tests/modules/cloudsql_instance/fixture/variables.tf +++ b/tests/modules/cloudsql_instance/fixture/variables.tf @@ -94,7 +94,7 @@ variable "region" { } variable "replicas" { - type = map(any) + type = any default = null } diff --git a/tests/modules/cloudsql_instance/test_plan.py b/tests/modules/cloudsql_instance/test_plan.py index 3c45f5c6..c4e8ba0a 100644 --- a/tests/modules/cloudsql_instance/test_plan.py +++ b/tests/modules/cloudsql_instance/test_plan.py @@ -35,8 +35,8 @@ def test_prefix(plan_runner): assert r['values']['name'] == 'prefix-db' replicas = """{ - replica1 = "europe-west3" - replica2 = "us-central1" + replica1 = { region = "europe-west3", encryption_key_name = null } + replica2 = { region = "us-central1", encryption_key_name = null } }""" _, resources = plan_runner(prefix="prefix") @@ -49,8 +49,8 @@ def test_replicas(plan_runner): "Test replicated instance." replicas = """{ - replica1 = "europe-west3" - replica2 = "us-central1" + replica1 = { region = "europe-west3", encryption_key_name = null } + replica2 = { region = "us-central1", encryption_key_name = null } }""" _, resources = plan_runner(replicas=replicas, prefix="prefix") @@ -80,10 +80,9 @@ def test_mysql_replicas_enables_backup(plan_runner): "Test MySQL backup setup with replicas." replicas = """{ - replica1 = "europe-west3" + replica1 = { region = "europe-west3", encryption_key_name = null } }""" - _, resources = plan_runner(replicas=replicas, - database_version="MYSQL_8_0") + _, resources = plan_runner(replicas=replicas, database_version="MYSQL_8_0") assert len(resources) == 2 primary = [r for r in resources if r['name'] == 'primary'][0] backup_config = primary['values']['settings'][0]['backup_configuration'][0] From b415d824c962ac0c86ae36de3109813dac970109 Mon Sep 17 00:00:00 2001 From: Lorenzo Caggioni Date: Wed, 13 Apr 2022 08:59:14 +0200 Subject: [PATCH 09/15] Fix README, bye bye pippo :-) --- modules/cloudsql-instance/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cloudsql-instance/README.md b/modules/cloudsql-instance/README.md index 75a7af0b..73b2e3f2 100644 --- a/modules/cloudsql-instance/README.md +++ b/modules/cloudsql-instance/README.md @@ -94,7 +94,7 @@ module "db" { # tftest modules=1 resources=6 ``` -### CMEK encryption pippo +### CMEK encryption ```hcl module "project" { From 53c9431c985daeae02dbf19a5e07fd0140af2c31 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Wed, 13 Apr 2022 09:05:28 +0200 Subject: [PATCH 10/15] Improve project module README (#627) * improve module README * Fix a few typos Co-authored-by: Julio Castillo --- modules/project/README.md | 116 +++++++++++++++++++++++++++++++++++--- 1 file changed, 109 insertions(+), 7 deletions(-) diff --git a/modules/project/README.md b/modules/project/README.md index 42d435d9..035aee41 100644 --- a/modules/project/README.md +++ b/modules/project/README.md @@ -1,8 +1,19 @@ # Project Module -## Examples +This module implements the creation and management of one GCP project including IAM, organization policies, Shared VPC host or service attachment, service API activation, and tag attachment. It also offers a convenient way to refer to managed service identities (aka robot service accounts) for APIs. -### Minimal example with IAM +## IAM Examples + +IAM is controlled via several variables that implement different levels of control: + +- `group_iam` and `iam` configure authoritative bindings that manage individual roles exclusively, mapping to the [`google_project_iam_binding`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam#google_project_iam_binding) resource +- `iam_additive` and `iam_additive_members` configure additive bindings that only manage individual role/member pairs, mapping to the [`google_project_iam_member`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam#google_project_iam_member) resource + +Be mindful about the service identity roles when using authoritative IAM, as you might end up inadvertently removing a role from a [service identity](https://cloud.google.com/iam/docs/service-accounts#google-managed) or default service account. For example, using `roles/editor` with `iam` or `group_iam` will remove the default permissions for the Cloud Services identity. A simple workaround for these scenarios is described below. + +### Authoritative IAM + +The `iam` variable is based on role keys and is typically used for service accounts, or where member values can be dynamic and would create potential problems in the underlying `for_each` cycle. ```hcl locals { @@ -28,16 +39,43 @@ module "project" { # tftest modules=1 resources=4 ``` -### Minimal example with IAM additive roles +The `group_iam` variable uses group email addresses as keys and is a convenient way to assign roles to humans following Google's best practices. The end result is readable code that also serves as documentation. + +```hcl +module "project" { + source = "./modules/project" + billing_account = "123456-123456-123456" + name = "project-example" + parent = "folders/1234567890" + prefix = "foo" + services = [ + "container.googleapis.com", + "stackdriver.googleapis.com" + ] + group_iam = { + "gcp-security-admins@example.com" = [ + "roles/cloudasset.owner", + "roles/cloudsupport.techSupportEditor", + "roles/iam.securityReviewer", + "roles/logging.admin", + ] + } +} +# tftest modules=1 resources=7 +``` + +### Additive IAM + +Additive IAM is typically used where bindings for specific roles are controlled by different modules or in different Terraform stages. One example is when the project is created by one team but a different team manages service account creation for the project, and some of the project-level roles overlap in the two configurations. ```hcl module "project" { source = "./modules/project" name = "project-example" - iam_additive = { "roles/viewer" = [ - "group:one@example.org", "group:two@xample.org" + "group:one@example.org", + "group:two@xample.org" ], "roles/storage.objectAdmin" = [ "group:two@example.org" @@ -50,13 +88,54 @@ module "project" { # tftest modules=1 resources=5 ``` -### Shared VPC service +### Service Identities and authoritative IAM + +As mentioned above, there are cases where authoritative management of specific IAM roles results in removal of default bindings from service identities. One example is outlined below, with a simple workaround leveraging the `service_accounts` output to identify the service identity. A full list of service identities and their roles can be found [here](https://cloud.google.com/iam/docs/service-agents). ```hcl module "project" { source = "./modules/project" name = "project-example" + group_iam = { + "roles/editor" = [ + "group:foo@example.com" + ] + } + iam = { + "roles/editor" = [ + "serviceAccount:${module.project.service_accounts.cloud_services}" + ] + } +} +# tftest modules=1 resources=3 +``` +## Shared VPC service + +The module allows managing Shared VPC status for both hosts and service projects, and includes a simple way of assigning Shared VPC roles to service identities. + +### Host project + +You can enable Shared VPC Host at the project level and manage project service association independently. + +```hcl +module "project" { + source = "./modules/project" + name = "project-example" + shared_vpc_host_config = { + enabled = true + service_projects = [] + } +} +# tftest modules=1 resources=2 +``` + +### Service project + +```hcl +module "project" { + source = "./modules/project" + name = "project-example" shared_vpc_service_config = { attach = true host_project = "my-host-project" @@ -76,7 +155,7 @@ module "project" { # tftest modules=1 resources=6 ``` -### Organization policies +## Organization policies ```hcl module "project" { @@ -184,6 +263,8 @@ module "project-host" { ## Cloud KMS encryption keys +The module offers a simple, centralized way to assign `roles/cloudkms.cryptoKeyEncrypterDecrypter` to service identities. + ```hcl module "project" { source = "./modules/project" @@ -238,6 +319,27 @@ module "project" { # tftest modules=2 resources=6 ``` +## Outputs + +Most of this module's outputs depend on its resources, to allow Terraform to compute all dependencies required for the project to be correctly configured. This allows you to reference outputs like `project_id` in other modules or resources without having to worry about setting `depends_on` blocks manually. + +One non-obvious output is `service_accounts`, which offers simple way to discover service identities and default service accounts, and guarantees that service identities that require an API call to trigger creation (like GCS or BigQuery) exist before use. + +```hcl +module "project" { + source = "./modules/project" + name = "project-example" + services = [ + "compute.googleapis.com" + ] +} + +output "compute_robot" { + value = module.project.service_accounts.robots.compute +} +# tftest modules=1 resources=2 +``` + From 874e9a57c70a78874287c72877af67ae9adcac39 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Wed, 13 Apr 2022 09:06:18 +0200 Subject: [PATCH 11/15] Update README.md --- modules/project/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/project/README.md b/modules/project/README.md index 035aee41..3e00fde3 100644 --- a/modules/project/README.md +++ b/modules/project/README.md @@ -4,7 +4,7 @@ This module implements the creation and management of one GCP project including ## IAM Examples -IAM is controlled via several variables that implement different levels of control: +IAM is managed via several variables that implement different levels of control: - `group_iam` and `iam` configure authoritative bindings that manage individual roles exclusively, mapping to the [`google_project_iam_binding`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam#google_project_iam_binding) resource - `iam_additive` and `iam_additive_members` configure additive bindings that only manage individual role/member pairs, mapping to the [`google_project_iam_member`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam#google_project_iam_member) resource From 4e1d06fd755ad09bd647f28c0aa5bbaaf4534940 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Wed, 13 Apr 2022 09:08:01 +0200 Subject: [PATCH 12/15] Update README.md --- modules/project/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/project/README.md b/modules/project/README.md index 3e00fde3..1f098cf6 100644 --- a/modules/project/README.md +++ b/modules/project/README.md @@ -9,7 +9,7 @@ IAM is managed via several variables that implement different levels of control: - `group_iam` and `iam` configure authoritative bindings that manage individual roles exclusively, mapping to the [`google_project_iam_binding`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam#google_project_iam_binding) resource - `iam_additive` and `iam_additive_members` configure additive bindings that only manage individual role/member pairs, mapping to the [`google_project_iam_member`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam#google_project_iam_member) resource -Be mindful about the service identity roles when using authoritative IAM, as you might end up inadvertently removing a role from a [service identity](https://cloud.google.com/iam/docs/service-accounts#google-managed) or default service account. For example, using `roles/editor` with `iam` or `group_iam` will remove the default permissions for the Cloud Services identity. A simple workaround for these scenarios is described below. +Be mindful about service identity roles when using authoritative IAM, as you might inadvertently remove a role from a [service identity](https://cloud.google.com/iam/docs/service-accounts#google-managed) or default service account. For example, using `roles/editor` with `iam` or `group_iam` will remove the default permissions for the Cloud Services identity. A simple workaround for these scenarios is described below. ### Authoritative IAM From 420c4de437ab60846702d359a7220ea27bb7defd Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Wed, 13 Apr 2022 09:10:56 +0200 Subject: [PATCH 13/15] Update README.md --- modules/project/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/project/README.md b/modules/project/README.md index 1f098cf6..2d1abb08 100644 --- a/modules/project/README.md +++ b/modules/project/README.md @@ -323,7 +323,7 @@ module "project" { Most of this module's outputs depend on its resources, to allow Terraform to compute all dependencies required for the project to be correctly configured. This allows you to reference outputs like `project_id` in other modules or resources without having to worry about setting `depends_on` blocks manually. -One non-obvious output is `service_accounts`, which offers simple way to discover service identities and default service accounts, and guarantees that service identities that require an API call to trigger creation (like GCS or BigQuery) exist before use. +One non-obvious output is `service_accounts`, which offers a simple way to discover service identities and default service accounts, and guarantees that service identities that require an API call to trigger creation (like GCS or BigQuery) exist before use. ```hcl module "project" { From eec0fd2fdf300f24001999e9b0801ab8a5da2912 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Wed, 13 Apr 2022 10:22:33 +0200 Subject: [PATCH 14/15] FAST: allow changing tag names from variables in resman (#628) --- fast/stages/01-resman/README.md | 21 ++++++++++--------- fast/stages/01-resman/branch-data-platform.tf | 12 ++++++++--- fast/stages/01-resman/branch-networking.tf | 12 ++++++++--- fast/stages/01-resman/branch-sandbox.tf | 4 +++- fast/stages/01-resman/branch-security.tf | 4 +++- fast/stages/01-resman/branch-teams.tf | 12 ++++++++--- fast/stages/01-resman/organization.tf | 8 +++---- fast/stages/01-resman/outputs.tf | 1 + fast/stages/01-resman/variables.tf | 17 +++++++++++++++ 9 files changed, 66 insertions(+), 25 deletions(-) diff --git a/fast/stages/01-resman/README.md b/fast/stages/01-resman/README.md index 30a86830..e3f07c67 100644 --- a/fast/stages/01-resman/README.md +++ b/fast/stages/01-resman/README.md @@ -183,20 +183,21 @@ Due to its simplicity, this stage lends itself easily to customizations: adding | [groups](variables.tf#L118) | Group names to grant organization-level permissions. | map(string) | | {…} | 00-bootstrap | | [organization_policy_configs](variables.tf#L143) | Organization policies customization. | object({…}) | | null | | | [outputs_location](variables.tf#L151) | Enable writing provider, tfvars and CI/CD workflow files to local filesystem. Leave null to disable | string | | null | | -| [team_folders](variables.tf#L168) | Team folders to be created. Format is described in a code comment. | map(object({…})) | | null | | +| [tag_names](variables.tf#L168) | Customized names for resource management tags. | object({…}) | | {…} | | +| [team_folders](variables.tf#L185) | Team folders to be created. Format is described in a code comment. | map(object({…})) | | null | | ## Outputs | name | description | sensitive | consumers | |---|---|:---:|---| -| [cicd_repositories](outputs.tf#L156) | WIF configuration for CI/CD repositories. | | | -| [dataplatform](outputs.tf#L168) | Data for the Data Platform stage. | | | -| [networking](outputs.tf#L184) | Data for the networking stage. | | | -| [project_factories](outputs.tf#L193) | Data for the project factories stage. | | | -| [providers](outputs.tf#L209) | Terraform provider files for this stage and dependent stages. | ✓ | 02-networking · 02-security · 03-dataplatform · xx-sandbox · xx-teams | -| [sandbox](outputs.tf#L216) | Data for the sandbox stage. | | xx-sandbox | -| [security](outputs.tf#L226) | Data for the networking stage. | | 02-security | -| [teams](outputs.tf#L236) | Data for the teams stage. | | | -| [tfvars](outputs.tf#L249) | Terraform variable files for the following stages. | ✓ | | +| [cicd_repositories](outputs.tf#L157) | WIF configuration for CI/CD repositories. | | | +| [dataplatform](outputs.tf#L169) | Data for the Data Platform stage. | | | +| [networking](outputs.tf#L185) | Data for the networking stage. | | | +| [project_factories](outputs.tf#L194) | Data for the project factories stage. | | | +| [providers](outputs.tf#L210) | Terraform provider files for this stage and dependent stages. | ✓ | 02-networking · 02-security · 03-dataplatform · xx-sandbox · xx-teams | +| [sandbox](outputs.tf#L217) | Data for the sandbox stage. | | xx-sandbox | +| [security](outputs.tf#L227) | Data for the networking stage. | | 02-security | +| [teams](outputs.tf#L237) | Data for the teams stage. | | | +| [tfvars](outputs.tf#L250) | Terraform variable files for the following stages. | ✓ | | diff --git a/fast/stages/01-resman/branch-data-platform.tf b/fast/stages/01-resman/branch-data-platform.tf index d518c9c1..c5e186ae 100644 --- a/fast/stages/01-resman/branch-data-platform.tf +++ b/fast/stages/01-resman/branch-data-platform.tf @@ -21,7 +21,9 @@ module "branch-dp-folder" { parent = "organizations/${var.organization.id}" name = "Data Platform" tag_bindings = { - context = try(module.organization.tag_values["context/data"].id, null) + context = try( + module.organization.tag_values["${var.tag_names.context}/data"].id, null + ) } } @@ -39,7 +41,9 @@ module "branch-dp-dev-folder" { "roles/resourcemanager.projectCreator" = [module.branch-dp-dev-sa.iam_email] } tag_bindings = { - context = try(module.organization.tag_values["environment/development"].id, null) + context = try( + module.organization.tag_values["${var.tag_names.environment}/development"].id, null + ) } } @@ -57,7 +61,9 @@ module "branch-dp-prod-folder" { "roles/resourcemanager.projectCreator" = [module.branch-dp-prod-sa.iam_email] } tag_bindings = { - context = try(module.organization.tag_values["environment/production"].id, null) + context = try( + module.organization.tag_values["${var.tag_names.environment}/production"].id, null + ) } } diff --git a/fast/stages/01-resman/branch-networking.tf b/fast/stages/01-resman/branch-networking.tf index 3d85f1be..5cf3c6e0 100644 --- a/fast/stages/01-resman/branch-networking.tf +++ b/fast/stages/01-resman/branch-networking.tf @@ -39,7 +39,9 @@ module "branch-network-folder" { "roles/compute.xpnAdmin" = [module.branch-network-sa.iam_email] } tag_bindings = { - context = try(module.organization.tag_values["context/networking"].id, null) + context = try( + module.organization.tag_values["${var.tag_names.context}/networking"].id, null + ) } } @@ -54,7 +56,9 @@ module "branch-network-prod-folder" { ] } tag_bindings = { - environment = try(module.organization.tag_values["environment/production"].id, null) + environment = try( + module.organization.tag_values["${var.tag_names.environment}/production"].id, null + ) } } @@ -69,7 +73,9 @@ module "branch-network-dev-folder" { ] } tag_bindings = { - environment = try(module.organization.tag_values["environment/development"].id, null) + environment = try( + module.organization.tag_values["${var.tag_names.environment}/development"].id, null + ) } } diff --git a/fast/stages/01-resman/branch-sandbox.tf b/fast/stages/01-resman/branch-sandbox.tf index dda4b1fc..f2ba0bfb 100644 --- a/fast/stages/01-resman/branch-sandbox.tf +++ b/fast/stages/01-resman/branch-sandbox.tf @@ -38,7 +38,9 @@ module "branch-sandbox-folder" { } } tag_bindings = { - context = try(module.organization.tag_values["context/sandbox"].id, null) + context = try( + module.organization.tag_values["${var.tag_names.context}/sandbox"].id, null + ) } } diff --git a/fast/stages/01-resman/branch-security.tf b/fast/stages/01-resman/branch-security.tf index bba54b6c..c2067304 100644 --- a/fast/stages/01-resman/branch-security.tf +++ b/fast/stages/01-resman/branch-security.tf @@ -40,7 +40,9 @@ module "branch-security-folder" { "roles/resourcemanager.projectCreator" = [module.branch-security-sa.iam_email] } tag_bindings = { - context = try(module.organization.tag_values["context/security"].id, null) + context = try( + module.organization.tag_values["${var.tag_names.context}/security"].id, null + ) } } diff --git a/fast/stages/01-resman/branch-teams.tf b/fast/stages/01-resman/branch-teams.tf index a5a16c76..124301d5 100644 --- a/fast/stages/01-resman/branch-teams.tf +++ b/fast/stages/01-resman/branch-teams.tf @@ -21,7 +21,9 @@ module "branch-teams-folder" { parent = "organizations/${var.organization.id}" name = "Teams" tag_bindings = { - context = try(module.organization.tag_values["context/teams"].id, null) + context = try( + module.organization.tag_values["${var.tag_names.context}/teams"].id, null + ) } } @@ -90,7 +92,9 @@ module "branch-teams-team-dev-folder" { "roles/resourcemanager.projectCreator" = [module.branch-teams-dev-pf-sa.iam_email] } tag_bindings = { - environment = try(module.organization.tag_values["environment/development"].id, null) + environment = try( + module.organization.tag_values["${var.tag_names.environment}/development"].id, null + ) } } @@ -111,7 +115,9 @@ module "branch-teams-team-prod-folder" { "roles/resourcemanager.projectCreator" = [module.branch-teams-prod-pf-sa.iam_email] } tag_bindings = { - environment = try(module.organization.tag_values["environment/production"].id, null) + environment = try( + module.organization.tag_values["${var.tag_names.environment}/production"].id, null + ) } } diff --git a/fast/stages/01-resman/organization.tf b/fast/stages/01-resman/organization.tf index 4f462059..b917b514 100644 --- a/fast/stages/01-resman/organization.tf +++ b/fast/stages/01-resman/organization.tf @@ -151,7 +151,7 @@ module "organization" { # ) } tags = { - context = { + (var.tag_names.context) = { description = "Resource management context." iam = {} values = { @@ -163,7 +163,7 @@ module "organization" { teams = null } } - environment = { + (var.tag_names.environment) = { description = "Environment definition." iam = {} values = { @@ -190,9 +190,9 @@ resource "google_organization_iam_member" "org_policy_admin" { title = "org_policy_tag_scoped" description = "Org policy tag scoped grant for ${each.value.0}/${each.value.1}." expression = <<-END - resource.matchTag('${var.organization.id}/context', '${each.value.0}') + resource.matchTag('${var.organization.id}/${var.tag_names.context}', '${each.value.0}') && - resource.matchTag('${var.organization.id}/environment', '${each.value.1}') + resource.matchTag('${var.organization.id}/${var.tag_names.environment}', '${each.value.1}') END } } diff --git a/fast/stages/01-resman/outputs.tf b/fast/stages/01-resman/outputs.tf index c9e68e66..aefdf9e5 100644 --- a/fast/stages/01-resman/outputs.tf +++ b/fast/stages/01-resman/outputs.tf @@ -150,6 +150,7 @@ locals { tfvars = { folder_ids = local.folder_ids service_accounts = local.service_accounts + tag_names = var.tag_names } } diff --git a/fast/stages/01-resman/variables.tf b/fast/stages/01-resman/variables.tf index b0a97cb0..d0c7416f 100644 --- a/fast/stages/01-resman/variables.tf +++ b/fast/stages/01-resman/variables.tf @@ -165,6 +165,23 @@ variable "prefix" { } } +variable "tag_names" { + description = "Customized names for resource management tags." + type = object({ + context = string + environment = string + }) + default = { + context = "context" + environment = "environment" + } + nullable = false + validation { + condition = alltrue([for k, v in var.tag_names : v != null]) + error_message = "Tag names cannot be null." + } +} + variable "team_folders" { description = "Team folders to be created. Format is described in a code comment." type = map(object({ From 6c82aa04b56b4acc3c5ac818657507d8ab5a62fa Mon Sep 17 00:00:00 2001 From: David Gleich Date: Wed, 13 Apr 2022 13:10:15 +0200 Subject: [PATCH 15/15] Reformatting with yapf --- .../network-dashboard/cloud-function/main.py | 47 +++++++++++++------ 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/examples/cloud-operations/network-dashboard/cloud-function/main.py b/examples/cloud-operations/network-dashboard/cloud-function/main.py index 593d7b60..a3ec9834 100644 --- a/examples/cloud-operations/network-dashboard/cloud-function/main.py +++ b/examples/cloud-operations/network-dashboard/cloud-function/main.py @@ -371,7 +371,9 @@ def get_gce_instances_data(metrics_dict, gce_instance_dict, limit_dict): current_quota_limit = get_quota_current_limit(f"projects/{project}", metric_instances_limit) if current_quota_limit is None: - print(f"Could not write number of instances for projects/{project} due to missing quotas") + print( + f"Could not write number of instances for projects/{project} due to missing quotas" + ) current_quota_limit_view = customize_quota_view(current_quota_limit) @@ -527,7 +529,9 @@ def get_l4_forwarding_rules_data(metrics_dict, forwarding_rules_dict, f"projects/{project}", L4_FORWARDING_RULES_LIMIT_METRIC) if current_quota_limit is None: - print(f"Could not write L4 forwarding rules to metric for projects/{project} due to missing quotas") + print( + f"Could not write L4 forwarding rules to metric for projects/{project} due to missing quotas" + ) continue current_quota_limit_view = customize_quota_view(current_quota_limit) @@ -555,6 +559,7 @@ def get_l4_forwarding_rules_data(metrics_dict, forwarding_rules_dict, print( f"Wrote number of L4 forwarding rules to metric for projects/{project}") + def get_l7_forwarding_rules_data(metrics_dict, forwarding_rules_dict, limit_dict): ''' @@ -573,10 +578,11 @@ def get_l7_forwarding_rules_data(metrics_dict, forwarding_rules_dict, current_quota_limit = get_quota_current_limit( f"projects/{project}", L7_FORWARDING_RULES_LIMIT_METRIC) if current_quota_limit is None: - print(f"Could not write number of L7 forwarding rules to metric for projects/{project} due to missing quotas") + print( + f"Could not write number of L7 forwarding rules to metric for projects/{project} due to missing quotas" + ) continue - current_quota_limit_view = customize_quota_view(current_quota_limit) for net in network_dict: @@ -625,7 +631,9 @@ def get_pgg_data(metric_dict, usage_dict, limit_metric, limit_dict): current_quota_limit = get_quota_current_limit(f"projects/{project}", limit_metric) if current_quota_limit is None: - print(f"Could not write number of L7 forwarding rules to metric for projects/{project} due to missing quotas") + print( + f"Could not write number of L7 forwarding rules to metric for projects/{project} due to missing quotas" + ) continue current_quota_limit_view = customize_quota_view(current_quota_limit) @@ -660,7 +668,9 @@ def get_pgg_data(metric_dict, usage_dict, limit_metric, limit_dict): current_peered_quota_limit = get_quota_current_limit( f"projects/{peered_network_dict['project_id']}", limit_metric) if current_peered_quota_limit is None: - print(f"Could not write metrics for peering to projects/{peered_network_dict['project_id']} due to missing quotas") + print( + f"Could not write metrics for peering to projects/{peered_network_dict['project_id']} due to missing quotas" + ) continue peering_project_limit = customize_quota_view(current_peered_quota_limit) @@ -753,7 +763,9 @@ def count_effective_limit(project_id, network_dict, usage_metric_name, peering_group_usage = network_dict['usage'] for peered_network in network_dict['peerings']: if 'usage' not in peered_network: - print(f"Can not add metrics for peered network in projects/{project_id} as no usage metrics exist due to missing permissions") + print( + f"Can not add metrics for peered network in projects/{project_id} as no usage metrics exist due to missing permissions" + ) continue peering_group_usage += peered_network['usage'] @@ -773,8 +785,11 @@ def count_effective_limit(project_id, network_dict, usage_metric_name, max(peered_network['limit'], get_limit_ppg(peered_network_link, limit_dict))) else: - print(f"Ignoring projects/{peered_network['project_id']} for limits in peering group of project {project_id} as no limits are available." + - "This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project") + print( + f"Ignoring projects/{peered_network['project_id']} for limits in peering group of project {project_id} as no limits are available." + + + "This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project" + ) # Calculates effective limit: Step 3: Find minimum from the list created by Step 2 limit_step3 = 0 @@ -1025,13 +1040,14 @@ def get_network_id(project_id, network_name): except errors.HttpError as err: # TODO: log proper warning if err.resp.status == http.HTTPStatus.FORBIDDEN: - print(f"Warning: error reading networks for {project_id}. " + - f"This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project") + print( + f"Warning: error reading networks for {project_id}. " + + f"This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project" + ) else: print(f"Warning: error reading networks for {project_id}: {err}") return 0 - network_id = 0 if 'items' in response: @@ -1069,10 +1085,13 @@ def get_quota_current_limit(project_link, metric_name): results_list = list(results) return results_list except exceptions.PermissionDenied as err: - print(f"Warning: error reading quotas for {project_link}. " + - f"This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project") + print( + f"Warning: error reading quotas for {project_link}. " + + f"This can happen if you don't have permissions on the project, for example if the project is in another organization or a Google managed project" + ) return None + def customize_quota_view(quota_results): ''' Customize the quota output for an easier parsable output.