From 52bea71359e94e649c53f55fde427a5234b50517 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Tue, 9 Aug 2022 13:44:34 +0200 Subject: [PATCH] refactor cloud run module --- modules/cloud-run/README.md | 14 +-- modules/cloud-run/main.tf | 111 ++++++++++++++---------- modules/cloud-run/variables.tf | 31 ++++--- tests/modules/cloud_run/fixture/main.tf | 44 +++++----- tests/modules/cloud_run/test_plan.py | 64 ++++++++++---- 5 files changed, 159 insertions(+), 105 deletions(-) diff --git a/modules/cloud-run/README.md b/modules/cloud-run/README.md index 3ac4075da9..205fcbc1fc 100644 --- a/modules/cloud-run/README.md +++ b/modules/cloud-run/README.md @@ -222,13 +222,13 @@ module "cloud_run" { | [prefix](variables.tf#L82) | Optional prefix used for resource names. | string | | null | | [pubsub_triggers](variables.tf#L93) | Eventarc triggers (Pub/Sub). | list(string) | | null | | [region](variables.tf#L99) | Region used for all resources. | string | | "europe-west1" | -| [revision_name](variables.tf#L105) | Revision name. | string | | null | -| [service_account](variables.tf#L111) | Service account email. Unused if service account is auto-created. | string | | null | -| [service_account_create](variables.tf#L117) | Auto-create service account. | bool | | false | -| [traffic](variables.tf#L123) | Traffic. | map(number) | | null | -| [volumes](variables.tf#L129) | Volumes. | list(object({…})) | | null | -| [vpc_connector](variables.tf#L142) | VPC connector configuration. Set create to 'true' if a new connecto needs to be created. | object({…}) | | null | -| [vpc_connector_config](variables.tf#L152) | VPC connector network configuration. Must be provided if new VPC connector is being created. | object({…}) | | null | +| [revision_annotations](variables.tf#L105) | Configure revision template annotations. | object({…}) | | null | +| [revision_name](variables.tf#L119) | Revision name. | string | | null | +| [service_account](variables.tf#L125) | Service account email. Unused if service account is auto-created. | string | | null | +| [service_account_create](variables.tf#L131) | Auto-create service account. | bool | | false | +| [traffic](variables.tf#L137) | Traffic. | map(number) | | null | +| [volumes](variables.tf#L143) | Volumes. | list(object({…})) | | null | +| [vpc_connector_create](variables.tf#L156) | Populate this to create a VPC connector. You can then refer to it in the template annotations. | object({…}) | | null | ## Outputs diff --git a/modules/cloud-run/main.tf b/modules/cloud-run/main.tf index 47fe678c79..314c2929e2 100644 --- a/modules/cloud-run/main.tf +++ b/modules/cloud-run/main.tf @@ -15,25 +15,47 @@ */ locals { + _vpcaccess_annotation = ( + local.vpc_connector_create + ? { + "run.googleapis.com/vpc-access-connector" = google_vpc_access_connector.connector.0.id + } + : ( + try(var.revision_annotations.vpcaccess_connector, null) == null + ? {} + : { + "run.googleapis.com/vpc-access-connector" = var.revision_annotations.vpcaccess_connector + } + ) + ) annotations = merge( var.ingress_settings == null ? {} : { "run.googleapis.com/ingress" = var.ingress_settings } ) - template_annotations = merge( - var.vpc_connector == null ? {} : { - "run.googleapis.com/vpc-access-connector" = ( - try(var.vpc_connector.create, false) - ? google_vpc_access_connector.connector.0.id - : var.vpc_connector.name - ) + prefix = var.prefix == null ? "" : "${var.prefix}-" + revision_annotations = merge( + try(var.revision_annotations.autoscaling.max_scale, null) == null ? {} : { + "autoscaling.knative.dev/maxScale" = var.revision_annotations.autoscaling.max_scale }, - try(var.vpc_connector.egress_settings, null) == null ? {} : { - "run.googleapis.com/vpc-access-egress" = var.vpc_connector.egress_settings - } + try(var.revision_annotations.cloudsql_instances, null) == null ? {} : { + "run.googleapis.com/cloudsql-instances" = join(",", coalesce( + var.revision_annotations.cloudsql_instances, [] + )) + }, + local._vpcaccess_annotation, + try(var.revision_annotations.autoscaling.max_scale, null) == null ? {} : { + "autoscaling.knative.dev/minScale" = var.revision_annotations.autoscaling.min_scale + }, + try(var.revision_annotations.vpcaccess_egress, null) == null ? {} : { + "run.googleapis.com/vpc-access-egress" = var.revision_annotations.vpcaccess_egress + }, + ) + revision_name = ( + try(var.revision_name, null) == null + ? null + : "${var.name}-${var.revision_name}" ) - revision_name = try(var.revision_name, null) == null ? null : "${var.name}-${var.revision_name}" - prefix = var.prefix == null ? "" : "${var.prefix}-" service_account_email = ( var.service_account_create ? ( @@ -43,15 +65,16 @@ locals { ) : var.service_account ) + vpc_connector_create = var.vpc_connector_create != null } resource "google_vpc_access_connector" "connector" { - count = try(var.vpc_connector.create, false) ? 1 : 0 + count = local.vpc_connector_create ? 1 : 0 project = var.project_id - name = var.vpc_connector.name + name = var.vpc_connector_create.name region = var.region - ip_cidr_range = var.vpc_connector_config.ip_cidr_range - network = var.vpc_connector_config.network + ip_cidr_range = var.vpc_connector_create.ip_cidr_range + network = var.vpc_connector_create.vpc_self_link } resource "google_cloud_run_service" "service" { @@ -67,14 +90,14 @@ resource "google_cloud_run_service" "service" { for i, container in var.containers : i => container } content { - image = containers.value["image"] - command = try(containers.value["options"]["command"], null) - args = try(containers.value["options"]["args"], null) + image = containers.value.image + command = try(containers.value.options.command, null) + args = try(containers.value.options.args, null) dynamic "env" { for_each = ( - try(containers.value["options"]["env"], null) == null + try(containers.value.options.env, null) == null ? {} - : containers.value["options"]["env"] + : containers.value.options.env ) content { name = env.key @@ -83,47 +106,47 @@ resource "google_cloud_run_service" "service" { } dynamic "env" { for_each = ( - try(containers.value["options"]["env_from"], null) == null + try(containers.value.options.env_from, null) == null ? {} - : containers.value["options"]["env_from"] + : containers.value.options.env_from ) content { name = env.key value_from { secret_key_ref { - name = env.value["name"] - key = env.value["key"] + name = env.value.name + key = env.value.key } } } } dynamic "ports" { for_each = ( - containers.value["ports"] == null + containers.value.ports == null ? {} : { - for port in containers.value["ports"] : + for port in containers.value.ports : "${port.name}-${port.container_port}" => port } ) content { - name = ports.value["name"] - protocol = ports.value["protocol"] - container_port = ports.value["container_port"] + name = ports.value.name + protocol = ports.value.protocol + container_port = ports.value.container_port } } dynamic "resources" { - for_each = containers.value["resources"] == null ? [] : [""] + for_each = containers.value.resources == null ? [] : [""] content { - limits = containers.value["resources"]["limits"] - requests = containers.value["resources"]["requests"] + limits = containers.value.resources.limits + requests = containers.value.resources.requests } } dynamic "volume_mounts" { for_each = ( - containers.value["volume_mounts"] == null + containers.value.volume_mounts == null ? {} - : containers.value["volume_mounts"] + : containers.value.volume_mounts ) content { name = volume_mounts.key @@ -136,18 +159,16 @@ resource "google_cloud_run_service" "service" { dynamic "volumes" { for_each = var.volumes == null ? [] : var.volumes content { - name = volumes.value["name"] + name = volumes.value.name secret { - secret_name = volumes.value["secret_name"] + secret_name = volumes.value.secret_name dynamic "items" { for_each = ( - volumes.value["items"] == null - ? [] - : volumes.value["items"] + volumes.value.items == null ? [] : volumes.value.items ) content { - key = items.value["key"] - path = items.value["path"] + key = items.value.key + path = items.value.path } } } @@ -156,7 +177,7 @@ resource "google_cloud_run_service" "service" { } metadata { name = local.revision_name - annotations = local.template_annotations + annotations = local.revision_annotations } } @@ -204,11 +225,11 @@ resource "google_eventarc_trigger" "audit_log_triggers" { } matching_criteria { attribute = "serviceName" - value = each.value["service_name"] + value = each.value.service_name } matching_criteria { attribute = "methodName" - value = each.value["method_name"] + value = each.value.method_name } destination { cloud_run_service { diff --git a/modules/cloud-run/variables.tf b/modules/cloud-run/variables.tf index 81777d9af3..ab9b552b41 100644 --- a/modules/cloud-run/variables.tf +++ b/modules/cloud-run/variables.tf @@ -102,6 +102,20 @@ variable "region" { default = "europe-west1" } +variable "revision_annotations" { + description = "Configure revision template annotations." + type = object({ + autoscaling = object({ + max_scale = number + min_scale = number + }) + cloudsql_instances = list(string) + vpcaccess_connector = string + vpcaccess_egress = string + }) + default = null +} + variable "revision_name" { description = "Revision name." type = string @@ -139,21 +153,12 @@ variable "volumes" { default = null } -variable "vpc_connector" { - description = "VPC connector configuration. Set create to 'true' if a new connecto needs to be created." - type = object({ - create = bool - name = string - egress_settings = string - }) - default = null -} - -variable "vpc_connector_config" { - description = "VPC connector network configuration. Must be provided if new VPC connector is being created." +variable "vpc_connector_create" { + description = "Populate this to create a VPC connector. You can then refer to it in the template annotations." type = object({ ip_cidr_range = string - network = string + name = string + vpc_self_link = string }) default = null } diff --git a/tests/modules/cloud_run/fixture/main.tf b/tests/modules/cloud_run/fixture/main.tf index acc3805134..4692c22c7f 100644 --- a/tests/modules/cloud_run/fixture/main.tf +++ b/tests/modules/cloud_run/fixture/main.tf @@ -12,21 +12,28 @@ # See the License for the specific language governing permissions and # limitations under the License. -variable "vpc_connector" { - type = any - default = null +variable "revision_annotations" { + description = "Configure revision template annotations." + type = any + default = null } -variable "vpc_connector_config" { - type = any - default = null +variable "vpc_connector_create" { + description = "Populate this to create a VPC connector. You can then refer to it in the template annotations." + type = any + default = null } module "cloud_run" { - source = "../../../../modules/cloud-run" - project_id = "my-project" - name = "hello" - revision_name = "blue" + source = "../../../../modules/cloud-run" + project_id = "my-project" + name = "hello" + audit_log_triggers = [ + { + "service_name" : "cloudresourcemanager.googleapis.com", + "method_name" : "SetIamPolicy" + } + ] containers = [{ image = "us-docker.pkg.dev/cloudrun/container/hello" options = null @@ -34,19 +41,14 @@ module "cloud_run" { resources = null volume_mounts = null }] - audit_log_triggers = [ - { - "service_name" : "cloudresourcemanager.googleapis.com", - "method_name" : "SetIamPolicy" - } - ] + iam = { + "roles/run.invoker" = ["allUsers"] + } pubsub_triggers = [ "topic1", "topic2" ] - iam = { - "roles/run.invoker" = ["allUsers"] - } - vpc_connector = var.vpc_connector - vpc_connector_config = var.vpc_connector_config + revision_name = "blue" + revision_annotations = var.revision_annotations + vpc_connector_create = var.vpc_connector_create } diff --git a/tests/modules/cloud_run/test_plan.py b/tests/modules/cloud_run/test_plan.py index 44dec1f6db..0671097ae6 100644 --- a/tests/modules/cloud_run/test_plan.py +++ b/tests/modules/cloud_run/test_plan.py @@ -57,25 +57,51 @@ def test_pubsub_triggers(resources): assert len(pubsub_triggers) == 2 -def test_vpc_connector_none(plan_runner): - "Test VPC connector creation." - _, resources = plan_runner() - assert len( - [r for r in resources if r['type'] == 'google_vpc_access_connector']) == 0 - - -def test_vpc_connector_nocreate(plan_runner): - "Test VPC connector creation." - _, resources = plan_runner( - vpc_connector='{create=false, name="foo", egress_settings=null}') - assert len( - [r for r in resources if r['type'] == 'google_vpc_access_connector']) == 0 +def test_revision_annotations(plan_runner): + revision_annotations = '''{ + autoscaling = null + cloudsql_instances = ["a", "b"] + vpcaccess_connector = "foo" + vpcaccess_egress = "all-traffic" + }''' + _, resources = plan_runner(revision_annotations=revision_annotations) + r = [ + r['values'] for r in resources if r['type'] == 'google_cloud_run_service' + ][0] + assert r['template'][0]['metadata'][0]['annotations'] == { + 'run.googleapis.com/cloudsql-instances': 'a,b', + 'run.googleapis.com/vpc-access-connector': 'foo', + 'run.googleapis.com/vpc-access-egress': 'all-traffic' + } + + +def test_revision_annotations_autoscaling(plan_runner): + revision_annotations = '''{ + autoscaling = {max_scale = 5, min_scale = 1} + cloudsql_instances = null + vpcaccess_connector = null + vpcaccess_egress = null + }''' + _, resources = plan_runner(revision_annotations=revision_annotations) + r = [ + r['values'] for r in resources if r['type'] == 'google_cloud_run_service' + ][0] + assert r['template'][0]['metadata'][0]['annotations'] == { + 'autoscaling.knative.dev/maxScale': '5', + 'autoscaling.knative.dev/minScale': '1' + } + + +def test_revision_annotations_none(resources): + r = [ + r['values'] for r in resources if r['type'] == 'google_cloud_run_service' + ][0] + assert r['template'][0]['metadata'][0].get('annotations') is None def test_vpc_connector_create(plan_runner): - "Test VPC connector creation." - _, resources = plan_runner( - vpc_connector='{create=true, name="foo", egress_settings=null}', - vpc_connector_config='{ip_cidr_range="10.0.0.0/28", network="default"}') - assert len( - [r for r in resources if r['type'] == 'google_vpc_access_connector']) == 1 + vpc_connector_create = '''{ + ip_cidr_range = "10.10.10.0/24", name = "foo", vpc_self_link = "foo-vpc" + }''' + _, resources = plan_runner(vpc_connector_create=vpc_connector_create) + assert any(r['type'] == 'google_vpc_access_connector' for r in resources)