chore(new): enhance multi-tenancy support in configmap and secrets#1175
chore(new): enhance multi-tenancy support in configmap and secrets#1175guimoreirar merged 7 commits intomainfrom
Conversation
chore: backmerge
…secrets - Added new configuration options for multi-tenancy in configmap.yaml, including MULTI_TENANT_URL, AWS_REGION, and various Redis settings. - Introduced new secrets for multi-tenancy, such as MULTI_TENANT_REDIS_PASSWORD and MULTI_TENANT_SERVICE_API_KEY in secrets.yaml.
chore(new): enhance multi-tenancy support in configmap and secrets
WalkthroughBumped BR Bank Transfer Helm chart package to 1.1.2-beta.2 and updated README mapping. Renamed multi-tenancy flag from Changes
Comment |
## (2026-04-08)
gandalf-at-lerian
left a comment
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
Review: bank-transfer multi-tenancy config enhancement
Good additions — conditional rendering with {{- if }} for optional fields is the right pattern, and the new secrets (MULTI_TENANT_REDIS_PASSWORD, MULTI_TENANT_SERVICE_API_KEY) are correctly guarded. Two issues:
🔴 values.yaml and values-template.yaml not updated
Both files still have MULTI_TENANT_INFRA_ENABLED: "false" — but the configmap template now references MULTI_TENANT_ENABLED (without _INFRA). The values files need to be updated to match. Anyone doing helm install with defaults will get MULTI_TENANT_ENABLED: "false" from the template default, but the stale MULTI_TENANT_INFRA_ENABLED key in values.yaml is confusing and suggests an incomplete rename.
Also, none of the new configmap keys (MULTI_TENANT_REDIS_PORT, MULTI_TENANT_MAX_TENANT_POOLS, etc.) have corresponding entries in values.yaml. The defaults work via the | default in the template, but it breaks the contract that values.yaml documents all available config.
🟡 Unconditional rendering of MULTI_TENANT_REDIS_* when disabled
MULTI_TENANT_REDIS_HOST is conditional, but MULTI_TENANT_REDIS_PORT, MULTI_TENANT_REDIS_TLS, and all the pool/timeout/circuit-breaker keys render unconditionally. When MULTI_TENANT_ENABLED=false, the configmap gets ~10 unused keys. Not a blocker, but wrapping the whole block in {{- if eq (.Values.bankTransfer.configmap.MULTI_TENANT_ENABLED | default "false") "true" }} would be cleaner.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 32-50: Add missing configmap keys to the bankTransfer.configmap
block in values.yaml so the chart values referenced by the template are
discoverable; specifically declare (even as commented or empty stubs)
MULTI_TENANT_URL, AWS_REGION, MULTI_TENANT_REDIS_HOST, and
MULTI_TENANT_INTEGRATION_SECRET_NAME_TEMPLATE under bankTransfer.configmap (so
the template keys MULTI_TENANT_URL, AWS_REGION, MULTI_TENANT_REDIS_HOST and
MULTI_TENANT_INTEGRATION_SECRET_NAME_TEMPLATE resolve to documented values), and
consider adding placeholders for the other defaults already used
(MULTI_TENANT_REDIS_PORT, MULTI_TENANT_REDIS_TLS, MULTI_TENANT_MAX_TENANT_POOLS,
MULTI_TENANT_IDLE_TIMEOUT_SEC, MULTI_TENANT_TIMEOUT,
MULTI_TENANT_CIRCUIT_BREAKER_THRESHOLD,
MULTI_TENANT_CIRCUIT_BREAKER_TIMEOUT_SEC, MULTI_TENANT_CACHE_TTL_SEC,
MULTI_TENANT_CONNECTIONS_CHECK_INTERVAL_SEC) to keep parity between
charts/plugin-br-bank-transfer/templates/configmap.yaml and
bankTransfer.configmap in values.yaml.
- Line 31: The new config key MULTI_TENANT_ENABLED will break existing overrides
using bankTransfer.configmap.MULTI_TENANT_INFRA_ENABLED; update the template to
fall back to the old key when the new one is unset by using Helm
default/coalesce logic so MULTI_TENANT_ENABLED reads
bankTransfer.configmap.MULTI_TENANT_ENABLED if present, otherwise
bankTransfer.configmap.MULTI_TENANT_INFRA_ENABLED, and default to "false" if
neither is set (reference MULTI_TENANT_ENABLED and MULTI_TENANT_INFRA_ENABLED in
configmap.yaml and values under bankTransfer.configmap).
In `@charts/plugin-br-bank-transfer/templates/secrets.yaml`:
- Around line 19-26: Add the two new secret keys used in the template to the
chart's default values so they are discoverable and overridable: declare
MULTI_TENANT_REDIS_PASSWORD and MULTI_TENANT_SERVICE_API_KEY under the
bankTransfer.secrets section in charts/plugin-br-bank-transfer/values.yaml (same
nesting as other secrets), provide empty string defaults and short comments
describing their purpose/format so Helm users can see and override them.
🪄 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: 21b4a365-4d28-4ec4-8d1d-3df505c29021
📒 Files selected for processing (4)
README.mdcharts/plugin-br-bank-transfer/Chart.yamlcharts/plugin-br-bank-transfer/templates/configmap.yamlcharts/plugin-br-bank-transfer/templates/secrets.yaml
…_ENABLED for consistency - Updated configuration in values-template.yaml and values.yaml to reflect the new naming convention. - Adjusted configmap.yaml to ensure proper handling of the renamed variable.
…nfig fix(bank-transfer): rename MULTI_TENANT_INFRA_ENABLED to MULTI_TENANT_ENABLED for consistency
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
charts/plugin-br-bank-transfer/templates/configmap.yaml (1)
31-31:⚠️ Potential issue | 🟠 MajorPreserve backward compatibility for existing overrides.
Line 31 drops support for legacy
bankTransfer.configmap.MULTI_TENANT_INFRA_ENABLED, which can silently disable multi-tenant mode on upgrade for existing value files.Suggested compatibility-safe patch
- MULTI_TENANT_ENABLED: {{ .Values.bankTransfer.configmap.MULTI_TENANT_ENABLED | default "false" | quote }} + MULTI_TENANT_ENABLED: {{ .Values.bankTransfer.configmap.MULTI_TENANT_ENABLED | default .Values.bankTransfer.configmap.MULTI_TENANT_INFRA_ENABLED | default "false" | quote }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plugin-br-bank-transfer/templates/configmap.yaml` at line 31, Update the configmap template line that sets MULTI_TENANT_ENABLED so it preserves backward compatibility by preferring the new key bankTransfer.configmap.MULTI_TENANT_ENABLED but falling back to the legacy bankTransfer.configmap.MULTI_TENANT_INFRA_ENABLED if the new key is unset, and finally defaulting to "false"; implement this using nested Helm default() calls around the existing MULTI_TENANT_ENABLED assignment so upgrades won’t silently disable multi-tenant mode.
🤖 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`:
- Line 32: The conditional comparing the MULTI_TENANT_ENABLED value uses eq
without normalizing the YAML boolean to a string; update the expression that
checks .Values.bankTransfer.configmap.MULTI_TENANT_ENABLED to call toString
before eq (i.e., use (.Values.bankTransfer.configmap.MULTI_TENANT_ENABLED |
default "false" | toString)) so it matches the toString normalization used
elsewhere and ensures correct string comparison in the template.
---
Duplicate comments:
In `@charts/plugin-br-bank-transfer/templates/configmap.yaml`:
- Line 31: Update the configmap template line that sets MULTI_TENANT_ENABLED so
it preserves backward compatibility by preferring the new key
bankTransfer.configmap.MULTI_TENANT_ENABLED but falling back to the legacy
bankTransfer.configmap.MULTI_TENANT_INFRA_ENABLED if the new key is unset, and
finally defaulting to "false"; implement this using nested Helm default() calls
around the existing MULTI_TENANT_ENABLED assignment so upgrades won’t silently
disable multi-tenant mode.
🪄 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: 9ad7b35e-9c7f-4c61-aea1-f05e392c2051
📒 Files selected for processing (5)
README.mdcharts/plugin-br-bank-transfer/Chart.yamlcharts/plugin-br-bank-transfer/templates/configmap.yamlcharts/plugin-br-bank-transfer/values-template.yamlcharts/plugin-br-bank-transfer/values.yaml
4 participants
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.