[Quickfix][Osdev 2328] - Disable prevent destroy for msk cluster#864
[Quickfix][Osdev 2328] - Disable prevent destroy for msk cluster#864VadimKovalenkoSNF wants to merge 1 commit intoreleases/v.2.18from
Conversation
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
|
VadimKovalenkoSNF
requested review from
protsack-stephan,
roman-stolar and
vlad-shapik
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 40.32%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
📝 WalkthroughWalkthroughThis change reduces API cache time-to-live (TTL) values from 1800 to 120 seconds across all environment configurations and base Terraform variables, updates the CloudFront path pattern for the facilities API endpoint, and toggles the MSK configuration lifecycle protection from preventing destruction to allowing it. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-06-06T13:47:56.598ZApplied to files:
📚 Learning: 2025-08-28T05:09:22.632ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deployment/terraform/kafka.tf (1)
25-31: Consider makingprevent_destroyenvironment-specific to protect production.Disabling
prevent_destroyapplies to all environments including Production, not just Pre-prod. Accidental deletion of the MSK configuration in Production could cause service disruption.Consider using a variable to conditionally set this based on environment:
♻️ Suggested approach
+variable "msk_prevent_destroy" { + description = "Enable lifecycle protection for MSK configuration" + type = bool + default = true +}Then in
kafka.tf, use a dynamic approach or setprevent_destroy = truein production tfvars while keeping itfalsefor Pre-prod.Note: Since
prevent_destroydoesn't support interpolation directly, you may need to use separate resource definitions or accept the current approach as a temporary workaround.If this is a temporary change for Pre-prod teardown, consider adding a TODO comment and reverting after the cleanup is complete.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
deployment/environments/terraform-preprod.tfvarsdeployment/environments/terraform-production.tfvarsdeployment/environments/terraform-rba.tfvarsdeployment/environments/terraform-staging.tfvarsdeployment/terraform/cdn.tfdeployment/terraform/kafka.tfdeployment/terraform/variables.tf
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-06T13:47:56.598Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 649
File: deployment/terraform/waf.tf:15-15
Timestamp: 2025-06-06T13:47:56.598Z
Learning: In deployment/terraform/waf.tf, the user VadimKovalenkoSNF prefers to keep IPv6 detection logic simple using dot-splitting heuristics rather than complex regex patterns, especially since they control the IPv6 and IPv4 addresses used in whitelist and denylist configurations.
Applied to files:
deployment/terraform/cdn.tf
📚 Learning: 2025-08-28T05:09:22.632Z
Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 722
File: deployment/terraform/batch.tf:521-539
Timestamp: 2025-08-28T05:09:22.632Z
Learning: RBA environment uses EventBridge scheduling for database sync, while Preprod environment will use a different approach for data synchronization. Infrastructure changes for automated sync should be applied to RBA only.
Applied to files:
deployment/environments/terraform-rba.tfvars
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-flake8-linter
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-fe-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-django-code-quality
🔇 Additional comments (6)
deployment/terraform/variables.tf (1)
46-68: TTL reduction from 30 minutes to 2 minutes is significant.The default cache TTL reduction from 1800s to 120s will increase origin traffic by ~15x for these endpoints. This change is applied consistently across defaults and all environment tfvars files.
Ensure the backend infrastructure can handle the increased load, and monitor CloudFront cache hit ratios and origin response times after deployment.
deployment/environments/terraform-production.tfvars (1)
12-15: Production cache TTL reduced to 2 minutes.This change will result in more frequent cache invalidation in Production. The values are consistent with other environments and the updated defaults.
deployment/environments/terraform-preprod.tfvars (1)
13-16: LGTM!The TTL values are consistent with Production and other environments.
deployment/terraform/cdn.tf (1)
3-8: Pattern change is safe; all API calls in the codebase use trailing slash format.The change from
api/facilities*toapi/facilities/*is compatible with existing usage. Every API call found in the codebase—including React utilities (/api/facilities/,/api/facilities/${osId}/) and Django tests—consistently uses the trailing slash format (/api/facilities/). The wildcard*in the new pattern matches zero or more characters, so/api/facilities/matchesapi/facilities/*just as it did the old pattern. No action required.deployment/environments/terraform-staging.tfvars (1)
11-14: TTL reduction from 30 minutes to 2 minutes looks intentional.The change aligns with the PR objective to tighten cache behavior. Setting
default_ttlequal tomax_ttlat 120 seconds means CloudFront will cap caching at 2 minutes regardless of originCache-Controlheaders.Be aware this will increase origin request volume by up to 15x for these endpoints. Ensure the backend can handle the increased load, especially during traffic spikes.
deployment/environments/terraform-rba.tfvars (1)
12-15: TTL reduction consistent with other environments.The cache TTL changes match the staging configuration, maintaining environment parity. Since RBA appears to use the production domain (
opensupplyhub.org), monitor origin load and response times after deployment to ensure the 15x increase in potential cache misses doesn't degrade user experience.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 55.73%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.75%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Backmerge PR of #864 Follow-up fix for [OSDEV-2318](https://opensupplyhub.atlassian.net/browse/OSDEV-2318) Disable `prevent_destroy` for msk cluster to unblock `Pre-prod` deletion. [OSDEV-2318]: https://opensupplyhub.atlassian.net/browse/OSDEV-2318?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Django App | Unittest test suite - Code coverage reportTotal: 81.87%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
|
Close, because we don't need those changes in the current release branch. |
VadimKovalenkoSNF
had a problem deploying
to
Development
GitHub Actions
Failure
3 participants
Follow-up fix for OSDEV-2318
Disable
prevent_destroyfor msk cluster to unblockPre-proddeletion.