[DCP - Terraform] Initializes the APIs, Service Account, Cloud Run Service and Spanner Instance and DB.#15
[DCP - Terraform] Initializes the APIs, Service Account, Cloud Run Service and Spanner Instance and DB.#15gmechali wants to merge 3 commits intodatacommonsorg:mainfrom
Conversation
…panner Instance and DB initialization
Summary of ChangesHello @gmechali, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive set of Terraform scripts designed to automate the initial setup and deployment of the Data Commons Platform (DCP) within a new Google Cloud Project. The changes streamline the provisioning of core GCP services, including API enablement, service account creation with appropriate permissions, Cloud Run service deployment, and Spanner instance and database setup, ensuring a consistent and repeatable infrastructure foundation for DCP. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Terraform scripts for setting up the Data Commons Platform on GCP, including enabling APIs, creating a service account, and provisioning Cloud Run and Spanner resources. The primary security concern identified is the explicit granting of public access to the Cloud Run service via the allUsers IAM binding, which bypasses standard IAM-based access controls and requires careful evaluation. Other issues include the need to scope down IAM permissions, make public access and database deletion protection configurable (especially for production environments), correct the .gitignore for reproducible builds, and avoid the use of the :latest Docker image tag.
|
|
||
| *.pyc | ||
| .venv/ | ||
| uv.lock No newline at end of file |
There was a problem hiding this comment.
The uv.lock file is being ignored. However, this file is essential for ensuring reproducible builds, as it's used in the Dockerfile with uv sync --frozen. Without this file in version control, builds may fail or use different package versions, leading to unpredictable behavior. Please remove uv.lock from the .gitignore file and commit it to the repository.
| name = var.spanner_database_id | ||
|
|
||
| # Prevent deletion of data | ||
| deletion_protection = false # Set to true for production |
There was a problem hiding this comment.
The Spanner database has deletion_protection hardcoded to false. While the comment indicates it should be true for production, it's easy to forget to change this manually, creating a significant risk of accidental data loss in production environments. This setting should be controlled by a variable (e.g., spanner_deletion_protection) to be set appropriately for each environment.
You will also need to add a new variable spanner_deletion_protection in variables.tf, likely with a default of false.
deletion_protection = var.spanner_deletion_protection
| resource "google_cloud_run_service_iam_binding" "public_invoker" { | ||
| location = google_cloud_run_v2_service.dcp_service.location | ||
| service = google_cloud_run_v2_service.dcp_service.name | ||
| role = "roles/run.invoker" | ||
| members = [ | ||
| "allUsers" | ||
| ] |
There was a problem hiding this comment.
The Terraform configuration grants the roles/run.invoker role to allUsers for the Cloud Run service, making it publicly accessible without GCP IAM authentication. This significantly increases the attack surface and could lead to unauthorized access if the application lacks robust internal authentication. It is recommended to control this public access with a variable, such as allow_unauthenticated_invocations, defaulting to false for safety. This makes the decision to expose the service explicit and requires adding this variable to variables.tf.
resource "google_cloud_run_service_iam_binding" "public_invoker" {
count = var.allow_unauthenticated_invocations ? 1 : 0
location = google_cloud_run_v2_service.dcp_service.location
service = google_cloud_run_v2_service.dcp_service.name
role = "roles/run.invoker"
members = [
"allUsers"
]
}
| resource "google_project_iam_member" "spanner_user" { | ||
| project = var.project_id | ||
| role = "roles/spanner.databaseUser" | ||
| member = "serviceAccount:${google_service_account.dcp_runner.email}" | ||
| } |
There was a problem hiding this comment.
The service account is granted roles/spanner.databaseUser at the project level, which is overly permissive. It's a security best practice to scope permissions to the specific resource they are needed for (principle of least privilege). Please grant the role at the Spanner instance level instead using google_spanner_instance_iam_member.
# Grant Spanner Database User role to the Service Account
resource "google_spanner_instance_iam_member" "spanner_user" {
instance = var.spanner_instance_id
role = "roles/spanner.databaseUser"
member = "serviceAccount:${google_service_account.dcp_runner.email}"
depends_on = [
google_spanner_instance.main
]
}
| variable "image_url" { | ||
| description = "Docker image URL to deploy" | ||
| type = string | ||
| default = "gcr.io/datcom-ci/datacommons-platform:latest" |
There was a problem hiding this comment.
The default image_url uses the :latest tag. Using mutable tags like :latest is not recommended for deployments as it can lead to unexpected code being deployed if the image is updated. This makes deployments less predictable and rollbacks harder. It is a best practice to use immutable image tags, such as a git commit SHA or a semantic version number (e.g., gcr.io/datcom-ci/datacommons-platform:v1.2.3 or gcr.io/datcom-ci/datacommons-platform:sha-a1b2c3d).
1 participant
This group of terraform script controls the setup of a new Data Commons Platform deployment within a GCP Project that has nothing setup.
Note that after running this in datcom-website-dev, we succeeded with each of the following resources:
And after running the commands in the DCP setup: https://github.com/datacommonsorg/datacommons?tab=readme-ov-file#2-define-your-schema but changing the URL to the new Cloud Run Service, you can inspect the logs to find the requests, and inspect the DB to find the schema and nodes successfully saved!
Note - I have added more optional variables to control the Spanner DB + cloud run service setup but have not yet tested them. Thoughts on including those?
Lastly - I believe the container failed to start due to the Dockerfile missing the "start" command. Not sure how it worked locally, but included the change and that fixed the cloud run server.