Refactor Terraform configuration to use variables instead of local configurations#1
Merged
jesbinjoseph merged 25 commits intoMay 19, 2026
Conversation
…nfigurations - Updated GKE cluster module to use `var.zone` and `var.node_pools` instead of local configurations. - Modified jumphost resources to reference `var.app`, `var.environment`, and `var.service_account_email`. - Replaced local configurations with variables in locals.tf for common labels and required tags. - Adjusted network module to utilize `var.gke_subnets`, `var.gke_pods_range`, and `var.gke_services_range`. - Changed output values to use variables for database connection strings and names. - Enhanced Makefile to include new targets for pulling and pushing tfvars from GCP Secret Manager. - Updated Cloud DNS module to utilize variables for naming and domain configuration. - Removed deprecated config.tf file and adjusted enable-apis.tf to use variables. - Deleted unused backup and bootstrap scripts, and added new scripts for managing tfvars in Secret Manager. - Expanded variables.tf to include additional configuration options and defaults.
There was a problem hiding this comment.
Pull request overview
This PR refactors the OpenTofu/Terraform configuration to consume environment configuration via var.* from tfvars files (stored in GCP Secret Manager) instead of decoding a single JSON “config” secret into local.cfg.
Changes:
- Replaces
local.cfg[...]usages across modules with explicit variables invariables.tf, and updates resources/modules to referencevar.*. - Introduces tfvars Secret Manager workflows (
scripts/tfvars-pull.sh,scripts/tfvars-push.sh) and updates module Makefiles to pull/push and pass-var-file. - Updates environment documentation/templates to tfvars (
environments/sample.tfvars) and removes the legacy JSON/config tooling (config.tf, JSON samples, bootstrap/edit/view scripts).
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| variables.tf | Expands the shared input variable contract to replace prior local.cfg config decoding. |
| scripts/tfvars-push.sh | Adds helper to push tfvars into Secret Manager and verify upload via hash. |
| scripts/tfvars-pull.sh | Adds helper to pull tfvars from Secret Manager to a local file for module runs. |
| scripts/config-view.sh | Removes legacy JSON config viewing script (no longer used with tfvars model). |
| scripts/config-edit.sh | Removes legacy JSON config editing script (no longer used with tfvars model). |
| scripts/bootstrap.sh | Removes legacy JSON secret bootstrap script (replaced by tfvars push workflow). |
| scripts/backup-k8s-resources.sh | Removes unused k8s backup helper script. |
| README.md | Adds repo-level documentation reflecting the tfvars + Secret Manager workflow. |
| pre-infra/Makefile | Adds pull/push tfvars targets and runs plan/apply/destroy with -var-file. |
| pre-infra/enable-apis.tf | Switches conditional API enablement to variable flags (and drops some prior conditionals). |
| pre-infra/cloud-dns.tf | Updates Cloud DNS naming/config to use var.*. |
| KMS/Makefile | Adds pull/push tfvars targets and runs plan/apply/destroy with -var-file. |
| KMS/kms.tf | Updates KMS keyring naming and owners to use var.*. |
| infra/outputs.tf | Updates DB-related outputs to use var.* naming components. |
| infra/network.tf | Updates network CIDR inputs to use var.* instead of decoded config. |
| infra/Makefile | Adds pull/push tfvars targets and runs plan/apply/destroy with -var-file. |
| infra/locals.tf | Rebuilds derived names/labels/tags from var.* and override variables. |
| infra/jumphost.tf | Updates jumphost naming/zone/SSH keys/service account to use var.*. |
| infra/gke.tf | Updates cluster zone and node pool configuration to use var.*. |
| infra/github-wif.tf | Updates GitHub WIF gating/config to use variables rather than decoded config. |
| infra/gcs.tf | Updates bucket/service-account/KMS IAM wiring to use var.* naming + project number input. |
| infra/cloudarmour.tf | Updates Cloud Armor enablement and naming to use var.*. |
| infra/cloud-sql.tf | Updates Cloud SQL naming/sizing/zone inputs to use var.*. |
| infra/billing-budget.tf | Removes billing budget resources (previously config-gated). |
| environments/sample.tfvars | Adds canonical sanitized tfvars template for environments. |
| environments/sample.json | Removes legacy JSON sample payload. |
| environments/README.md | Updates environment docs to tfvars usage and variable reference. |
| deploy/secrets.tf | Switches DICOM secret gating to var.enable_dicom. |
| deploy/Makefile | Adds pull/push tfvars targets and runs plan/apply/destroy with -var-file. |
| deploy/locals.tf | Rebuilds deploy locals from var.* (names, domains, secrets/config map merges). |
| deploy/legacy-ingress.tf | Switches legacy ingress domain iteration to use var.*. |
| deploy/helm.tf | Switches DICOM Helm release gating to var.enable_dicom. |
| deploy/helm-values.tf | Switches Helm image/domain inputs to var.* instead of decoded config. |
| config.tf | Removes the JSON Secret Manager decode-based config mechanism (local.cfg). |
| AGENTS.md | Adds contributor/operator guidelines for module ordering and tfvars workflow. |
| .github/instructions/terraform.instructions.md | Adds Terraform/OpenTofu contribution conventions aligned to the new tfvars model. |
| .github/instructions/helm.instructions.md | Adds Helm chart conventions and Terraform value-injection guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+6
to
10
| variable "project_number" { | ||
| description = "GCP project number" | ||
| type = string | ||
| default = null | ||
| } |
…rraform instructions
…rocess and disable file-based generation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 45 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
.github/workflows/helm-lint-test.yml:1
- Removing
ct.yamltogether with.github/workflows/helm-lint-test.ymleliminates all automated lint/template coverage for the Helm charts underhelm_charts/. If this is intentional (e.g. CI is being moved elsewhere), please note it in the PR description; otherwise consider keeping the workflow so chart regressions are still caught on PRs.
tellmeY18
approved these changes
May 18, 2026
…xternal certificate support and checksum-based pod restarts
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
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.
var.zoneandvar.node_poolsinstead of local configurations.var.app,var.environment, andvar.service_account_email.var.gke_subnets,var.gke_pods_range, andvar.gke_services_range.