Feat/integration tests for secure cloud run (#56)

* Adds integration tests for secure_cloud_run example * fix lint * fix impersionate * fix impersonate position * changing terraformSA call * split terraformSA * setting access-context-manager on int.cloudbuild * fixing cloudbuild syntax * fixes concurrency errors in build * fixes SA for terraform * increasing time_sleep * test:removing kms integration tests * adding impersonate to KMS test * reset and update ip_cidr_range variable * fix missing fields
GoogleCloudPlatform · Sep 28, 2022 · 69aee67 · 69aee67
1 parent 30ebaef
commit 69aee67
Show file tree

Hide file tree

Showing 29 changed files with 522 additions and 126 deletions.
diff --git a/Makefile b/Makefile
@@ -1,4 +1,4 @@
-# Copyright 2019 Google LLC
+# Copyright 2021 Google LLC
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -22,11 +22,37 @@ DOCKER_TAG_VERSION_DEVELOPER_TOOLS := 1.5
 DOCKER_IMAGE_DEVELOPER_TOOLS := cft/developer-tools
 REGISTRY_URL := gcr.io/cloud-foundation-cicd
 
+# Execute lint tests within the docker container
+.PHONY: docker_test_lint
+docker_test_lint:
+	docker run --rm -it \
+		-e ENABLE_PARALLEL=1 \
+		-v $(CURDIR):/workspace \
+		$(REGISTRY_URL)/${DOCKER_IMAGE_DEVELOPER_TOOLS}:${DOCKER_TAG_VERSION_DEVELOPER_TOOLS} \
+		/usr/local/bin/test_lint.sh
+
+# Generate documentation
+.PHONY: docker_generate_docs
+docker_generate_docs:
+	docker run --rm -it \
+		-v $(CURDIR):/workspace \
+		$(REGISTRY_URL)/${DOCKER_IMAGE_DEVELOPER_TOOLS}:${DOCKER_TAG_VERSION_DEVELOPER_TOOLS} \
+		/bin/bash -c 'source /usr/local/bin/task_helper_functions.sh && generate_docs'
+
+# Alias for backwards compatibility
+.PHONY: generate_docs
+generate_docs: docker_generate_docs
+
 # Enter docker container for local development
 .PHONY: docker_run
 docker_run:
 	docker run --rm -it \
 		-e SERVICE_ACCOUNT_JSON \
+		-e TF_VAR_org_id \
+		-e TF_VAR_folder_id \
+		-e TF_VAR_billing_account \
+		-e TF_VAR_group_email \
+		-e TF_VAR_domain_to_allow \
 		-v "$(CURDIR)":/workspace \
 		$(REGISTRY_URL)/${DOCKER_IMAGE_DEVELOPER_TOOLS}:${DOCKER_TAG_VERSION_DEVELOPER_TOOLS} \
 		/bin/bash
@@ -39,6 +65,7 @@ docker_test_prepare:
 		-e TF_VAR_org_id \
 		-e TF_VAR_folder_id \
 		-e TF_VAR_billing_account \
+		-e TF_VAR_group_email \
 		-v "$(CURDIR)":/workspace \
 		$(REGISTRY_URL)/${DOCKER_IMAGE_DEVELOPER_TOOLS}:${DOCKER_TAG_VERSION_DEVELOPER_TOOLS} \
 		/usr/local/bin/execute_with_credentials.sh prepare_environment
@@ -63,24 +90,3 @@ docker_test_integration:
 		-v "$(CURDIR)":/workspace \
 		$(REGISTRY_URL)/${DOCKER_IMAGE_DEVELOPER_TOOLS}:${DOCKER_TAG_VERSION_DEVELOPER_TOOLS} \
 		/usr/local/bin/test_integration.sh
-
-# Execute lint tests within the docker container
-.PHONY: docker_test_lint
-docker_test_lint:
-	docker run --rm -it \
-		-e EXCLUDE_LINT_DIRS \
-		-v "$(CURDIR)":/workspace \
-		$(REGISTRY_URL)/${DOCKER_IMAGE_DEVELOPER_TOOLS}:${DOCKER_TAG_VERSION_DEVELOPER_TOOLS} \
-		/usr/local/bin/test_lint.sh
-
-# Generate documentation
-.PHONY: docker_generate_docs
-docker_generate_docs:
-	docker run --rm -it \
-		-v "$(CURDIR)":/workspace \
-		$(REGISTRY_URL)/${DOCKER_IMAGE_DEVELOPER_TOOLS}:${DOCKER_TAG_VERSION_DEVELOPER_TOOLS} \
-		/bin/bash -c 'source /usr/local/bin/task_helper_functions.sh && generate_docs'
-
-# Alias for backwards compatibility
-.PHONY: generate_docs
-generate_docs: docker_generate_docs
diff --git a/README.md b/README.md
@@ -46,7 +46,7 @@ module "cloud_run" {
 | domain\_map\_annotations | Annotations to the domain map | `map(string)` | `{}` | no |
 | domain\_map\_labels | A set of key/value label pairs to assign to the Domain mapping | `map(string)` | `{}` | no |
 | encryption\_key | CMEK encryption key self-link expected in the format projects/PROJECT/locations/LOCATION/keyRings/KEY-RING/cryptoKeys/CRYPTO-KEY. | `string` | `null` | no |
-| env\_secret\_vars | Environment variables (Secret Manager) | <pre>list(object({<br>    name = string<br>    value_from = set(object({<br>      secret_key_ref = map(string)<br>    }))<br>  }))</pre> | `[]` | no |
+| env\_secret\_vars | [Beta] Environment variables (Secret Manager) | <pre>list(object({<br>    name = string<br>    value_from = set(object({<br>      secret_key_ref = map(string)<br>    }))<br>  }))</pre> | `[]` | no |
 | env\_vars | Environment variables (cleartext) | <pre>list(object({<br>    value = string<br>    name  = string<br>  }))</pre> | `[]` | no |
 | force\_override | Option to force override existing mapping | `bool` | `false` | no |
 | generate\_revision\_name | Option to enable revision name generation | `bool` | `true` | no |
@@ -66,8 +66,8 @@ module "cloud_run" {
 | timeout\_seconds | Timeout for each request | `number` | `120` | no |
 | traffic\_split | Managing traffic routing to the service | <pre>list(object({<br>    latest_revision = bool<br>    percent         = number<br>    revision_name   = string<br>  }))</pre> | <pre>[<br>  {<br>    "latest_revision": true,<br>    "percent": 100,<br>    "revision_name": "v1-0-0"<br>  }<br>]</pre> | no |
 | verified\_domain\_name | Custom Domain Name | `string` | `""` | no |
-| volume\_mounts | Volume Mounts to be attached to the container (when using secret) | <pre>list(object({<br>    mount_path = string<br>    name       = string<br>  }))</pre> | `[]` | no |
-| volumes | Volumes needed for environment variables (when using secret) | <pre>list(object({<br>    name = string<br>    secret = set(object({<br>      secret_name = string<br>      items       = map(string)<br>    }))<br>  }))</pre> | `[]` | no |
+| volume\_mounts | [Beta] Volume Mounts to be attached to the container (when using secret) | <pre>list(object({<br>    mount_path = string<br>    name       = string<br>  }))</pre> | `[]` | no |
+| volumes | [Beta] Volumes needed for environment variables (when using secret) | <pre>list(object({<br>    name = string<br>    secret = set(object({<br>      secret_name = string<br>      items       = map(string)<br>    }))<br>  }))</pre> | `[]` | no |
 
 ## Outputs
 

diff --git a/build/int.cloudbuild.yaml b/build/int.cloudbuild.yaml
@@ -1,4 +1,4 @@
-# Copyright 2019 Google LLC
+# 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.
@@ -14,6 +14,17 @@
 
 timeout: 3600s
 steps:
+- id: secure cloud run on foundation
+  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
+  args: ['/bin/bash', '-c', 'cft test run TestSecureCloudRun --test-dir /workspace/test/integration --verbose']
+  env:
+  - 'TF_VAR_org_id=$_SFB_ORG_ID'
+  - 'TF_VAR_billing_account=$_BILLING_ACCOUNT'
+  - 'TF_VAR_terraform_sa=$_SFB_TF_SA_NAME'
+  - 'TF_VAR_cloudbuild_project_id=$_SFB_CLOUDBUILD_PROJECT_ID'
+  - 'TF_VAR_seed_project_id=$_SFB_SEED_PROJECT_ID'
+  - 'TF_VAR_domain=test.infra.cft.tips'
+  - 'TF_VAR_resource_names_suffix=$SHORT_SHA'
 - id: prepare
   name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
   args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && prepare_environment']

diff --git a/examples/secure_cloud_run/README.md b/examples/secure_cloud_run/README.md
@@ -18,15 +18,14 @@ This example assumes that below mentioned prerequisites are in place before cons
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
-| artifact\_registry\_repository\_location | Artifact Registry Repository location to grant serverless identity viewer role. | `string` | n/a | yes |
-| artifact\_registry\_repository\_name | Artifact Registry Repository name to grant serverless identity viewer role | `string` | n/a | yes |
-| artifact\_registry\_repository\_project\_id | Artifact Registry Repository Project ID to grant serverless identity viewer role. | `string` | n/a | yes |
 | cloud\_run\_sa | Service account to be used on Cloud Run. | `string` | n/a | yes |
-| domain | Domain name to run the load balancer on. Used if `ssl` is `true`. Modify the default value below for your `domain` name | `string` | n/a | yes |
+| domain | Domain name to run the load balancer on. Used if `ssl` is `true`. | `string` | n/a | yes |
 | folder\_id | The folder ID to apply the policy to. | `string` | `""` | no |
+| ip\_cidr\_range | The range of internal addresses that are owned by this subnetwork. Provide this property when you create the subnetwork. For example, 10.0.0.0/28 or 192.168.0.0/28. Ranges must be unique and non-overlapping within a network. Only IPv4 is supported | `string` | n/a | yes |
 | kms\_project\_id | The project where KMS will be created. | `string` | n/a | yes |
 | organization\_id | The organization ID to apply the policy to. | `string` | `""` | no |
 | policy\_for | Policy Root: set one of the following values to determine where the policy is applied. Possible values: ["project", "folder", "organization"]. | `string` | `"project"` | no |
+| resource\_names\_suffix | A suffix to concat in the end of the network resources names. | `string` | `null` | no |
 | serverless\_project\_id | The project where cloud run is going to be deployed. | `string` | n/a | yes |
 | shared\_vpc\_name | Shared VPC name which is going to be re-used to create Serverless Connector. | `string` | n/a | yes |
 | ssl | Run load balancer on HTTPS and provision managed certificate with provided `domain`. | `bool` | `true` | no |
@@ -38,6 +37,7 @@ This example assumes that below mentioned prerequisites are in place before cons
 |------|-------------|
 | cloud\_services\_sa | Service Account for Cloud Run Service. |
 | connector\_id | VPC serverless connector ID. |
+| domain | Domain name to run the load balancer on. Used if `ssl` is `true`. |
 | domain\_map\_id | Unique Identifier for the created domain map. |
 | domain\_map\_status | Status of Domain mapping. |
 | folder\_id | The folder ID to apply the policy to. |
@@ -54,6 +54,7 @@ This example assumes that below mentioned prerequisites are in place before cons
 | service\_id | Unique Identifier for the created service. |
 | service\_status | Status of the created service. |
 | service\_url | The URL on which the deployed service is available. |
+| shared\_vpc\_name | Shared VPC name which is going to be re-used to create Serverless Connector. |
 | vpc\_project\_id | The project where VPC Connector is going to be deployed. |
 
 <!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->

diff --git a/examples/secure_cloud_run/main.tf b/examples/secure_cloud_run/main.tf
@@ -15,37 +15,33 @@
  */
 
 locals {
-  cloudrun_key_name     = "cloud-run-${random_id.suffix.hex}"
-  cloudrun_keyring_name = "cloud-run-keyring-${random_id.suffix.hex}"
-}
-resource "random_id" "suffix" {
-  byte_length = 4
+  cloudrun_key_name     = "cloud-run-${var.resource_names_suffix}"
+  cloudrun_keyring_name = "cloud-run-keyring-${var.resource_names_suffix}"
 }
 
 module "secure_cloud_run" {
   source = "../../modules/secure-cloud-run"
 
-  connector_name                          = "serverless-connector"
-  subnet_name                             = "vpc-subnet"
-  vpc_project_id                          = var.vpc_project_id
-  serverless_project_id                   = var.serverless_project_id
-  domain                                  = var.domain
-  kms_project_id                          = var.kms_project_id
-  shared_vpc_name                         = var.shared_vpc_name
-  ip_cidr_range                           = "10.35.0.0/28"
-  key_name                                = local.cloudrun_key_name
-  keyring_name                            = local.cloudrun_keyring_name
-  prevent_destroy                         = false
-  key_rotation_period                     = "2592000s"
-  service_name                            = "hello-world"
-  location                                = "us-central1"
-  region                                  = "us-central1"
-  image                                   = "us-docker.pkg.dev/cloudrun/container/hello"
-  cloud_run_sa                            = var.cloud_run_sa
-  artifact_registry_repository_location   = var.artifact_registry_repository_location
-  artifact_registry_repository_name       = var.artifact_registry_repository_name
-  artifact_registry_repository_project_id = var.artifact_registry_repository_project_id
-  policy_for                              = var.policy_for
-  folder_id                               = var.folder_id
-  organization_id                         = var.organization_id
+  connector_name        = "con-run"
+  subnet_name           = "vpc-subnet"
+  vpc_project_id        = var.vpc_project_id
+  serverless_project_id = var.serverless_project_id
+  domain                = var.domain
+  kms_project_id        = var.kms_project_id
+  shared_vpc_name       = var.shared_vpc_name
+  ip_cidr_range         = var.ip_cidr_range
+  key_name              = local.cloudrun_key_name
+  keyring_name          = local.cloudrun_keyring_name
+  prevent_destroy       = false
+  key_rotation_period   = "2592000s"
+  service_name          = "hello-world"
+  location              = "us-central1"
+  region                = "us-central1"
+  image                 = "us-docker.pkg.dev/cloudrun/container/hello"
+  cloud_run_sa          = var.cloud_run_sa
+  policy_for            = var.policy_for
+  folder_id             = var.folder_id
+  organization_id       = var.organization_id
+  resource_names_suffix = var.resource_names_suffix
+  create_subnet         = true
 }
diff --git a/examples/secure_cloud_run/outputs.tf b/examples/secure_cloud_run/outputs.tf
@@ -108,3 +108,13 @@ output "organization_id" {
   description = "The organization ID to apply the policy to."
   value       = var.organization_id
 }
+
+output "domain" {
+  description = "Domain name to run the load balancer on. Used if `ssl` is `true`."
+  value       = var.domain
+}
+
+output "shared_vpc_name" {
+  description = "Shared VPC name which is going to be re-used to create Serverless Connector."
+  value       = var.shared_vpc_name
+}
diff --git a/examples/secure_cloud_run/variables.tf b/examples/secure_cloud_run/variables.tf
@@ -46,22 +46,7 @@ variable "kms_project_id" {
 }
 
 variable "domain" {
-  description = "Domain name to run the load balancer on. Used if `ssl` is `true`. Modify the default value below for your `domain` name"
-  type        = string
-}
-
-variable "artifact_registry_repository_project_id" {
-  description = "Artifact Registry Repository Project ID to grant serverless identity viewer role."
-  type        = string
-}
-
-variable "artifact_registry_repository_location" {
-  description = "Artifact Registry Repository location to grant serverless identity viewer role."
-  type        = string
-}
-
-variable "artifact_registry_repository_name" {
-  description = "Artifact Registry Repository name to grant serverless identity viewer role"
+  description = "Domain name to run the load balancer on. Used if `ssl` is `true`."
   type        = string
 }
 
@@ -82,3 +67,14 @@ variable "organization_id" {
   type        = string
   default     = ""
 }
+
+variable "resource_names_suffix" {
+  description = "A suffix to concat in the end of the network resources names."
+  type        = string
+  default     = null
+}
+
+variable "ip_cidr_range" {
+  description = "The range of internal addresses that are owned by this subnetwork. Provide this property when you create the subnetwork. For example, 10.0.0.0/28 or 192.168.0.0/28. Ranges must be unique and non-overlapping within a network. Only IPv4 is supported"
+  type        = string
+}
diff --git a/examples/secure_cloud_run/versions.tf b/examples/secure_cloud_run/versions.tf
@@ -20,11 +20,11 @@ terraform {
   required_providers {
     google = {
       source  = "hashicorp/google"
-      version = ">= 3.53, < 5.0"
+      version = "< 5.0"
     }
     google-beta = {
       source  = "hashicorp/google-beta"
-      version = ">= 3.53, < 5.0"
+      version = "< 5.0"
     }
   }
 }
diff --git a/modules/secure-cloud-run-core/README.md b/modules/secure-cloud-run-core/README.md
@@ -25,8 +25,6 @@ resources of this module:
 * Serverless Project
   * Google Cloud Run Service: `run.googleapis.com`
 
-**Note:** `Secret Manager API` needs to be enabled in case of using secrets.
-
 ### Service Account
 
 A service account with the following roles must be used to provision
@@ -36,8 +34,6 @@ the resources of this module:
 * Compute Network User: `roles/compute.networkUser`
 * Artifact Registry Reader: `roles/artifactregistry.reader`
 
-**Note:** [Secret Manager Secret Accessor](https://cloud.google.com/run/docs/configuring/secrets#access-secret) role must be granted to the Cloud Run service account to allow read access on the secret.
-
 ## Usage
 
 ```hcl

diff --git a/modules/secure-cloud-run-core/versions.tf b/modules/secure-cloud-run-core/versions.tf
@@ -20,11 +20,11 @@ terraform {
   required_providers {
     google = {
       source  = "hashicorp/google"
-      version = ">= 3.53, < 5.0"
+      version = "< 5.0"
     }
     google-beta = {
       source  = "hashicorp/google-beta"
-      version = ">= 3.53, < 5.0"
+      version = "< 5.0"
     }
   }
   provider_meta "google" {

diff --git a/modules/secure-cloud-run-net/README.md b/modules/secure-cloud-run-net/README.md
@@ -69,6 +69,7 @@ module "cloud_run_network" {
 | flow\_sampling | Sampling rate of VPC flow logs. The value must be in [0,1]. Where 1.0 means all logs, 0.5 mean half of the logs and 0.0 means no logs are reported. | `number` | `1` | no |
 | ip\_cidr\_range | The range of internal addresses that are owned by this subnetwork. Provide this property when you create the subnetwork. For example, 10.0.0.0/8 or 192.168.0.0/16. Ranges must be unique and non-overlapping within a network. Only IPv4 is supported | `string` | n/a | yes |
 | location | The location where resources are going to be deployed. | `string` | n/a | yes |
+| resource\_names\_suffix | A suffix to concat in the end of the resources names. | `string` | `null` | no |
 | serverless\_project\_id | The project where cloud run is going to be deployed. | `string` | n/a | yes |
 | shared\_vpc\_name | Shared VPC name which is going to be used to create Serverless Connector. | `string` | n/a | yes |
 | subnet\_name | Subnet name to be re-used to create Serverless Connector. | `string` | n/a | yes |
@@ -82,5 +83,6 @@ module "cloud_run_network" {
 | connector\_id | VPC serverless connector ID. |
 | gca\_vpcaccess\_sa | Service Account for VPC Access. |
 | run\_identity\_services\_sa | Service Identity to run services. |
+| subnet\_name | The name of the sub-network used to create VPC Connector. |
 
 <!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
diff --git a/modules/secure-cloud-run-net/firewall.tf b/modules/secure-cloud-run-net/firewall.tf
@@ -15,7 +15,8 @@
  */
 
 locals {
-  tags = ["vpc-connector"]
+  tags   = ["vpc-connector"]
+  suffix = var.resource_names_suffix == null ? "" : "-${var.resource_names_suffix}"
 }
 
 module "firewall_rules" {
@@ -26,7 +27,7 @@ module "firewall_rules" {
   network_name = var.shared_vpc_name
 
   rules = [{
-    name                    = "serverless-to-vpc-connector"
+    name                    = "serverless-to-vpc-connector${local.suffix}"
     description             = null
     priority                = null
     direction               = "INGRESS"
@@ -53,7 +54,7 @@ module "firewall_rules" {
     }
     },
     {
-      name                    = "vpc-connector-to-serverless"
+      name                    = "vpc-connector-to-serverless${local.suffix}"
       description             = null
       priority                = null
       direction               = "EGRESS"
@@ -80,7 +81,7 @@ module "firewall_rules" {
       }
     },
     {
-      name                    = "vpc-connector-to-lb"
+      name                    = "vpc-connector-to-lb${local.suffix}"
       description             = null
       priority                = null
       direction               = "EGRESS"
@@ -99,7 +100,7 @@ module "firewall_rules" {
       }
     },
     {
-      name                    = "vpc-connector-health-checks"
+      name                    = "vpc-connector-health-checks${local.suffix}"
       description             = null
       priority                = null
       direction               = "INGRESS"
@@ -118,7 +119,7 @@ module "firewall_rules" {
       }
     },
     {
-      name                    = "vpc-connector-requests"
+      name                    = "vpc-connector-requests${local.suffix}"
       description             = null
       priority                = null
       direction               = "INGRESS"

diff --git a/modules/secure-cloud-run-net/iam.tf b/modules/secure-cloud-run-net/iam.tf
@@ -49,7 +49,7 @@ resource "google_project_iam_member" "cloud_services" {
 }
 
 resource "google_project_iam_member" "run_identity_services" {
-  count = var.connector_on_host_project ? 1 : 0
+  count = var.connector_on_host_project ? 0 : 1
 
   project = var.vpc_project_id
   role    = "roles/vpcaccess.user"