feat(infra): add GCP Terraform module for Cloud Run deployment#652
Open
slavarosin wants to merge 3 commits intothe-momentum:mainfrom
Open
feat(infra): add GCP Terraform module for Cloud Run deployment#652slavarosin wants to merge 3 commits intothe-momentum:mainfrom
slavarosin wants to merge 3 commits intothe-momentum:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds a complete GCP Terraform deployment: dev and prod environment entrypoints with GCS remote backends, providers, variables and tfvars examples; a reusable open_wearables_stack module (networking, Cloud Run v2 services/jobs, Cloud SQL, Memorystore, Artifact Registry, Cloud Build triggers, Cloud Tasks, Cloud Scheduler, IAM, secrets, outputs) and lockfiles. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub
participant CloudBuild
participant ArtifactRegistry
participant CloudRun
participant CloudSQL
participant Redis
participant CloudScheduler
participant CloudTasks
GitHub->>CloudBuild: push to branch (trigger)
CloudBuild->>ArtifactRegistry: build & push image
ArtifactRegistry->>CloudRun: deploy image to service
CloudRun->>CloudSQL: connect / execute queries
CloudRun->>Redis: cache reads/writes
CloudScheduler->>CloudTasks: schedule -> enqueue task
CloudTasks->>CloudRun: dispatch task to worker endpoint
CloudRun->>CloudSQL: process task -> DB
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add a reusable Terraform module that provisions the full Open Wearables stack on Google Cloud Platform: - Cloud Run services: API, Worker, Frontend, Init Job (migrations) - Cloud SQL (PostgreSQL) and Memorystore (Redis) - VPC network with Serverless VPC Connector - Artifact Registry for Docker images - Cloud Tasks queues for async task dispatch - Cloud Scheduler for periodic jobs - Secret Manager for credentials - Cloud Build triggers for CI/CD - IAM roles and service accounts Includes example environment configurations (dev/prod) with terraform.tfvars.example templates. All resources are parameterized via variables with sensible defaults. Also adds Terraform-related entries to .gitignore (state files, secrets, plan output).
slavarosin
force-pushed
the
feat/gcp-terraform-module
branch
from
6e3ff3b to
cda3e48
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (5)
infra/gcp/terraform/environments/prod/terraform.tfvars.example (1)
34-36: Use a more obvious placeholder for the password.
"replace-me"could accidentally be deployed. Use a clearly invalid placeholder like"<YOUR_DB_PASSWORD_HERE>"or add a comment indicating this should be sourced from secrets in practice.🔧 Proposed fix
cloud_sql_db_name = "open-wearables" cloud_sql_db_user = "open_wearables" -cloud_sql_db_password = "replace-me" +cloud_sql_db_password = "<YOUR_DB_PASSWORD_HERE>" # Recommended: use Secret Manager instead🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/environments/prod/terraform.tfvars.example` around lines 34 - 36, Update the example Terraform variables so the password placeholder is clearly invalid and documents secrets usage: replace the current cloud_sql_db_password value "replace-me" with an unmistakable placeholder such as "<YOUR_DB_PASSWORD_HERE>" and add a short inline comment indicating this value must be provided from a secret manager in production; ensure cloud_sql_db_name and cloud_sql_db_user remain unchanged but consider adding a similar comment if needed to avoid accidental deployment of example credentials.infra/gcp/terraform/environments/prod/outputs.tf (1)
16-18: Missing description for consistency.All other outputs have descriptions. Add one here for consistency.
🔧 Proposed fix
output "artifact_registry_repository_url" { + description = "The URL of the Artifact Registry repository." value = module.open_wearables_stack.artifact_registry_repository_url }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/environments/prod/outputs.tf` around lines 16 - 18, Add a description field to the Terraform output "artifact_registry_repository_url" so it matches the style of other outputs; update the output block that references module.open_wearables_stack.artifact_registry_repository_url to include a descriptive description string explaining what the URL is (e.g., "Artifact Registry repository URL for wearables artifacts") to maintain consistency and clarity across outputs.infra/gcp/terraform/environments/dev/main.tf (2)
23-30: Same concern: hardcoded instance names.
axl-postgresandaxl-redisare hardcoded. While the comments explain the intent, parameterizing these would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/environments/dev/main.tf` around lines 23 - 30, The hardcoded resource names axl-postgres and axl-redis in the Terraform config should be parameterized: replace the literals used by cloud_sql_instance_name and memorystore_name with variables (e.g., var.cloud_sql_instance_name and var.memorystore_name), add those variables to your variables definitions (variables for cloud_sql_instance_name and memorystore_name with sensible defaults or no default), and update any references that assume the hardcoded names (e.g., places conditional on create_cloud_sql or create_memorystore) so the instance names come from the new variables rather than fixed strings.
20-21: Hardcoded subnet and VPC connector names reduce flexibility.Unlike
prod/main.tfwhich usesvar.vpc_connector_name, these values are hardcoded. Consider parameterizing them for consistency and easier maintenance.🔧 Proposed fix
- subnetwork_name = "axl-srvless-subnet" # Explicitly match AXL if not dynamic - vpc_connector_name = "axl-srvless-connector" + subnetwork_name = var.subnetwork_name + vpc_connector_name = var.vpc_connector_nameThen declare in
variables.tfwith appropriate defaults:variable "subnetwork_name" { description = "Name of the existing subnetwork to use." type = string default = "axl-srvless-subnet" } variable "vpc_connector_name" { description = "Name of the VPC connector." type = string default = "axl-srvless-connector" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/environments/dev/main.tf` around lines 20 - 21, The hardcoded values for subnetwork_name and vpc_connector_name reduce flexibility; replace the literal strings "axl-srvless-subnet" and "axl-srvless-connector" in the main.tf resource/module block with variables (use var.subnetwork_name and var.vpc_connector_name) and add corresponding variable declarations in variables.tf (declare variable "subnetwork_name" and "vpc_connector_name" with descriptions, type = string, and the suggested defaults) so the dev environment matches prod's parameterized approach; ensure any references to these identifiers (subnetwork_name, vpc_connector_name) throughout the Terraform code use the new var.* names.infra/gcp/terraform/modules/open_wearables_stack/iam.tf (1)
13-18: Potential plan failure ifservice_account_project_roleskeys don't matchservice_accountskeys.If a user overrides
service_account_project_roleswith a key that doesn't exist inservice_accounts, Line 18 will fail during plan with a key-not-found error ongoogle_service_account.accounts[each.value.account_key].Consider adding a validation block on
service_account_project_rolesto ensure all keys exist inservice_accounts, or usetry()with a clearer error message.💡 Example validation in variables.tf
variable "service_account_project_roles" { # ... existing definition ... validation { condition = alltrue([ for k in keys(var.service_account_project_roles) : contains(keys(var.service_accounts), k) ]) error_message = "All keys in service_account_project_roles must exist in service_accounts." } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/modules/open_wearables_stack/iam.tf` around lines 13 - 18, The IAM binding loop google_project_iam_member.service_account_roles can fail if keys in var.service_account_project_roles don't match var.service_accounts; add a validation to variable "service_account_project_roles" (or to "service_accounts") to assert every key in service_account_project_roles exists in keys(var.service_accounts), or wrap the lookup google_service_account.accounts[each.value.account_key] in try() and produce a clear error message; reference local.service_account_role_pairs, var.service_account_project_roles, var.service_accounts and google_service_account.accounts when implementing the check so the plan fails with a user-friendly validation error rather than a key-not-found during plan.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infra/gcp/terraform/environments/dev/backend.tf`:
- Around line 2-5: The backend "gcs" block currently hardcodes the bucket
attribute (bucket = "axl-platform-tfstate-602690ff"); remove the hardcoded
bucket line from the backend "gcs" stanza and keep only the prefix (prefix =
"envs/dev/ow-terraform-state") so the backend is only partially configured, then
document/expect supplying the bucket at runtime via terraform init
-backend-config=bucket=<BUCKET>; this avoids committing project-specific state
and enables passing the bucket per-environment during init.
In `@infra/gcp/terraform/environments/dev/terraform.tfvars.example`:
- Around line 43-46: The dev terraform example mistakenly sets ENVIRONMENT =
"production" in the backend_api_env block; update that value to the dev setting
(e.g., "development") and make the same change for backend_worker_env and
backend_init_env so the dev tfvars example reflects the correct environment;
locate the ENVIRONMENT key inside the backend_api_env, backend_worker_env, and
backend_init_env blocks and replace "production" with "development".
- Around line 23-25: The tfvars example sets create_cloud_sql = true and
create_memorystore = true, but main.tf currently ignores those variables by
hardcoding create_cloud_sql = false and create_memorystore = false; either
remove the hardcodes in main.tf and wire the module/locals to use
var.create_cloud_sql and var.create_memorystore (so the example values take
effect), or update the terraform.tfvars.example to set both create_cloud_sql and
create_memorystore to false and add a short comment that these features are
disabled by default; update whichever files reference create_cloud_sql and
create_memorystore to keep config and examples consistent.
In `@infra/gcp/terraform/environments/dev/variables.tf`:
- Around line 1-5: The variable "project_id" currently has a production default
("axl-platform-prod") which risks deploying dev into prod; remove the default
assignment so the variable is required: edit the variable "project_id" block to
keep description and type but delete the default = "axl-platform-prod" line
(leaving no default), causing Terraform to require the value via tfvars or CLI
when deploying the dev environment.
In `@infra/gcp/terraform/environments/prod/backend.tf`:
- Around line 2-5: The GCS backend block hardcodes a project-specific bucket
(backend "gcs" with bucket = "axl-platform-tfstate-602690ff"); remove the bucket
attribute from the backend block (keep prefix if desired) and convert the
backend to a partial config, then require the environment-specific bucket to be
provided at init (e.g., terraform init -backend-config="bucket=...") or via a
CI-provided backend config file; update any docs/CI scripts to pass the
per-environment bucket instead of committing it in the backend block.
In `@infra/gcp/terraform/environments/prod/variables.tf`:
- Around line 274-278: The variable declaration
backend_api_allow_unauthenticated currently defaults to true which enables
public access in prod; change the variable "backend_api_allow_unauthenticated"
default to false in the variable block so production is private by default, and
update any environment-specific overrides (e.g., prod.tfvars or workspace
overrides) to explicitly opt-in where needed and adjust any related docs or
comments mentioning the default behavior.
- Around line 316-320: The variable declaration for
axl_api_service_account_email currently embeds a project-specific default
("axl-run-sa@axl-platform-prod.iam.gserviceaccount.com"); remove the default
value so the variable only declares description and type, leaving it unset and
requiring callers to supply it via tfvars or environment; update the variable
block for axl_api_service_account_email (keeping its description and type) and
ensure any docs or examples instruct users to provide the service account email
in their environment-specific tfvars.
In `@infra/gcp/terraform/environments/prod/versions.tf`:
- Around line 1-3: The terraform block's required_version currently allows ">=
1.6.0" which is weaker than the module's constraint; update the required_version
in the terraform block to match the module's constraint (e.g., set
required_version = ">= 1.13.0, < 2.0.0") so the environment and modules use the
same supported Terraform versions; verify the change in the terraform { ... }
block and ensure it aligns with modules/open_wearables_stack/versions.tf.
In `@infra/gcp/terraform/modules/open_wearables_stack/cloud_build.tf`:
- Around line 18-27: The substitutions block currently dereferences
google_artifact_registry_repository.containers[0] for _REPOSITORY which breaks
planning when create_artifact_registry = false; change resolution to use a local
that returns google_artifact_registry_repository.containers[0].name when the
module is creating the repo and falls back to
var.artifact_registry_repository_id when reusing an existing repo, then update
the substitutions to use that local (i.e., replace direct use of
google_artifact_registry_repository.containers[0] with the new local value
referenced by _REPOSITORY); apply the same fix for the second trigger occurrence
(lines 47-54) so both triggers use the fallback local.
In `@infra/gcp/terraform/modules/open_wearables_stack/cloud_run.tf`:
- Around line 47-56: The current dispatcher_runtime_env hardcodes access to
google_cloud_tasks_queue.queues["default"], ["sdk_sync"], and
["garmin_backfill"], which will break if var.queue_configs is changed; update
the block so it no longer assumes those fixed keys by either adding a validation
on var.queue_configs to require those keys or (preferably) deriving the env
values safely from the configured queues using lookup/contains (or try) against
var.queue_configs and/or google_cloud_tasks_queue.queues and falling back to an
empty string when a queue is absent; target the dispatcher_runtime_env
assignment and references to google_cloud_tasks_queue.queues and
var.queue_configs to implement the safe lookup/fallback or add the validation.
- Around line 14-17: The current fallback for frontend_service_account_email
uses google_service_account.accounts["api"], which causes the public frontend to
run as the API identity; change the fallback to use a dedicated frontend service
account (google_service_account.accounts["frontend"]) or require an explicit
var.frontend_service_account_email when the frontend is enabled; update the
expression that sets frontend_service_account_email (replace
google_service_account.accounts["api"].email with
google_service_account.accounts["frontend"].email or remove the automatic
fallback and add validation that throws if frontend is enabled but no
frontend_service_account_email is provided) so the frontend no longer inherits
backend/API roles.
In `@infra/gcp/terraform/modules/open_wearables_stack/data_services.tf`:
- Around line 64-80: The resources google_sql_database.app and
google_sql_user.app are still created when create_cloud_sql = false but
cloud_sql_instance_name is set, causing unintended mutation of existing infra;
update the count logic so these resources are only created when you intend to
manage DB objects. Either (A) change both count expressions to depend solely on
var.create_cloud_sql (count = var.create_cloud_sql ? 1 : 0) so the module never
creates DB/user when reusing an existing instance, or (B) add a new boolean
input (e.g., var.manage_cloud_sql_resources default false) and set count =
(var.create_cloud_sql || var.manage_cloud_sql_resources) ? 1 : 0 so callers can
explicitly opt in to managing DB/user on an existing instance; apply the same
change to google_sql_database.app and google_sql_user.app.
- Around line 107-125: The google_storage_bucket.task_payloads resource should
respect the declared variables instead of hardcoding creation/name: change its
count to var.create_task_payload_bucket ? 1 : 0, set its name to use
var.task_payload_bucket_name when provided (fallback to
"${local.resource_prefix}-task-payloads" if empty), and add
public_access_prevention = "enforced" to harden access. Update any consumers
(e.g., cloud_run.tf which uses google_storage_bucket.task_payloads[0].name) to
use the configured bucket name variable (var.task_payload_bucket_name) when
create_task_payload_bucket is false or otherwise reference the created resource
conditionally so lookups don't fail when the bucket isn't created. Ensure
references and variable defaults are consistent across the module.
- Around line 42-44: Update the Cloud SQL ip_configuration block to use private
IP: change ipv4_enabled to false and add private_network referencing the VPC
self-link (use google_compute_network.main[0].self_link). Ensure this
ip_configuration change aligns with the existing
google_service_networking_connection.private_vpc_connection dependency so the
instance uses the private VPC connection.
In `@infra/gcp/terraform/modules/open_wearables_stack/main.tf`:
- Around line 70-76: The resource google_secret_manager_secret_version uses
for_each = var.secret_values but then indexes
google_secret_manager_secret.secrets[each.key], which fails if keys differ;
change the iteration to use the keys of the provided map (e.g., for_each =
toset(keys(var.secret_values)) or equivalent) so you only iterate over actual
secret keys, or add an input validation block that enforces
keys(var.secret_values) is a subset of the declared secret set (the variable
backing google_secret_manager_secret.secrets) to prevent drift; update
references in google_secret_manager_secret_version and any related resources to
use each.key from the chosen iteration set (or adjust variable validation) so
plans cannot produce invalid index errors.
In `@infra/gcp/terraform/modules/open_wearables_stack/network.tf`:
- Around line 36-44: The VPC resource google_compute_network.main is created
without any firewall rules; add explicit, configurable least-privilege firewall
resources (e.g., create google_compute_firewall resources tied to the network)
or expose a variable such as
var.default_firewall_rules/var.create_default_firewall to control them;
implement at minimum a deny-by-default posture with explicit allow rules for
required management and internal traffic (for example allow internal CIDR
traffic and scoped SSH/RDP only from approved CIDRs) and ensure new
google_compute_firewall resources depend on google_compute_network.main (or
reuse local.network_name_resolved) so they attach to the correct network and are
configurable via the module inputs.
- Line 33: The vpc_connector_id expression dereferences
google_vpc_access_connector.main[0].id unconditionally which errors when that
resource has count = 0; update the logic to avoid direct indexing when the
resource may not exist: use a safe try around the resource access (e.g.
try(google_vpc_access_connector.main[0].id, null)) and/or make the resource
creation count conditional on both var.create_vpc_connector and
var.create_network so indexing only occurs when the resource exists; apply the
same defensive change to the other occurrences in the block that reference
google_vpc_access_connector.main[0].id (lines referencing
google_vpc_access_connector.main and the vpc_connector_id expression).
In `@infra/gcp/terraform/modules/open_wearables_stack/runtime_outputs.tf`:
- Around line 1-29: The outputs (network_name, vpc_connector_id,
cloud_sql_connection_name, cloud_sql_instance_name, redis_host, redis_port)
currently pull only from the resource arrays and return null when the module is
attached to existing infra; change each output value to use the already-resolved
locals instead (e.g., replace length(google_compute_network.main) ... one(...)
with local.network_name, replace the google_vpc_access_connector expression with
local.vpc_connector_id, the google_sql_database_instance expressions with
local.cloud_sql_connection_name and local.cloud_sql_instance_name, and the
google_redis_instance expressions with local.redis_host and local.redis_port) so
the module exposes resolved connection details whether resources were created or
referenced.
In `@infra/gcp/terraform/modules/open_wearables_stack/variables.tf`:
- Around line 708-712: The variable declaration for cloud_sql_db_password is
missing sensitivity metadata; update the variable "cloud_sql_db_password" in
variables.tf to include sensitive = true so Terraform redacts it in
CLI/plan/apply output and logs. Locate the variable block for
cloud_sql_db_password and add the sensitive attribute (sensitive = true) while
keeping description, type, and default intact; ensure any references to
cloud_sql_db_password remain unchanged.
---
Nitpick comments:
In `@infra/gcp/terraform/environments/dev/main.tf`:
- Around line 23-30: The hardcoded resource names axl-postgres and axl-redis in
the Terraform config should be parameterized: replace the literals used by
cloud_sql_instance_name and memorystore_name with variables (e.g.,
var.cloud_sql_instance_name and var.memorystore_name), add those variables to
your variables definitions (variables for cloud_sql_instance_name and
memorystore_name with sensible defaults or no default), and update any
references that assume the hardcoded names (e.g., places conditional on
create_cloud_sql or create_memorystore) so the instance names come from the new
variables rather than fixed strings.
- Around line 20-21: The hardcoded values for subnetwork_name and
vpc_connector_name reduce flexibility; replace the literal strings
"axl-srvless-subnet" and "axl-srvless-connector" in the main.tf resource/module
block with variables (use var.subnetwork_name and var.vpc_connector_name) and
add corresponding variable declarations in variables.tf (declare variable
"subnetwork_name" and "vpc_connector_name" with descriptions, type = string, and
the suggested defaults) so the dev environment matches prod's parameterized
approach; ensure any references to these identifiers (subnetwork_name,
vpc_connector_name) throughout the Terraform code use the new var.* names.
In `@infra/gcp/terraform/environments/prod/outputs.tf`:
- Around line 16-18: Add a description field to the Terraform output
"artifact_registry_repository_url" so it matches the style of other outputs;
update the output block that references
module.open_wearables_stack.artifact_registry_repository_url to include a
descriptive description string explaining what the URL is (e.g., "Artifact
Registry repository URL for wearables artifacts") to maintain consistency and
clarity across outputs.
In `@infra/gcp/terraform/environments/prod/terraform.tfvars.example`:
- Around line 34-36: Update the example Terraform variables so the password
placeholder is clearly invalid and documents secrets usage: replace the current
cloud_sql_db_password value "replace-me" with an unmistakable placeholder such
as "<YOUR_DB_PASSWORD_HERE>" and add a short inline comment indicating this
value must be provided from a secret manager in production; ensure
cloud_sql_db_name and cloud_sql_db_user remain unchanged but consider adding a
similar comment if needed to avoid accidental deployment of example credentials.
In `@infra/gcp/terraform/modules/open_wearables_stack/iam.tf`:
- Around line 13-18: The IAM binding loop
google_project_iam_member.service_account_roles can fail if keys in
var.service_account_project_roles don't match var.service_accounts; add a
validation to variable "service_account_project_roles" (or to
"service_accounts") to assert every key in service_account_project_roles exists
in keys(var.service_accounts), or wrap the lookup
google_service_account.accounts[each.value.account_key] in try() and produce a
clear error message; reference local.service_account_role_pairs,
var.service_account_project_roles, var.service_accounts and
google_service_account.accounts when implementing the check so the plan fails
with a user-friendly validation error rather than a key-not-found during plan.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d320342-7ecd-4630-aca7-d97bbe35e06d
📒 Files selected for processing (26)
.gitignoreinfra/gcp/terraform/environments/dev/backend.tfinfra/gcp/terraform/environments/dev/main.tfinfra/gcp/terraform/environments/dev/outputs.tfinfra/gcp/terraform/environments/dev/providers.tfinfra/gcp/terraform/environments/dev/terraform.tfvars.exampleinfra/gcp/terraform/environments/dev/variables.tfinfra/gcp/terraform/environments/dev/versions.tfinfra/gcp/terraform/environments/prod/backend.tfinfra/gcp/terraform/environments/prod/main.tfinfra/gcp/terraform/environments/prod/outputs.tfinfra/gcp/terraform/environments/prod/providers.tfinfra/gcp/terraform/environments/prod/terraform.tfvars.exampleinfra/gcp/terraform/environments/prod/variables.tfinfra/gcp/terraform/environments/prod/versions.tfinfra/gcp/terraform/modules/open_wearables_stack/.terraform.lock.hclinfra/gcp/terraform/modules/open_wearables_stack/cloud_build.tfinfra/gcp/terraform/modules/open_wearables_stack/cloud_run.tfinfra/gcp/terraform/modules/open_wearables_stack/data_services.tfinfra/gcp/terraform/modules/open_wearables_stack/iam.tfinfra/gcp/terraform/modules/open_wearables_stack/main.tfinfra/gcp/terraform/modules/open_wearables_stack/network.tfinfra/gcp/terraform/modules/open_wearables_stack/outputs.tfinfra/gcp/terraform/modules/open_wearables_stack/runtime_outputs.tfinfra/gcp/terraform/modules/open_wearables_stack/variables.tfinfra/gcp/terraform/modules/open_wearables_stack/versions.tf
Comment on lines
+2
to
+5
| backend "gcs" { | ||
| prefix = "envs/dev/ow-terraform-state" | ||
| bucket = "axl-platform-tfstate-602690ff" | ||
| } |
There was a problem hiding this comment.
Avoid hardcoded backend bucket in committed environment config.
Line 4 hardcodes a project-specific GCS bucket, which conflicts with the “no hardcoded project IDs/domains” objective and makes environment cloning brittle. Prefer partial backend config and pass bucket via -backend-config at init time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/environments/dev/backend.tf` around lines 2 - 5, The
backend "gcs" block currently hardcodes the bucket attribute (bucket =
"axl-platform-tfstate-602690ff"); remove the hardcoded bucket line from the
backend "gcs" stanza and keep only the prefix (prefix =
"envs/dev/ow-terraform-state") so the backend is only partially configured, then
document/expect supplying the bucket at runtime via terraform init
-backend-config=bucket=<BUCKET>; this avoids committing project-specific state
and enables passing the bucket per-environment during init.
Comment on lines
+23
to
+25
| create_network = true | ||
| create_cloud_sql = true | ||
| create_memorystore = true |
There was a problem hiding this comment.
Mismatch with dev/main.tf hardcoded values.
This example sets create_cloud_sql = true and create_memorystore = true, but dev/main.tf hardcodes create_cloud_sql = false and create_memorystore = false (Lines 23, 29). These tfvars values will be ignored.
Either update dev/main.tf to use var.create_cloud_sql and var.create_memorystore, or update this example to reflect the hardcoded behavior:
🔧 Option: Update example to match reality
create_network = true
-create_cloud_sql = true
-create_memorystore = true
+create_cloud_sql = false # dev/main.tf uses existing AXL instance
+create_memorystore = false # dev/main.tf uses existing AXL Redis📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| create_network = true | |
| create_cloud_sql = true | |
| create_memorystore = true | |
| create_network = true | |
| create_cloud_sql = false # dev/main.tf uses existing AXL instance | |
| create_memorystore = false # dev/main.tf uses existing AXL Redis |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/environments/dev/terraform.tfvars.example` around lines
23 - 25, The tfvars example sets create_cloud_sql = true and create_memorystore
= true, but main.tf currently ignores those variables by hardcoding
create_cloud_sql = false and create_memorystore = false; either remove the
hardcodes in main.tf and wire the module/locals to use var.create_cloud_sql and
var.create_memorystore (so the example values take effect), or update the
terraform.tfvars.example to set both create_cloud_sql and create_memorystore to
false and add a short comment that these features are disabled by default;
update whichever files reference create_cloud_sql and create_memorystore to keep
config and examples consistent.
Comment on lines
+43
to
+46
| backend_api_env = { | ||
| ENVIRONMENT = "production" | ||
| CORS_ORIGINS = "https://frontend-dev.example.run.app" | ||
| } |
There was a problem hiding this comment.
ENVIRONMENT = "production" appears incorrect for dev.
The dev environment example sets ENVIRONMENT = "production" in multiple places. This looks like a copy/paste error from the prod example.
🔧 Proposed fix
backend_api_env = {
- ENVIRONMENT = "production"
+ ENVIRONMENT = "development"
CORS_ORIGINS = "https://frontend-dev.example.run.app"
}Apply similar changes to backend_worker_env and backend_init_env.
Also applies to: 55-57, 66-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/environments/dev/terraform.tfvars.example` around lines
43 - 46, The dev terraform example mistakenly sets ENVIRONMENT = "production" in
the backend_api_env block; update that value to the dev setting (e.g.,
"development") and make the same change for backend_worker_env and
backend_init_env so the dev tfvars example reflects the correct environment;
locate the ENVIRONMENT key inside the backend_api_env, backend_worker_env, and
backend_init_env blocks and replace "production" with "development".
Comment on lines
+1
to
+5
| variable "project_id" { | ||
| description = "Google Cloud project ID." | ||
| type = string | ||
| default = "axl-platform-prod" | ||
| } |
There was a problem hiding this comment.
Remove the production project default from the dev environment.
Line 4 defaults project_id to axl-platform-prod, so a missing tfvars can deploy dev infra into prod by mistake. Make this input required.
Suggested fix
variable "project_id" {
description = "Google Cloud project ID."
type = string
- default = "axl-platform-prod"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| variable "project_id" { | |
| description = "Google Cloud project ID." | |
| type = string | |
| default = "axl-platform-prod" | |
| } | |
| variable "project_id" { | |
| description = "Google Cloud project ID." | |
| type = string | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/environments/dev/variables.tf` around lines 1 - 5, The
variable "project_id" currently has a production default ("axl-platform-prod")
which risks deploying dev into prod; remove the default assignment so the
variable is required: edit the variable "project_id" block to keep description
and type but delete the default = "axl-platform-prod" line (leaving no default),
causing Terraform to require the value via tfvars or CLI when deploying the dev
environment.
Comment on lines
+2
to
+5
| backend "gcs" { | ||
| prefix = "envs/prod/ow-terraform-state" | ||
| bucket = "axl-platform-tfstate-602690ff" | ||
| } |
There was a problem hiding this comment.
Same backend portability issue in prod: hardcoded bucket.
Line 4 embeds a specific backend bucket. Recommend using partial backend config and supplying bucket per environment at init, instead of committing project-specific state bucket names.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/environments/prod/backend.tf` around lines 2 - 5, The GCS
backend block hardcodes a project-specific bucket (backend "gcs" with bucket =
"axl-platform-tfstate-602690ff"); remove the bucket attribute from the backend
block (keep prefix if desired) and convert the backend to a partial config, then
require the environment-specific bucket to be provided at init (e.g., terraform
init -backend-config="bucket=...") or via a CI-provided backend config file;
update any docs/CI scripts to pass the per-environment bucket instead of
committing it in the backend block.
Comment on lines
+70
to
+76
| resource "google_secret_manager_secret_version" "versions" { | ||
| for_each = var.create_secrets ? var.secret_values : {} | ||
|
|
||
| secret = google_secret_manager_secret.secrets[each.key].id | ||
| secret_data = each.value | ||
|
|
||
| depends_on = [google_secret_manager_secret.secrets] |
There was a problem hiding this comment.
Validate secret_values against the declared secret set.
for_each = var.secret_values is followed by google_secret_manager_secret.secrets[each.key]. If a caller adds a value but forgets to add the same key to secret_names, planning fails with an invalid index. Either derive the secret set from keys(var.secret_values) or add input validation so the two variables cannot drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/modules/open_wearables_stack/main.tf` around lines 70 -
76, The resource google_secret_manager_secret_version uses for_each =
var.secret_values but then indexes
google_secret_manager_secret.secrets[each.key], which fails if keys differ;
change the iteration to use the keys of the provided map (e.g., for_each =
toset(keys(var.secret_values)) or equivalent) so you only iterate over actual
secret keys, or add an input validation block that enforces
keys(var.secret_values) is a subset of the declared secret set (the variable
backing google_secret_manager_secret.secrets) to prevent drift; update
references in google_secret_manager_secret_version and any related resources to
use each.key from the chosen iteration set (or adjust variable validation) so
plans cannot produce invalid index errors.
| locals { | ||
| network_id = var.create_network ? google_compute_network.main[0].id : try(data.google_compute_network.main[0].id, null) | ||
| subnetwork_name = var.create_network ? google_compute_subnetwork.main[0].name : try(data.google_compute_subnetwork.main[0].name, var.subnetwork_name) | ||
| vpc_connector_id = var.create_vpc_connector ? google_vpc_access_connector.main[0].id : try(data.google_vpc_access_connector.main[0].id, var.vpc_connector_name != null ? "projects/${var.project_id}/locations/${var.region}/connectors/${var.vpc_connector_name}" : null) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the problematic gating/indexing pattern exists in this module.
rg -n -C2 'vpc_connector_id|google_vpc_access_connector|count = var\.create_vpc_connector.*var\.create_network|main\[0\]' infra/gcp/terraform/modules/open_wearables_stack/network.tfRepository: the-momentum/open-wearables
Length of output: 1789
🏁 Script executed:
rg -n 'google_vpc_access_connector\.main' infra/gcp/terraform/modules/open_wearables_stack/network.tfRepository: the-momentum/open-wearables
Length of output: 342
vpc_connector_id fails with index error when reusing existing network.
If create_vpc_connector = true and create_network = false, the google_vpc_access_connector.main resource has count = 0 (Line 58) but Line 33 unconditionally dereferences [0]. Terraform plan fails with invalid index during the "use existing VPC" scenario.
Suggested fix
-data "google_vpc_access_connector" "main" {
- count = var.create_vpc_connector ? 0 : (var.vpc_connector_name != null ? 1 : 0)
+data "google_vpc_access_connector" "main" {
+ count = var.create_vpc_connector ? 0 : (var.vpc_connector_name != null ? 1 : 0)
name = var.vpc_connector_name
region = var.region
project = var.project_id
}
@@
-locals {
- vpc_connector_id = var.create_vpc_connector ? google_vpc_access_connector.main[0].id : try(data.google_vpc_access_connector.main[0].id, var.vpc_connector_name != null ? "projects/${var.project_id}/locations/${var.region}/connectors/${var.vpc_connector_name}" : null)
-}
+locals {
+ vpc_connector_id = try(
+ google_vpc_access_connector.main[0].id,
+ data.google_vpc_access_connector.main[0].id,
+ var.vpc_connector_name != null ? "projects/${var.project_id}/locations/${var.region}/connectors/${var.vpc_connector_name}" : null
+ )
+}
@@
-resource "google_vpc_access_connector" "main" {
- count = var.create_vpc_connector && var.create_network ? 1 : 0
+resource "google_vpc_access_connector" "main" {
+ count = var.create_vpc_connector ? 1 : 0
@@
- network = google_compute_network.main[0].id
+ network = var.create_network ? google_compute_network.main[0].name : var.network_name
ip_cidr_range = "10.11.0.0/28"
+
+ lifecycle {
+ precondition {
+ condition = var.create_network || var.network_name != null
+ error_message = "network_name must be set when create_vpc_connector=true and create_network=false."
+ }
+ }Also applies to: 58–68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/modules/open_wearables_stack/network.tf` at line 33, The
vpc_connector_id expression dereferences google_vpc_access_connector.main[0].id
unconditionally which errors when that resource has count = 0; update the logic
to avoid direct indexing when the resource may not exist: use a safe try around
the resource access (e.g. try(google_vpc_access_connector.main[0].id, null))
and/or make the resource creation count conditional on both
var.create_vpc_connector and var.create_network so indexing only occurs when the
resource exists; apply the same defensive change to the other occurrences in the
block that reference google_vpc_access_connector.main[0].id (lines referencing
google_vpc_access_connector.main and the vpc_connector_id expression).
Comment on lines
+36
to
+44
| resource "google_compute_network" "main" { | ||
| count = var.create_network ? 1 : 0 | ||
|
|
||
| project = var.project_id | ||
| name = local.network_name_resolved | ||
| auto_create_subnetworks = false | ||
|
|
||
| depends_on = [google_project_service.required] | ||
| } |
There was a problem hiding this comment.
Add explicit firewall policy when creating the VPC.
The module creates a custom network but no firewall resources. This weakens security posture and triggers CKV2_GCP_18; please add explicit least-privilege firewall rules (or make them configurable defaults).
🧰 Tools
🪛 Checkov (3.2.508)
[medium] 36-44: Ensure GCP network defines a firewall and does not use the default firewall
(CKV2_GCP_18)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/modules/open_wearables_stack/network.tf` around lines 36
- 44, The VPC resource google_compute_network.main is created without any
firewall rules; add explicit, configurable least-privilege firewall resources
(e.g., create google_compute_firewall resources tied to the network) or expose a
variable such as var.default_firewall_rules/var.create_default_firewall to
control them; implement at minimum a deny-by-default posture with explicit allow
rules for required management and internal traffic (for example allow internal
CIDR traffic and scoped SSH/RDP only from approved CIDRs) and ensure new
google_compute_firewall resources depend on google_compute_network.main (or
reuse local.network_name_resolved) so they attach to the correct network and are
configurable via the module inputs.
Comment on lines
+1
to
+29
| output "network_name" { | ||
| description = "Created VPC network name." | ||
| value = length(google_compute_network.main) > 0 ? one(google_compute_network.main[*].name) : null | ||
| } | ||
|
|
||
| output "vpc_connector_id" { | ||
| description = "Serverless VPC Access connector ID." | ||
| value = length(google_vpc_access_connector.main) > 0 ? one(google_vpc_access_connector.main[*].id) : null | ||
| } | ||
|
|
||
| output "cloud_sql_connection_name" { | ||
| description = "Cloud SQL connection name." | ||
| value = length(google_sql_database_instance.main) > 0 ? one(google_sql_database_instance.main[*].connection_name) : null | ||
| } | ||
|
|
||
| output "cloud_sql_instance_name" { | ||
| description = "Cloud SQL instance name." | ||
| value = length(google_sql_database_instance.main) > 0 ? one(google_sql_database_instance.main[*].name) : null | ||
| } | ||
|
|
||
| output "redis_host" { | ||
| description = "Memorystore Redis host." | ||
| value = length(google_redis_instance.main) > 0 ? one(google_redis_instance.main[*].host) : null | ||
| } | ||
|
|
||
| output "redis_port" { | ||
| description = "Memorystore Redis port." | ||
| value = length(google_redis_instance.main) > 0 ? one(google_redis_instance.main[*].port) : null | ||
| } |
There was a problem hiding this comment.
Expose resolved values for reused infrastructure too.
These outputs only read the created resources, so they become null whenever the module attaches to an existing VPC connector, Cloud SQL instance, or Redis instance via the data sources/locals in infra/gcp/terraform/modules/open_wearables_stack/network.tf:30-34 and infra/gcp/terraform/modules/open_wearables_stack/data_services.tf:19-24. That breaks the reuse path and causes downstream outputs like infra/gcp/terraform/environments/dev/outputs.tf:36-44 to lose valid connection details. Prefer the already-resolved locals here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/modules/open_wearables_stack/runtime_outputs.tf` around
lines 1 - 29, The outputs (network_name, vpc_connector_id,
cloud_sql_connection_name, cloud_sql_instance_name, redis_host, redis_port)
currently pull only from the resource arrays and return null when the module is
attached to existing infra; change each output value to use the already-resolved
locals instead (e.g., replace length(google_compute_network.main) ... one(...)
with local.network_name, replace the google_vpc_access_connector expression with
local.vpc_connector_id, the google_sql_database_instance expressions with
local.cloud_sql_connection_name and local.cloud_sql_instance_name, and the
google_redis_instance expressions with local.redis_host and local.redis_port) so
the module exposes resolved connection details whether resources were created or
referenced.
Comment on lines
+708
to
+712
| variable "cloud_sql_db_password" { | ||
| description = "PostgreSQL database password." | ||
| type = string | ||
| default = null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep "variables.tf"Repository: the-momentum/open-wearables
Length of output: 233
🏁 Script executed:
find . -name "variables.tf" -type f 2>/dev/null | head -10Repository: the-momentum/open-wearables
Length of output: 239
🏁 Script executed:
pwd && find . -type d -name "open_wearables_stack" 2>/dev/nullRepository: the-momentum/open-wearables
Length of output: 140
🏁 Script executed:
fd -t f "variables.tf" --path-separator /Repository: the-momentum/open-wearables
Length of output: 233
🏁 Script executed:
rg "cloud_sql_db_password" --type tf -A 3 -B 1Repository: the-momentum/open-wearables
Length of output: 3821
🏁 Script executed:
git ls-files | grep -i terraform | head -20Repository: the-momentum/open-wearables
Length of output: 1144
🏁 Script executed:
find . -type f -name "variables.tf" 2>/dev/null | xargs grep -l "cloud_sql_db_password" 2>/dev/nullRepository: the-momentum/open-wearables
Length of output: 239
🏁 Script executed:
rg "variable.*cloud_sql_db_password" --type tf -A 5Repository: the-momentum/open-wearables
Length of output: 1482
🏁 Script executed:
fd -e tf -x basename {} \; | sort | uniqRepository: the-momentum/open-wearables
Length of output: 220
🏁 Script executed:
find . -type f \( -name "*.tf" \) 2>/dev/null | wc -lRepository: the-momentum/open-wearables
Length of output: 73
🏁 Script executed:
wc -l infra/gcp/terraform/modules/open_wearables_stack/variables.tfRepository: the-momentum/open-wearables
Length of output: 136
🏁 Script executed:
sed -n '700,720p' infra/gcp/terraform/modules/open_wearables_stack/variables.tfRepository: the-momentum/open-wearables
Length of output: 538
🏁 Script executed:
rg "secret_values" infra/gcp/terraform/modules/open_wearables_stack/variables.tf -A 3Repository: the-momentum/open-wearables
Length of output: 247
🏁 Script executed:
cat -n infra/gcp/terraform/modules/open_wearables_stack/variables.tf | grep -A 10 "cloud_sql_db_password"Repository: the-momentum/open-wearables
Length of output: 449
🏁 Script executed:
rg "cloud_sql_db_password" infra/gcp/terraform/modules/open_wearables_stack/outputs.tfRepository: the-momentum/open-wearables
Length of output: 53
🏁 Script executed:
rg "DB_PASSWORD" infra/gcp/terraform/modules/open_wearables_stack/ --type tfRepository: the-momentum/open-wearables
Length of output: 223
🏁 Script executed:
rg "variable \"secret_values\"" infra/gcp/terraform/modules/open_wearables_stack/variables.tf -A 5Repository: the-momentum/open-wearables
Length of output: 270
🏁 Script executed:
rg "sensitive.*true" infra/gcp/terraform/modules/open_wearables_stack/variables.tfRepository: the-momentum/open-wearables
Length of output: 91
🏁 Script executed:
cat infra/gcp/terraform/modules/open_wearables_stack/outputs.tfRepository: the-momentum/open-wearables
Length of output: 1501
Mark the DB password input as sensitive.
cloud_sql_db_password is the one secret-like input here that is not declared sensitive, so Terraform will not automatically redact it everywhere the variable is reused. Setting sensitive = true hides it in plan/apply output and CLI logs, even though Terraform still persists the value in state. This inconsistency is especially important given that the password is used as an environment variable in Cloud Run and as a resource parameter in the Cloud SQL database user resource.
Suggested change
variable "cloud_sql_db_password" {
description = "PostgreSQL database password."
type = string
default = null
+ sensitive = true
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| variable "cloud_sql_db_password" { | |
| description = "PostgreSQL database password." | |
| type = string | |
| default = null | |
| } | |
| variable "cloud_sql_db_password" { | |
| description = "PostgreSQL database password." | |
| type = string | |
| default = null | |
| sensitive = true | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/modules/open_wearables_stack/variables.tf` around lines
708 - 712, The variable declaration for cloud_sql_db_password is missing
sensitivity metadata; update the variable "cloud_sql_db_password" in
variables.tf to include sensitive = true so Terraform redacts it in
CLI/plan/apply output and logs. Locate the variable block for
cloud_sql_db_password and add the sensitive attribute (sensitive = true) while
keeping description, type, and default intact; ensure any references to
cloud_sql_db_password remain unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
infra/gcp/terraform/modules/open_wearables_stack/cloud_run.tf (1)
79-83: Worker invoker members may contain null emails.
local.worker_invoker_membersusescompact()to remove nulls, butlocal.task_dispatch_invoker_service_account_emailandlocal.scheduler_service_account_emailcould be null if service accounts aren't created and no explicit email is provided. This could result in an empty set when invocations are expected.🛡️ Consider adding a precondition
Add validation to ensure the required service account emails are available when their features are enabled:
lifecycle { precondition { condition = !var.enable_cloud_tasks_dispatch || local.task_dispatch_invoker_service_account_email != null error_message = "A task dispatcher service account email must be available when enable_cloud_tasks_dispatch is true." } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/modules/open_wearables_stack/cloud_run.tf` around lines 79 - 83, local.worker_invoker_members can become empty because local.task_dispatch_invoker_service_account_email and local.scheduler_service_account_email may be null while their feature flags (var.enable_cloud_tasks_dispatch, var.create_default_scheduler_jobs) are true; add validation to the relevant resource (or module) to require those emails when the features are enabled by adding lifecycle precondition checks that assert !var.enable_cloud_tasks_dispatch || local.task_dispatch_invoker_service_account_email != null and !var.create_default_scheduler_jobs || local.scheduler_service_account_email != null with clear error_message strings so invocations aren’t left without a valid service account email.infra/gcp/terraform/modules/open_wearables_stack/network.tf (2)
67-68: Hardcoded VPC connector CIDR may conflict with user networks.The
ip_cidr_range = "10.11.0.0/28"is hardcoded. When connecting to an existing network, this CIDR might overlap with existing allocations. Consider making this configurable via a variable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/modules/open_wearables_stack/network.tf` around lines 67 - 68, The hardcoded CIDR ("10.11.0.0/28") for the VPC connector can overlap user networks; replace this literal with a configurable variable (e.g., var.vpc_connector_cidr) and update any references (the resource setting ip_cidr_range and any module inputs) so consumers can override it; add a sensible default and optionally a validation rule in variables.tf to ensure a /28 CIDR format and document the new variable where google_compute_network.main[0].id and ip_cidr_range are used.
57-71: VPC connector can only be created with a new network.The count condition on line 58 requires both
create_vpc_connector && create_network, which means users cannot create a VPC connector against an existing network. If this is intentional, document it; otherwise, consider allowing connector creation with an existing network by acceptingvar.network_nameas fallback.♻️ Proposed fix to support existing networks
resource "google_vpc_access_connector" "main" { - count = var.create_vpc_connector && var.create_network ? 1 : 0 + count = var.create_vpc_connector ? 1 : 0 project = var.project_id name = local.vpc_connector_name_resolved region = var.region machine_type = var.vpc_connector_machine_type min_instances = var.vpc_connector_min_instances max_instances = var.vpc_connector_max_instances - network = google_compute_network.main[0].id + network = var.create_network ? google_compute_network.main[0].name : var.network_name ip_cidr_range = "10.11.0.0/28" + lifecycle { + precondition { + condition = var.create_network || var.network_name != null + error_message = "network_name must be set when create_vpc_connector=true and create_network=false." + } + } + depends_on = [google_project_service.required] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/modules/open_wearables_stack/network.tf` around lines 57 - 71, The google_vpc_access_connector resource is gated by count = var.create_vpc_connector && var.create_network which prevents creating a connector for an existing network; update the condition to allow creation when create_vpc_connector is true and either a new network is being created or an existing network name is provided (e.g., var.create_vpc_connector && (var.create_network || var.network_name != "")), and change the network attribute to select the correct id depending on which is used (use google_compute_network.main[0].id when creating a network, otherwise use a data source like data.google_compute_network.existing.id resolved from var.network_name). Also add the new var.network_name and a data.google_compute_network block (referenced above) so the resource google_vpc_access_connector.main can attach to an existing network when requested.infra/gcp/terraform/modules/open_wearables_stack/main.tf (1)
25-34: Service account ID truncation may cause collisions.The 30-character truncation on line 29 could produce duplicate
account_idvalues whenname_prefixis long enough that differenteach.keyvalues get truncated to the same string. Consider adding validation or using a hash suffix for uniqueness.♻️ Proposed validation to catch collisions
resource "google_service_account" "accounts" { for_each = var.create_service_accounts ? var.service_accounts : {} project = var.project_id account_id = substr(replace("${local.resource_prefix}-${each.key}", "_", "-"), 0, 30) display_name = each.value.display_name description = each.value.description depends_on = [google_project_service.required] + + lifecycle { + precondition { + condition = length("${local.resource_prefix}-${each.key}") <= 30 || alltrue([ + for k, v in var.service_accounts : + substr(replace("${local.resource_prefix}-${k}", "_", "-"), 0, 30) != substr(replace("${local.resource_prefix}-${each.key}", "_", "-"), 0, 30) || k == each.key + ]) + error_message = "Service account ID truncation would cause a collision. Use a shorter name_prefix." + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/modules/open_wearables_stack/main.tf` around lines 25 - 34, The google_service_account "accounts" resource currently builds account_id with substr(replace("${local.resource_prefix}-${each.key}", "_", "-"), 0, 30) which can truncate distinct keys to identical IDs; change account_id construction to ensure uniqueness by appending a short deterministic hash suffix (e.g., a 6-char sha256/hex of each.key or "${each.key}") after truncating the readable prefix, and/or add a validation block (using locals and a for/contains check) that detects duplicate account_ids at plan time; update references to local.resource_prefix and each.key in the new expression and ensure the combined length stays within the provider limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infra/gcp/terraform/modules/open_wearables_stack/cloud_run.tf`:
- Around line 32-45: Remove the plain DB_PASSWORD injection from
backend_runtime_env and instead reference the secret via the Cloud Run secret
env pattern used elsewhere: add DB password to the secret env maps (e.g.,
backend_api_secret_env and/or backend_worker_secret_env) and stop merging
var.cloud_sql_db_password into backend_runtime_env; use the same
value_source.secret_key_ref approach used for
backend_api_secret_env/backend_worker_secret_env so the Cloud Run service reads
DB_PASSWORD from Secret Manager rather than an inline env var (update any
references to var.cloud_sql_db_password and local.cloud_sql_connection_name
accordingly).
In `@infra/gcp/terraform/modules/open_wearables_stack/data_services.tf`:
- Around line 82-105: The google_redis_instance resource
(google_redis_instance.main) must enable Redis AUTH; add the attribute
auth_enabled = true inside the resource block (alongside name, tier,
memory_size_gb, etc.) so the instance is created with authentication enabled;
after adding auth_enabled, ensure any provisioning or module inputs that expect
unauthenticated access are updated to use authenticated connections/credentials.
---
Nitpick comments:
In `@infra/gcp/terraform/modules/open_wearables_stack/cloud_run.tf`:
- Around line 79-83: local.worker_invoker_members can become empty because
local.task_dispatch_invoker_service_account_email and
local.scheduler_service_account_email may be null while their feature flags
(var.enable_cloud_tasks_dispatch, var.create_default_scheduler_jobs) are true;
add validation to the relevant resource (or module) to require those emails when
the features are enabled by adding lifecycle precondition checks that assert
!var.enable_cloud_tasks_dispatch ||
local.task_dispatch_invoker_service_account_email != null and
!var.create_default_scheduler_jobs || local.scheduler_service_account_email !=
null with clear error_message strings so invocations aren’t left without a valid
service account email.
In `@infra/gcp/terraform/modules/open_wearables_stack/main.tf`:
- Around line 25-34: The google_service_account "accounts" resource currently
builds account_id with substr(replace("${local.resource_prefix}-${each.key}",
"_", "-"), 0, 30) which can truncate distinct keys to identical IDs; change
account_id construction to ensure uniqueness by appending a short deterministic
hash suffix (e.g., a 6-char sha256/hex of each.key or "${each.key}") after
truncating the readable prefix, and/or add a validation block (using locals and
a for/contains check) that detects duplicate account_ids at plan time; update
references to local.resource_prefix and each.key in the new expression and
ensure the combined length stays within the provider limit.
In `@infra/gcp/terraform/modules/open_wearables_stack/network.tf`:
- Around line 67-68: The hardcoded CIDR ("10.11.0.0/28") for the VPC connector
can overlap user networks; replace this literal with a configurable variable
(e.g., var.vpc_connector_cidr) and update any references (the resource setting
ip_cidr_range and any module inputs) so consumers can override it; add a
sensible default and optionally a validation rule in variables.tf to ensure a
/28 CIDR format and document the new variable where
google_compute_network.main[0].id and ip_cidr_range are used.
- Around line 57-71: The google_vpc_access_connector resource is gated by count
= var.create_vpc_connector && var.create_network which prevents creating a
connector for an existing network; update the condition to allow creation when
create_vpc_connector is true and either a new network is being created or an
existing network name is provided (e.g., var.create_vpc_connector &&
(var.create_network || var.network_name != "")), and change the network
attribute to select the correct id depending on which is used (use
google_compute_network.main[0].id when creating a network, otherwise use a data
source like data.google_compute_network.existing.id resolved from
var.network_name). Also add the new var.network_name and a
data.google_compute_network block (referenced above) so the resource
google_vpc_access_connector.main can attach to an existing network when
requested.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28286b2a-5ce9-4903-a5dc-44857aa3d054
📒 Files selected for processing (26)
.gitignoreinfra/gcp/terraform/environments/dev/backend.tfinfra/gcp/terraform/environments/dev/main.tfinfra/gcp/terraform/environments/dev/outputs.tfinfra/gcp/terraform/environments/dev/providers.tfinfra/gcp/terraform/environments/dev/terraform.tfvars.exampleinfra/gcp/terraform/environments/dev/variables.tfinfra/gcp/terraform/environments/dev/versions.tfinfra/gcp/terraform/environments/prod/.terraform.lock.hclinfra/gcp/terraform/environments/prod/backend.tfinfra/gcp/terraform/environments/prod/main.tfinfra/gcp/terraform/environments/prod/outputs.tfinfra/gcp/terraform/environments/prod/providers.tfinfra/gcp/terraform/environments/prod/terraform.tfvars.exampleinfra/gcp/terraform/environments/prod/variables.tfinfra/gcp/terraform/environments/prod/versions.tfinfra/gcp/terraform/modules/open_wearables_stack/cloud_build.tfinfra/gcp/terraform/modules/open_wearables_stack/cloud_run.tfinfra/gcp/terraform/modules/open_wearables_stack/data_services.tfinfra/gcp/terraform/modules/open_wearables_stack/iam.tfinfra/gcp/terraform/modules/open_wearables_stack/main.tfinfra/gcp/terraform/modules/open_wearables_stack/network.tfinfra/gcp/terraform/modules/open_wearables_stack/outputs.tfinfra/gcp/terraform/modules/open_wearables_stack/runtime_outputs.tfinfra/gcp/terraform/modules/open_wearables_stack/variables.tfinfra/gcp/terraform/modules/open_wearables_stack/versions.tf
✅ Files skipped from review due to trivial changes (15)
- infra/gcp/terraform/environments/dev/providers.tf
- infra/gcp/terraform/environments/prod/providers.tf
- .gitignore
- infra/gcp/terraform/environments/prod/versions.tf
- infra/gcp/terraform/environments/dev/versions.tf
- infra/gcp/terraform/modules/open_wearables_stack/iam.tf
- infra/gcp/terraform/environments/prod/.terraform.lock.hcl
- infra/gcp/terraform/modules/open_wearables_stack/versions.tf
- infra/gcp/terraform/environments/dev/terraform.tfvars.example
- infra/gcp/terraform/environments/dev/main.tf
- infra/gcp/terraform/environments/prod/outputs.tf
- infra/gcp/terraform/modules/open_wearables_stack/runtime_outputs.tf
- infra/gcp/terraform/environments/prod/terraform.tfvars.example
- infra/gcp/terraform/modules/open_wearables_stack/cloud_build.tf
- infra/gcp/terraform/modules/open_wearables_stack/variables.tf
🚧 Files skipped from review as they are similar to previous changes (7)
- infra/gcp/terraform/environments/prod/backend.tf
- infra/gcp/terraform/environments/dev/backend.tf
- infra/gcp/terraform/environments/prod/main.tf
- infra/gcp/terraform/environments/dev/outputs.tf
- infra/gcp/terraform/modules/open_wearables_stack/outputs.tf
- infra/gcp/terraform/environments/dev/variables.tf
- infra/gcp/terraform/environments/prod/variables.tf
Comment on lines
+32
to
+45
| backend_runtime_env = merge( | ||
| local.cloud_sql_connection_name != null ? merge( | ||
| { | ||
| DB_NAME = var.cloud_sql_db_name | ||
| DB_USER = var.cloud_sql_db_user | ||
| DB_SOCKET_PATH = "/cloudsql/${local.cloud_sql_connection_name}" | ||
| }, | ||
| var.cloud_sql_db_password != null ? { DB_PASSWORD = var.cloud_sql_db_password } : {} | ||
| ) : {}, | ||
| local.redis_host != null ? { | ||
| REDIS_HOST = local.redis_host | ||
| REDIS_PORT = tostring(local.redis_port) | ||
| } : {}, | ||
| ) |
There was a problem hiding this comment.
Sensitive database password in plain environment variable.
Line 39 injects DB_PASSWORD as a plain environment variable. Consider using Secret Manager references via value_source.secret_key_ref instead, similar to how backend_api_secret_env is handled on lines 145-156.
🔒 Proposed approach using Secret Manager
Instead of:
var.cloud_sql_db_password != null ? { DB_PASSWORD = var.cloud_sql_db_password } : {}Consider adding DB_PASSWORD to backend_api_secret_env / backend_worker_secret_env defaults or documentation, and remove it from backend_runtime_env.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/modules/open_wearables_stack/cloud_run.tf` around lines
32 - 45, Remove the plain DB_PASSWORD injection from backend_runtime_env and
instead reference the secret via the Cloud Run secret env pattern used
elsewhere: add DB password to the secret env maps (e.g., backend_api_secret_env
and/or backend_worker_secret_env) and stop merging var.cloud_sql_db_password
into backend_runtime_env; use the same value_source.secret_key_ref approach used
for backend_api_secret_env/backend_worker_secret_env so the Cloud Run service
reads DB_PASSWORD from Secret Manager rather than an inline env var (update any
references to var.cloud_sql_db_password and local.cloud_sql_connection_name
accordingly).
Comment on lines
+82
to
+105
| resource "google_redis_instance" "main" { | ||
| count = var.create_memorystore ? 1 : 0 | ||
|
|
||
| project = var.project_id | ||
| region = var.region | ||
| name = local.memorystore_name_resolved | ||
| tier = var.memorystore_tier | ||
| memory_size_gb = var.memorystore_memory_size_gb | ||
| redis_version = var.memorystore_version | ||
| connect_mode = "PRIVATE_SERVICE_ACCESS" | ||
| authorized_network = local.network_id | ||
|
|
||
| lifecycle { | ||
| precondition { | ||
| condition = local.network_id != null | ||
| error_message = "A network ID must be available (created or provided) when create_memorystore is true." | ||
| } | ||
| } | ||
|
|
||
| depends_on = [ | ||
| google_project_service.required, | ||
| google_service_networking_connection.private_vpc_connection | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does GCP Memorystore for Redis support auth_enabled parameter in Terraform?
💡 Result:
Yes, GCP Memorystore for Redis supports the auth_enabled parameter in Terraform. It is an optional boolean parameter in the google_redis_instance resource that indicates whether OSS Redis AUTH is enabled for the instance. Default value is false. When set to true, AUTH is enabled, and the auth_string output will be populated.
Citations:
- 1: https://registry.terraform.io/providers/hashicorp/google/6.24.0/docs/resources/redis_instance.html
- 2: https://github.com/terraform-google-modules/terraform-google-memorystore
Enable AUTH on Memorystore Redis instance.
Add auth_enabled = true to the google_redis_instance resource to enforce authenticated connections and address CKV_GCP_95 (Redis lacks authentication).
Proposed fix
resource "google_redis_instance" "main" {
count = var.create_memorystore ? 1 : 0
project = var.project_id
region = var.region
name = local.memorystore_name_resolved
tier = var.memorystore_tier
memory_size_gb = var.memorystore_memory_size_gb
redis_version = var.memorystore_version
connect_mode = "PRIVATE_SERVICE_ACCESS"
authorized_network = local.network_id
+ auth_enabled = true
lifecycle {
precondition {
condition = local.network_id != null
error_message = "A network ID must be available (created or provided) when create_memorystore is true."
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resource "google_redis_instance" "main" { | |
| count = var.create_memorystore ? 1 : 0 | |
| project = var.project_id | |
| region = var.region | |
| name = local.memorystore_name_resolved | |
| tier = var.memorystore_tier | |
| memory_size_gb = var.memorystore_memory_size_gb | |
| redis_version = var.memorystore_version | |
| connect_mode = "PRIVATE_SERVICE_ACCESS" | |
| authorized_network = local.network_id | |
| lifecycle { | |
| precondition { | |
| condition = local.network_id != null | |
| error_message = "A network ID must be available (created or provided) when create_memorystore is true." | |
| } | |
| } | |
| depends_on = [ | |
| google_project_service.required, | |
| google_service_networking_connection.private_vpc_connection | |
| ] | |
| } | |
| resource "google_redis_instance" "main" { | |
| count = var.create_memorystore ? 1 : 0 | |
| project = var.project_id | |
| region = var.region | |
| name = local.memorystore_name_resolved | |
| tier = var.memorystore_tier | |
| memory_size_gb = var.memorystore_memory_size_gb | |
| redis_version = var.memorystore_version | |
| connect_mode = "PRIVATE_SERVICE_ACCESS" | |
| authorized_network = local.network_id | |
| auth_enabled = true | |
| lifecycle { | |
| precondition { | |
| condition = local.network_id != null | |
| error_message = "A network ID must be available (created or provided) when create_memorystore is true." | |
| } | |
| } | |
| depends_on = [ | |
| google_project_service.required, | |
| google_service_networking_connection.private_vpc_connection | |
| ] | |
| } |
🧰 Tools
🪛 Checkov (3.2.508)
[medium] 82-105: Ensure Memorystore for Redis has AUTH enabled
(CKV_GCP_95)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/modules/open_wearables_stack/data_services.tf` around
lines 82 - 105, The google_redis_instance resource (google_redis_instance.main)
must enable Redis AUTH; add the attribute auth_enabled = true inside the
resource block (alongside name, tier, memory_size_gb, etc.) so the instance is
created with authentication enabled; after adding auth_enabled, ensure any
provisioning or module inputs that expect unauthenticated access are updated to
use authenticated connections/credentials.
- dev/terraform.tfvars.example: fix ENVIRONMENT from "production" to "dev" - dev/main.tf: use variables for create_cloud_sql and create_memorystore instead of hardcoded false (was contradicting tfvars.example) - prod/versions.tf: match module's terraform version constraint (>= 1.13.0) - prod/variables.tf: clarify backend_api_allow_unauthenticated default (API uses its own JWT auth, Cloud Run IAM would block all requests)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
infra/gcp/terraform/environments/prod/variables.tf (1)
228-266: Add descriptions for env/secret env variables for consistency.Lines 228-266 are missing descriptions, unlike the rest of this file. Adding them improves self-documentation for
terraform-docsand operator onboarding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/gcp/terraform/environments/prod/variables.tf` around lines 228 - 266, The variables block for environment and secret maps (backend_api_env, backend_worker_env, backend_init_env, frontend_env, backend_api_secret_env, backend_worker_secret_env, backend_init_secret_env, frontend_secret_env) lacks description attributes; update each variable declaration to include a clear description string describing its purpose and expected contents (e.g., "Environment variables for backend API" / "Secret environment variables for backend API with secret and optional version"), matching the style used elsewhere in the file so terraform-docs and operators get consistent documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infra/gcp/terraform/environments/prod/variables.tf`:
- Around line 124-128: The variable block for cloud_sql_db_password is missing
Terraform sensitivity metadata; update the variable "cloud_sql_db_password"
definition to mark it as sensitive by adding sensitive = true to the variable
block (keep description, type and default as-is) so Terraform will redact it in
plans/outputs; also scan for any outputs or locals referencing
cloud_sql_db_password and ensure they do not expose its value.
---
Nitpick comments:
In `@infra/gcp/terraform/environments/prod/variables.tf`:
- Around line 228-266: The variables block for environment and secret maps
(backend_api_env, backend_worker_env, backend_init_env, frontend_env,
backend_api_secret_env, backend_worker_secret_env, backend_init_secret_env,
frontend_secret_env) lacks description attributes; update each variable
declaration to include a clear description string describing its purpose and
expected contents (e.g., "Environment variables for backend API" / "Secret
environment variables for backend API with secret and optional version"),
matching the style used elsewhere in the file so terraform-docs and operators
get consistent documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92ff83ad-39c7-45b5-a3f8-836df20f5172
📒 Files selected for processing (4)
infra/gcp/terraform/environments/dev/main.tfinfra/gcp/terraform/environments/dev/terraform.tfvars.exampleinfra/gcp/terraform/environments/prod/variables.tfinfra/gcp/terraform/environments/prod/versions.tf
✅ Files skipped from review due to trivial changes (3)
- infra/gcp/terraform/environments/prod/versions.tf
- infra/gcp/terraform/environments/dev/terraform.tfvars.example
- infra/gcp/terraform/environments/dev/main.tf
Comment on lines
+124
to
+128
| variable "cloud_sql_db_password" { | ||
| description = "Database password (optional if managed outside)." | ||
| type = string | ||
| default = null | ||
| } |
There was a problem hiding this comment.
Mark cloud_sql_db_password as sensitive.
Line 124-128 defines a credential input without Terraform sensitivity metadata, which risks accidental exposure in plans/output.
Suggested fix
variable "cloud_sql_db_password" {
description = "Database password (optional if managed outside)."
type = string
default = null
+ sensitive = true
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/gcp/terraform/environments/prod/variables.tf` around lines 124 - 128,
The variable block for cloud_sql_db_password is missing Terraform sensitivity
metadata; update the variable "cloud_sql_db_password" definition to mark it as
sensitive by adding sensitive = true to the variable block (keep description,
type and default as-is) so Terraform will redact it in plans/outputs; also scan
for any outputs or locals referencing cloud_sql_db_password and ensure they do
not expose its value.
Add secret definitions and env vars for all supported providers: Polar, Oura, Fitbit, Garmin, Suunto, Whoop, Strava. Users only need to configure the providers they plan to use.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
infra/gcp/terraform/modules/open_wearables_stack/) that provisions the full Open Wearables stack on Google Cloud Platform with a singleterraform applydev/andprod/) withterraform.tfvars.exampletemplates.gitignoreupdate to exclude Terraform state, secrets, and plan filesWhat the module creates
Design decisions
external_api_service_account_emailfor cross-project service accessDepends on
This module works with the Cloud Run deployment support from #651 (startup scripts, Cloud Tasks dispatcher, internal tasks API).
Test plan
terraform initsucceedsterraform validatepassesterraform planwith example tfvars produces expected resources.gitignorecorrectly excludes state and secret filesPancake Recipe
Your chef: Claude Opus 4.6
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores