From f4490fcaea04af83d2143009069e724398414cf3 Mon Sep 17 00:00:00 2001 From: lcaggio Date: Wed, 19 Apr 2023 11:22:50 +0200 Subject: [PATCH] Fix comments --- blueprints/data-solutions/bq-ml/README.md | 2 +- .../data-solutions/data-playground/README.md | 2 +- .../data-solutions/vertex-mlops/README.md | 12 +++--- .../data-solutions/vertex-mlops/ci-cd.tf | 6 +-- .../data-solutions/vertex-mlops/main.tf | 41 +++++++++++-------- .../data-solutions/vertex-mlops/variables.tf | 32 +++++++-------- .../data-solutions/vertex-mlops/vertex.tf | 15 ++++--- 7 files changed, 57 insertions(+), 53 deletions(-) diff --git a/blueprints/data-solutions/bq-ml/README.md b/blueprints/data-solutions/bq-ml/README.md index 45a18f00..385ec529 100644 --- a/blueprints/data-solutions/bq-ml/README.md +++ b/blueprints/data-solutions/bq-ml/README.md @@ -98,5 +98,5 @@ module "test" { prefix = "prefix" } -# tftest modules=9 resources=47 +# tftest modules=9 resources=48 ``` diff --git a/blueprints/data-solutions/data-playground/README.md b/blueprints/data-solutions/data-playground/README.md index 6691e496..a2de4db9 100644 --- a/blueprints/data-solutions/data-playground/README.md +++ b/blueprints/data-solutions/data-playground/README.md @@ -86,5 +86,5 @@ module "test" { parent = "folders/467898377" } } -# tftest modules=8 resources=40 +# tftest modules=8 resources=41 ``` diff --git a/blueprints/data-solutions/vertex-mlops/README.md b/blueprints/data-solutions/vertex-mlops/README.md index 929b06fc..2ce403b6 100644 --- a/blueprints/data-solutions/vertex-mlops/README.md +++ b/blueprints/data-solutions/vertex-mlops/README.md @@ -52,19 +52,19 @@ This blueprint can be used as a building block for setting up an end2end ML Ops | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [notebooks](variables.tf#L73) | Vertex AI workbenchs to be deployed. Service Account runtime/instances deployed. | map(object({…})) | ✓ | | -| [project_config](variables.tf#L100) | Provide 'billing_account_id' value if project creation is needed, uses existing 'project_id' if null. Parent is in 'folders/nnn' or 'organizations/nnn' format. | object({…}) | ✓ | | +| [notebooks](variables.tf#L73) | Vertex AI workbenchs to be deployed. Service Account runtime/instances deployed. | map(object({…})) | ✓ | | +| [project_config](variables.tf#L100) | Provide 'billing_account_id' value if project creation is needed, uses existing 'project_id' if null. Parent is in 'folders/nnn' or 'organizations/nnn' format. | object({…}) | ✓ | | | [bucket_name](variables.tf#L18) | GCS bucket name to store the Vertex AI artifacts. | string | | null | | [dataset_name](variables.tf#L24) | BigQuery Dataset to store the training data. | string | | null | -| [groups](variables.tf#L30) | Name of the groups (name@domain.org) to apply opinionated IAM permissions. | object({…}) | | {…} | +| [groups](variables.tf#L30) | Name of the groups (name@domain.org) to apply opinionated IAM permissions. | object({…}) | | {…} | | [identity_pool_claims](variables.tf#L45) | Claims to be used by Workload Identity Federation (i.e.: attribute.repository/ORGANIZATION/REPO). If a not null value is provided, then google_iam_workload_identity_pool resource will be created. | string | | null | | [labels](variables.tf#L51) | Labels to be assigned at project level. | map(string) | | {} | | [location](variables.tf#L57) | Location used for multi-regional resources. | string | | "eu" | | [network_config](variables.tf#L63) | Shared VPC network configurations to use. If null networks will be created in projects with preconfigured values. | object({…}) | | null | | [prefix](variables.tf#L94) | Prefix used for the project id. | string | | null | -| [region](variables.tf#L113) | Region used for regional resources. | string | | "europe-west4" | -| [repo_name](variables.tf#L119) | Cloud Source Repository name. null to avoid to create it. | string | | null | -| [service_encryption_keys](variables.tf#L125) | Cloud KMS to use to encrypt different services. Key location should match service region. | object({…}) | | {…} | +| [region](variables.tf#L114) | Region used for regional resources. | string | | "europe-west4" | +| [repo_name](variables.tf#L120) | Cloud Source Repository name. null to avoid to create it. | string | | null | +| [service_encryption_keys](variables.tf#L126) | Cloud KMS to use to encrypt different services. Key location should match service region. | object({…}) | | {} | ## Outputs diff --git a/blueprints/data-solutions/vertex-mlops/ci-cd.tf b/blueprints/data-solutions/vertex-mlops/ci-cd.tf index dcff587e..086b9d50 100644 --- a/blueprints/data-solutions/vertex-mlops/ci-cd.tf +++ b/blueprints/data-solutions/vertex-mlops/ci-cd.tf @@ -60,9 +60,9 @@ module "secret-manager" { secrets = { github-key = [var.region] } - # encryption_key = { - # "${var.region}" = try(var.service_encryption_keys["secretmanager"], null) - # } + encryption_key = { + "${var.region}" = var.service_encryption_keys.secretmanager + } iam = { github-key = { "roles/secretmanager.secretAccessor" = [ diff --git a/blueprints/data-solutions/vertex-mlops/main.tf b/blueprints/data-solutions/vertex-mlops/main.tf index ccb8c9d0..2e093f86 100644 --- a/blueprints/data-solutions/vertex-mlops/main.tf +++ b/blueprints/data-solutions/vertex-mlops/main.tf @@ -64,8 +64,7 @@ locals { } ) - service_encryption_keys = var.service_encryption_keys - shared_vpc_project = try(var.network_config.host_project, null) + shared_vpc_project = try(var.network_config.host_project, null) subnet = ( local.use_shared_vpc @@ -109,7 +108,7 @@ module "gcs-bucket" { location = var.region storage_class = "REGIONAL" versioning = false - encryption_key = try(local.service_encryption_keys.storage, null) + encryption_key = var.service_encryption_keys.storage } # Default bucket for Cloud Build to prevent error: "'us' violates constraint ‘gcp.resourceLocations’" @@ -122,7 +121,7 @@ module "gcs-bucket-cloudbuild" { location = var.region storage_class = "REGIONAL" versioning = false - encryption_key = try(local.service_encryption_keys.storage, null) + encryption_key = var.service_encryption_keys.storage } module "bq-dataset" { @@ -131,7 +130,7 @@ module "bq-dataset" { project_id = module.project.project_id id = var.dataset_name location = var.region - encryption_key = try(local.service_encryption_keys.bq, null) + encryption_key = var.service_encryption_keys.bq } module "vpc-local" { @@ -191,18 +190,27 @@ module "cloudnat" { module "project" { source = "../../../modules/project" name = var.project_config.project_id - parent = try(var.project_config.parent, null) - billing_account = try(var.project_config.billing_account_id, null) + parent = var.project_config.parent + billing_account = var.project_config.billing_account_id project_create = var.project_config.billing_account_id != null prefix = var.prefix group_iam = local.group_iam iam = { - "roles/aiplatform.user" = [module.service-account-mlops.iam_email, module.service-account-notebook.iam_email] + "roles/aiplatform.user" = [ + module.service-account-mlops.iam_email, + module.service-account-notebook.iam_email + ] "roles/artifactregistry.reader" = [module.service-account-mlops.iam_email] "roles/artifactregistry.writer" = [module.service-account-github.iam_email] - "roles/bigquery.dataEditor" = [module.service-account-mlops.iam_email, module.service-account-notebook.iam_email] - "roles/bigquery.jobUser" = [module.service-account-mlops.iam_email, module.service-account-notebook.iam_email] - "roles/bigquery.user" = [module.service-account-mlops.iam_email, module.service-account-notebook.iam_email] + "roles/bigquery.dataEditor" = [ + module.service-account-mlops.iam_email, + module.service-account-notebook.iam_email + ] + "roles/bigquery.jobUser" = [ + module.service-account-mlops.iam_email, + module.service-account-notebook.iam_email + ] + "roles/bigquery.user" = [module.service-account-mlops.iam_email, module.service-account-notebook.iam_email] "roles/cloudbuild.builds.editor" = [ module.service-account-mlops.iam_email, module.service-account-github.iam_email @@ -232,11 +240,12 @@ module "project" { labels = var.labels service_encryption_key_ids = { - aiplatform = [try(local.service_encryption_keys.aiplatform, null)] - bq = [try(local.service_encryption_keys.bq, null)] - cloudbuild = [try(local.service_encryption_keys.storage, null)] - notebooks = [try(local.service_encryption_keys.notebooks, null)] - storage = [try(local.service_encryption_keys.storage, null)] + aiplatform = [var.service_encryption_keys.aiplatform] + bq = [var.service_encryption_keys.bq] + cloudbuild = [var.service_encryption_keys.storage] + notebooks = [var.service_encryption_keys.notebooks] + secretmanager = [var.service_encryption_keys.secretmanager] + storage = [var.service_encryption_keys.storage] } services = [ diff --git a/blueprints/data-solutions/vertex-mlops/variables.tf b/blueprints/data-solutions/vertex-mlops/variables.tf index 1aa06468..b5800534 100644 --- a/blueprints/data-solutions/vertex-mlops/variables.tf +++ b/blueprints/data-solutions/vertex-mlops/variables.tf @@ -30,9 +30,9 @@ variable "dataset_name" { variable "groups" { description = "Name of the groups (name@domain.org) to apply opinionated IAM permissions." type = object({ - gcp-ml-ds = optional(string, null) - gcp-ml-eng = optional(string, null) - gcp-ml-viewer = optional(string, null) + gcp-ml-ds = optional(string) + gcp-ml-eng = optional(string) + gcp-ml-viewer = optional(string) }) default = { gcp-ml-ds = null @@ -77,7 +77,7 @@ variable "notebooks" { machine_type = optional(string, "n1-standard-4") internal_ip_only = optional(bool, true) idle_shutdown = optional(bool, false) - owner = optional(string, null) + owner = optional(string) })) validation { condition = alltrue([ @@ -100,14 +100,15 @@ variable "prefix" { variable "project_config" { description = "Provide 'billing_account_id' value if project creation is needed, uses existing 'project_id' if null. Parent is in 'folders/nnn' or 'organizations/nnn' format." type = object({ - billing_account_id = optional(string, null) - parent = string + billing_account_id = optional(string) + parent = optional(string) project_id = string }) validation { condition = var.project_config.project_id != null error_message = "Project id must be set." } + nullable = false } variable "region" { @@ -125,17 +126,12 @@ variable "repo_name" { variable "service_encryption_keys" { description = "Cloud KMS to use to encrypt different services. Key location should match service region." type = object({ - aiplatform = optional(string, null) - bq = optional(string, null) - notebooks = optional(string, null) - secretmanager = optional(string, null) - storage = optional(string, null) + aiplatform = optional(string) + bq = optional(string) + notebooks = optional(string) + secretmanager = optional(string) + storage = optional(string) }) - default = { - aiplatform = null - bq = null - notebooks = null - secretmanager = null - storage = null - } + default = {} + nullable = false } diff --git a/blueprints/data-solutions/vertex-mlops/vertex.tf b/blueprints/data-solutions/vertex-mlops/vertex.tf index 264918b4..75152751 100644 --- a/blueprints/data-solutions/vertex-mlops/vertex.tf +++ b/blueprints/data-solutions/vertex-mlops/vertex.tf @@ -21,10 +21,10 @@ resource "google_vertex_ai_metadata_store" "store" { description = "Vertex Ai Metadata Store" region = var.region dynamic "encryption_spec" { - for_each = try(var.service_encryption_keys.aiplatform, null) == null ? [] : [""] + for_each = var.service_encryption_keys.aiplatform == null ? [] : [""] content { - kms_key_name = try(var.service_encryption_keys.aiplatform, null) + kms_key_name = var.service_encryption_keys.aiplatform } } # `state` value will be decided automatically based on the result of the configuration @@ -42,7 +42,6 @@ module "service-account-notebook" { resource "google_notebooks_runtime" "runtime" { for_each = { for k, v in var.notebooks : k => v if v.type == "MANAGED" } name = "${var.prefix}-${each.key}" - project = module.project.project_id location = var.region access_config { @@ -59,9 +58,9 @@ resource "google_notebooks_runtime" "runtime" { subnet = local.subnet internal_ip_only = var.notebooks[each.key].internal_ip_only dynamic "encryption_config" { - for_each = try(local.service_encryption_keys.notebooks, null) == null ? [] : [1] + for_each = var.service_encryption_keys.notebooks == null ? [] : [1] content { - kms_key = local.service_encryption_keys.notebooks + kms_key = var.service_encryption_keys.notebooks } } metadata = { @@ -83,7 +82,7 @@ resource "google_notebooks_runtime" "runtime" { resource "google_notebooks_instance" "playground" { for_each = { for k, v in var.notebooks : k => v if v.type == "USER_MANAGED" } name = "${var.prefix}-${each.key}" - location = format("%s-%s", var.region, "b") + location = "${var.region}-b" machine_type = var.notebooks[each.key].machine_type project = module.project.project_id @@ -95,8 +94,8 @@ resource "google_notebooks_instance" "playground" { install_gpu_driver = true boot_disk_type = "PD_SSD" boot_disk_size_gb = 110 - disk_encryption = try(local.service_encryption_keys.notebooks != null, false) ? "CMEK" : null - kms_key = try(local.service_encryption_keys.notebooks, null) + disk_encryption = var.service_encryption_keys.notebooks != null ? "CMEK" : null + kms_key = var.service_encryption_keys.notebooks no_public_ip = var.notebooks[each.key].internal_ip_only no_proxy_access = false