Fix/terraform module variable mismatches#1224
Open
OxDev-max wants to merge 4 commits into
Open
Conversation
- Pass db_host/db_port/db_name/db_user/db_password to the ecs module instead of a database_url variable it never declared. - Rename the redis module call argument from num_cache_nodes to num_cache_clusters to match the variable the module actually declares. - Fix ecs module's secret_arns output, which referenced a nonexistent aws_secretsmanager_secret.database_url resource, to reference the real per-field db secrets instead.
verify_contract_not_frozen only called balance(), which does not reflect frozen status, making it a no-op guard that always returned Ok. It now checks the contract's authorized() state on the token (the mechanism Classic Stellar assets use to implement freeze via SAC), returning ErrorCode::TokenFrozen when the contract has been deauthorized. Adds unit tests covering both the authorized and frozen (deauthorized) cases.
deletion_protection defaulted to false and was never overridden for prod by the root module or production.tfvars, and there was no lifecycle guard, so a terraform destroy or accidental recreation could wipe the production database. Force deletion_protection = true for the prod environment regardless of the input variable, and add a prevent_destroy lifecycle block on the DB instance.
redis_auth_token was declared with strong validation at the root module but never threaded into the redis module or the aws_elasticache_replication_group resource, so Redis was reachable by anything in the ecs_tasks security group with zero authentication despite transit_encryption_enabled = true. Add a redis_auth_token variable to the redis module, set it as the replication group's auth_token, pass it from the root module, and embed it in the redis_url output so the ECS task's existing REDIS_URL secret carries the credential to the API service.
|
@OxDev-max Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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.
Description
verify_contract_not_frozen was a no-op guard (contracts/predict-iq/src/modules/sac.rs)
The function only called balance(), which doesn't reflect frozen status — so it always returned Ok(()) regardless of whether the contract's tokens were actually frozen, despite its name and docs implying a real check. Fixed it to check the contract's authorized() state on the token (the mechanism Classic Stellar assets use to implement freeze via SAC deauthorization), returning ErrorCode::TokenFrozen when the contract has been deauthorized. Added unit tests covering both the authorized and frozen cases.
Terraform root/child module variable mismatches (infrastructure/terraform/main.tf, modules/ecs/main.tf)
Production RDS had no real delete protection (infrastructure/terraform/modules/rds/main.tf)
deletion_protection defaulted to false and was never overridden for prod by the root module or production/terraform.tfvars, and there was no prevent_destroy lifecycle guard — meaning a terraform destroy, an accidental recreation, or console deletion could wipe the production database. Now deletion_protection is forced to true whenever environment == "prod", and a lifecycle { prevent_destroy = true } block was added to the DB instance.
Redis had zero authentication (infrastructure/terraform/modules/redis/main.tf, root main.tf)
redis_auth_token was declared with strong validation at the root module (implying AUTH was intended) but was never passed into the redis module, which didn't even declare a variable to receive it, and the replication group had no auth_token argument despite transit_encryption_enabled = true. Added a redis_auth_token variable to the redis module, set it as the replication group's auth_token, passed it from the root module, and embedded it into the redis_url output so the existing ECS REDIS_URL secret carries the credential to the API service without any new plumbing on the ECS side.
Type of Change
Testing Done
Bundle Size
Checklist
docs/architecture.mdRelated Issues
Closes #1200
Closes #1201
Closes #1202
Closes #1203