-
Notifications
You must be signed in to change notification settings - Fork 5
infra PR3: add Terraform env stacks, CI validation, and runbook docs #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
59d4064
d1228ac
f9dd2e6
beaaec2
2faa10c
4d32aae
913c7d1
bd534d8
1d1e645
ee92363
6f376dc
1410cce
6e56b85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,48 @@ | ||||||||||||||||||||||||
| name: Terraform | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||||
| branches: [ main ] | ||||||||||||||||||||||||
| paths: | ||||||||||||||||||||||||
| - "infra/terraform/**" | ||||||||||||||||||||||||
| - ".github/workflows/terraform.yml" | ||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||
| branches: [ main ] | ||||||||||||||||||||||||
| paths: | ||||||||||||||||||||||||
| - "infra/terraform/**" | ||||||||||||||||||||||||
| - ".github/workflows/terraform.yml" | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||
| terraform: | ||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||
| - name: Checkout | ||||||||||||||||||||||||
| uses: actions/checkout@v4 | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| - name: Setup Terraform | ||||||||||||||||||||||||
| uses: hashicorp/setup-terraform@v3 | ||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||
| terraform_version: 1.5.7 | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| - name: Terraform fmt check | ||||||||||||||||||||||||
| run: terraform -chdir=infra/terraform fmt -check -recursive | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| - name: Terraform init (root) | ||||||||||||||||||||||||
| run: terraform -chdir=infra/terraform init -backend=false | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| - name: Terraform validate (root) | ||||||||||||||||||||||||
| run: terraform -chdir=infra/terraform validate | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| - name: Terraform validate modules | ||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||
| for module in network runner_vm secrets observability; do | ||||||||||||||||||||||||
| terraform -chdir=infra/terraform/modules/$module init -backend=false | ||||||||||||||||||||||||
| terraform -chdir=infra/terraform/modules/$module validate | ||||||||||||||||||||||||
| done | ||||||||||||||||||||||||
|
Comment on lines
+36
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CI validates a The Remove
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/terraform.yml
Line: 36-41
Comment:
**CI validates a `secrets` module that doesn't exist**
The `secrets` module is listed in the loop (`for module in network runner_vm secrets observability`), but `infra/terraform/modules/secrets/` does not exist in the repository. This will cause the "Terraform validate modules" step to fail with a directory-not-found error.
Remove `secrets` from this loop until the module is added, or add the module in this PR.
```suggestion
run: |
for module in network runner_vm observability; do
terraform -chdir=infra/terraform/modules/$module init -backend=false
terraform -chdir=infra/terraform/modules/$module validate
done
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| - name: Terraform validate environments | ||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||
| for env in dev prod; do | ||||||||||||||||||||||||
| terraform -chdir=infra/terraform/environments/$env init -backend=false | ||||||||||||||||||||||||
| terraform -chdir=infra/terraform/environments/$env validate | ||||||||||||||||||||||||
| done | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| --- | ||
| title: 'Terraform Deployment Runbook' | ||
| description: 'Bootstrap, plan, apply, and troubleshoot the GCP Terraform environments for Neural.' | ||
| --- | ||
|
|
||
| Use this runbook when operating the reference Terraform stack under `infra/terraform`. | ||
|
|
||
| ## 1. Prerequisites | ||
|
|
||
| - Terraform `1.5.x` | ||
| - GCP project with billing enabled | ||
| - `gcloud` authenticated to the target project | ||
| - IAM permissions for VPC, Compute Engine, Secret Manager, and Monitoring resources | ||
|
|
||
| ## 2. Bootstrap remote state | ||
|
|
||
| Create a dedicated state bucket (one-time): | ||
|
|
||
| ```bash | ||
| gcloud storage buckets create gs://neural-tf-state-prod \ | ||
| --project <PROJECT_ID> \ | ||
| --location us-central1 \ | ||
| --uniform-bucket-level-access | ||
|
|
||
| gcloud storage buckets update gs://neural-tf-state-prod --versioning | ||
| ``` | ||
|
|
||
| Initialize the production environment with backend config: | ||
|
|
||
| ```bash | ||
| cd infra/terraform/environments/prod | ||
| terraform init \ | ||
| -backend-config="bucket=neural-tf-state-prod" \ | ||
| -backend-config="prefix=neural/prod" | ||
| ``` | ||
|
|
||
| Repeat with a different bucket/prefix for `dev`. | ||
|
|
||
| ## 3. Plan and apply | ||
|
|
||
| Create `terraform.tfvars` from `terraform.tfvars.example`, then run: | ||
|
|
||
| ```bash | ||
| terraform fmt -check -recursive | ||
| terraform validate | ||
| terraform plan -out=tfplan | ||
| terraform apply tfplan | ||
| ``` | ||
|
|
||
| Recommended safety controls: | ||
|
|
||
| 1. Keep `plan` output in PR artifacts before apply. | ||
| 2. Require manual approval for `prod` applies. | ||
| 3. Use separate service accounts for `dev` and `prod`. | ||
|
|
||
| ## 4. Secret injection model | ||
|
|
||
| The reference stack provisions Secret Manager containers and grants runner access. | ||
| Add secret **versions** outside Terraform to avoid storing secret values in state: | ||
|
|
||
| ```bash | ||
| echo -n "<KALSHI_API_KEY_ID>" | gcloud secrets versions add kalshi-api-key-id --data-file=- | ||
| echo -n "<KALSHI_PRIVATE_KEY_PEM>" | gcloud secrets versions add kalshi-private-key-pem --data-file=- | ||
| ``` | ||
|
|
||
| In runtime bootstrap scripts, resolve secrets at startup using the runner service account. | ||
|
|
||
| ## 5. Destroy workflow | ||
|
|
||
| ```bash | ||
| terraform plan -destroy -out=tfdestroy | ||
| terraform apply tfdestroy | ||
| ``` | ||
|
|
||
| Before destroy: | ||
|
|
||
| 1. Drain/disable bot workloads. | ||
| 2. Export required logs and runtime artifacts. | ||
| 3. Confirm no shared resources are referenced by other environments. | ||
|
|
||
| ## 6. Troubleshooting | ||
|
|
||
| - **`terraform init` backend errors**: verify bucket name, region, and IAM access. | ||
| - **Provider auth failures**: run `gcloud auth application-default login` or configure workload identity. | ||
| - **Secret access denied**: verify `roles/secretmanager.secretAccessor` on the runner service account. | ||
| - **No alert notifications**: ensure `notification_channels` are valid Monitoring channel resource IDs. | ||
|
|
||
| ## Next | ||
|
|
||
| - Module and contract reference: `basics/infrastructure` | ||
| - Production promotion checklist: `workflows/promotion-checklist` |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| terraform { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required_version = ">= 1.5.0, < 2.0.0" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required_providers { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| google = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source = "hashicorp/google" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| version = "~> 5.0" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Configure this with backend config flags or a backend.hcl file during init. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Example: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # terraform init -backend-config="bucket=neural-tf-state-dev" -backend-config="prefix=neural/dev" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| backend "gcs" {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dev stack missing remote backend block The prod environment declares If this is intentional (e.g., dev is always local-only), it may be worth adding a comment to document the choice. Otherwise, consider adding the same Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: infra/terraform/environments/dev/main.tf
Line: 1-10
Comment:
**Dev stack missing remote backend block**
The prod environment declares `backend "gcs" {}` for remote state, but the dev environment has no backend block and will default to local state. This means the dev state file won't be shared or persisted across team members or CI runs.
If this is intentional (e.g., dev is always local-only), it may be worth adding a comment to document the choice. Otherwise, consider adding the same `backend "gcs" {}` block so that dev state can also be managed remotely with `-backend-config` flags, matching the pattern described in the runbook.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| provider "google" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project = var.project_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| region = var.region | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| zone = var.zone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| module "network" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source = "../../modules/network" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_id = var.project_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| region = var.region | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| network_name = "${var.stack_name}-vpc" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subnet_name = "${var.stack_name}-subnet" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subnet_cidr = var.subnet_cidr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allow_ssh_cidrs = var.allow_ssh_cidrs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| module "runner" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source = "../../modules/runner_vm" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_id = var.project_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| zone = var.zone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| instance_name = "${var.stack_name}-runner" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| machine_type = var.machine_type | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| network_self_link = module.network.network_self_link | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subnetwork_self_link = module.network.subnetwork_self_link | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| startup_script = var.startup_script | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tags = ["neural-runner", "env-dev"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+35
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Service account name collision across environments Both dev and prod runner modules rely on the Consider passing a per-environment
Suggested change
The same change is needed in Prompt To Fix With AIThis is a comment left during a code review.
Path: infra/terraform/environments/dev/main.tf
Line: 35-47
Comment:
**Service account name collision across environments**
Both dev and prod runner modules rely on the `runner_vm` module's default `service_account_id = "neural-runner"` (see `infra/terraform/modules/runner_vm/variables.tf:41`), since neither environment passes an explicit override. If both environments are deployed to the same GCP project, `terraform apply` on the second environment will fail with a "service account already exists" error because GCP service account IDs are project-unique.
Consider passing a per-environment `service_account_id` to disambiguate:
```suggestion
module "runner" {
source = "../../modules/runner_vm"
project_id = var.project_id
zone = var.zone
instance_name = "${var.stack_name}-runner"
machine_type = var.machine_type
network_self_link = module.network.network_self_link
subnetwork_self_link = module.network.subnetwork_self_link
service_account_id = "${var.stack_name}-runner"
startup_script = var.startup_script
tags = ["neural-runner", "env-dev"]
}
```
The same change is needed in `infra/terraform/environments/prod/main.tf:35-47`.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| module "secrets" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source = "../../modules/secrets" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_id = var.project_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| secret_ids = var.secret_ids | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| runner_service_account_email = module.runner.service_account_email | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+49
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: infra/terraform/environments/dev/main.tf
Line: 44-49
Comment:
`../../modules/secrets` doesn't exist - terraform init will fail. The base branch is missing this module despite the commit message mentioning it.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+49
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing Both the dev and prod environment stacks reference This will cause Either add the Prompt To Fix With AIThis is a comment left during a code review.
Path: infra/terraform/environments/dev/main.tf
Line: 44-50
Comment:
**Missing `secrets` module breaks init/validate**
Both the dev and prod environment stacks reference `../../modules/secrets`, but this module does not exist anywhere in the repository — not in the base branch (`codex/infra-pr2-terraform-gcp-modules`), not at HEAD, and not introduced in this PR. The base branch only contains `network`, `runner_vm`, and `observability` modules.
This will cause `terraform init` to fail with a "module not found" error, which means the CI workflow's "Terraform validate environments" step will also fail. The PR's own validation steps (`terraform -chdir=infra/terraform/environments/dev init -backend=false && terraform -chdir=infra/terraform/environments/dev validate`) cannot pass without it.
Either add the `secrets` module in this PR or remove the `module "secrets"` references from the environment stacks until the module is introduced.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| module "observability" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source = "../../modules/observability" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_id = var.project_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| instance_name = module.runner.instance_name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enable_alert_policy = var.enable_alert_policy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| notification_channels = var.notification_channels | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||||||||||||||
| output "runner_instance_name" { | ||||||||||||||||||
| value = module.runner.instance_name | ||||||||||||||||||
| description = "Compute instance name" | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| output "runner_external_ip" { | ||||||||||||||||||
| value = module.runner.instance_external_ip | ||||||||||||||||||
| description = "External IP of runner instance" | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| output "runner_service_account_email" { | ||||||||||||||||||
| value = module.runner.service_account_email | ||||||||||||||||||
| description = "Runner service account email" | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| output "secret_ids" { | ||||||||||||||||||
| value = module.secrets.secret_ids | ||||||||||||||||||
| description = "Provisioned Secret Manager IDs" | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+16
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. references Prompt To Fix With AIThis is a comment left during a code review.
Path: infra/terraform/environments/dev/outputs.tf
Line: 16-19
Comment:
references `module.secrets.secret_ids` but secrets module doesn't exist
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dev outputs missing
Suggested change
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: infra/terraform/environments/dev/outputs.tf
Line: 19
Comment:
dev outputs missing `log_metric_type` that prod has - consider adding for consistency:
```suggestion
description = "Provisioned Secret Manager IDs"
}
output "log_metric_type" {
value = module.observability.log_metric_type
description = "Log metric used for error alerting"
}
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||
|
|
||||||||||||||||||
| output "log_metric_type" { | ||||||||||||||||||
| value = module.observability.log_metric_type | ||||||||||||||||||
| description = "Log metric used for error alerting" | ||||||||||||||||||
| } | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| project_id = "my-gcp-project" | ||
| region = "us-central1" | ||
| zone = "us-central1-a" | ||
|
|
||
| allow_ssh_cidrs = ["35.235.240.0/20"] # IAP TCP tunnel range | ||
|
|
||
| # Optional: enable alerting and wire notification channels | ||
| # enable_alert_policy = true | ||
| # notification_channels = ["projects/my-gcp-project/notificationChannels/1234567890"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secretsmodule doesn't exist ininfra/terraform/modules/- this validation step will failPrompt To Fix With AI