fix(new): add new configuration options for deployment mode, private upstreams, and reconciliation settings#1238
Conversation
…ivate upstreams, and reconciliation settings
WalkthroughUpdated Helm chart templates for the bank transfer plugin to introduce new environment variables and optional configurations for deployment control, TLS/proxy settings, idempotency, CRM adapter, fees, polling, and reconciliation features, plus MongoDB TLS certificate support. Changes
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/plugin-br-bank-transfer/templates/configmap.yaml`:
- Around line 10-11: YAMLlint is complaining about "braces: too many spaces
inside braces" for the templated values DEPLOYMENT_MODE and
ALLOW_PRIVATE_UPSTREAMS in configmap.yaml; fix it by removing the extra spaces
immediately inside the Helm template delimiters so the templates read
{{.Values.bankTransfer.configmap.DEPLOYMENT_MODE | default "byoc" | quote}} and
{{.Values.bankTransfer.configmap.ALLOW_PRIVATE_UPSTREAMS | default "false" |
quote}} (i.e., no space after '{{' and before '}}') to satisfy brace-spacing
rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1c86d14-f0e8-408e-bafc-32890123d1e4
📒 Files selected for processing (2)
charts/plugin-br-bank-transfer/templates/configmap.yamlcharts/plugin-br-bank-transfer/templates/secrets.yaml
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
Clean addition — all new entries follow the existing chart patterns (inline defaults with | default, conditional blocks with {{- if .Values... }}).
A few observations:
-
Reconciliation defaults look reasonable — 30s interval, batch of 20, 5 max attempts, 60s stale threshold. Sane starting points for a bank transfer reconciliation loop.
-
MONGO_TLS_CA_CERT in secrets — correct placement. Conditional so it won't create noise for non-TLS setups.
-
DEPLOYMENT_MODE default "byoc" — makes sense as the safe default (bring-your-own-cloud vs managed).
-
No values.yaml changes — the inline defaults in templates are consistent with the existing chart convention, so this is fine. Worth documenting the new keys in the chart's README or values reference at some point though.
Minor: none of the PR checklist items are checked. Not blocking, but worth ticking off.
LGTM ✅
Midaz Pull Request Checklist
Pull Request Type
Checklist
Please check each item after it's completed.
Additional Notes
Obs: Please, always remember to target your PR to develop branch instead of main.