Repository navigation
REGBACKLOG-336: manage storage and eventing across STRR environments - #1790
Jacky-Pham wants to merge 14 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
hey Jimmy, the SRE IAM changes are merged and applied now. when you have a chance can you review this so I can run the terraform plan? |
There was a problem hiding this comment.
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds dev-only Terraform management for existing STRR Pub/Sub and Eventarc infrastructure.
Changes:
- Defines and imports existing dev resources.
- Configures remote state and keyless authentication.
- Adds manual Terraform plan/apply automation.
File summaries
| File | Description |
|---|---|
terraform/versions.tf |
Configures Terraform, provider, and backend. |
terraform/variables.tf |
Defines dev infrastructure inputs. |
terraform/README.md |
Documents scope and rollout. |
terraform/pubsub.tf |
Manages and imports Pub/Sub resources. |
terraform/eventarc.tf |
Manages and imports the Eventarc trigger. |
terraform/dev.tfvars |
Supplies dev environment values. |
terraform/data.tf |
Resolves the Pub/Sub service agent. |
terraform/.terraform.lock.hcl |
Locks the Google provider. |
.gitignore |
Excludes Terraform working directories. |
.github/workflows/strr-terraform.yaml |
Adds manual plan/apply automation. |
Review details
Files not reviewed (1)
- terraform/.terraform.lock.hcl: Generated file
- Files reviewed: 8/10 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Jacky-Pham
changed the title
|
Hey @jimmypalelil, 1790 is ready for your review. |
| if: >- | ||
| github.ref_protected && | ||
| github.ref == 'refs/heads/main' && | ||
| (github.event_name == 'push' || |
There was a problem hiding this comment.
are these only supposed to be deployed to dev? shouldnt all the envs be included so they are aligned?
There was a problem hiding this comment.
Added the other environment configs too. DEV stays first, then we can enable each target once access and bootstrap are verified. PROD still needs approval.
Jacky-Pham
changed the title
|
jimmypalelil
left a comment
jimmypalelil
left a comment
There was a problem hiding this comment.
[Just curious about this one]
Regarding the terraform directory: would it be cleaner to colocate the Terraform configurations within their respective packages? For instance:
⚬ strr-api/terraform/
⚬ strr-batch-validator/terraform/
⚬ strr-email/terraform/
This would allow each application to manage its own specific resources closer to the source. Not sure if this was considered.
| with: | ||
| stack: api | ||
| action: apply | ||
| target: ${{ inputs.target || 'dev' }} |
There was a problem hiding this comment.
isnt inputs.target a required input? we dont need the fallback here right
| github.ref == 'refs/heads/main' && | ||
| (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && | ||
| contains(fromJSON(vars.STRR_TERRAFORM_ENVIRONMENTS || '["dev"]'), inputs.target || 'dev') && | ||
| inputs.target != 'uat' |
There was a problem hiding this comment.
why is uat skipped here? if we dont wanna deploy to uat then the option itselft should be eliminated right?
| permissions: | ||
| contents: read | ||
| id-token: write | ||
| uses: ./.github/workflows/strr-terraform.yaml |
There was a problem hiding this comment.
dont we need to pass in secrets here as well?
| github.ref == 'refs/heads/main' && | ||
| (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && | ||
| contains(fromJSON(vars.STRR_TERRAFORM_ENVIRONMENTS || '["dev"]'), inputs.target || 'dev') && | ||
| inputs.target != 'sandbox' |
There was a problem hiding this comment.
Why is Sandbox environment skipped?
| @@ -0,0 +1,372 @@ | |||
| """Exercise the actual workflow gates without cloud credentials or deployments.""" | |||
There was a problem hiding this comment.
is this test required? it seems like its just testing gihtub's internal logic for workflow dispatch
Manage STRR storage and eventing through three Terraform roots, with environment-specific resources and state. UAT shares TEST's API/email state; existing unconfigured components stay unconfigured.
DEV applies automatically after an approved merge. Other application targets are enabled after access, bootstrap and runtime verification; PROD keeps its approval gate. Temporary bootstrap imports existing resources before applying bucket logging, versioning and cleanup settings.
Validation: 10 mocked Terraform scenarios and 23 workflow tests passed. Read-only DEV, TEST and PROD email plans showed no resource changes.
REGBACKLOG-336