fix(bank-transfer): correct encryption key env var names#1168
fix(bank-transfer): correct encryption key env var names#1168ferr3ira-gabriel merged 1 commit intodevelopfrom
Conversation
The source code expects JD_INCOMING_RAW_XML_ENCRYPTION_KEY and RECIPIENT_DETAILS_ENCRYPTION_KEY (hex-encoded), but the helm chart was creating env vars with _BASE64 suffix which the app doesn't read. This caused the pod to crash with 'JD_INCOMING_RAW_XML_ENCRYPTION_KEY is required' error because the env var name didn't match. Changes: - Rename env vars from *_BASE64 to match source code expectations - Update comments to clarify hex-encoding format (64 hex chars) - Update values.yaml, values-template.yaml, and README.md accordingly Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
801347c to
fb79deb
Compare
WalkthroughUpdates the Brazilian bank transfer plugin's Helm chart to change encryption key configuration from base64-encoded to hex-encoded format. Changes include updated secret key names and documentation across README, Kubernetes Secret template, and Helm values files. 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/README.md`:
- Around line 191-193: Add a new "Upgrading" or "Migration Notes" section to the
README that documents the breaking change: list the environment variable renames
(e.g., JD_WEBHOOK_NOTIFICATION_RAW_XML_DECRYPTION_KEY_BASE64 →
JD_WEBHOOK_NOTIFICATION_RAW_XML_DECRYPTION_KEY, RECIPIENT_DETAILS_ENCRYPTION_KEY
and JD_INCOMING_RAW_XML_ENCRYPTION_KEY name changes if applicable), and clearly
state the encoding change from base64 to hex (32-byte AES-256 key must now be
hex-encoded as 64 hex characters). Provide a short migration recipe: how to
convert an existing base64 key to a hex string (one-line guidance) and instruct
users to rename the variables in their values/Secrets; reference the exact
variable names shown in the diff (JD_INCOMING_RAW_XML_ENCRYPTION_KEY,
RECIPIENT_DETAILS_ENCRYPTION_KEY,
JD_WEBHOOK_NOTIFICATION_RAW_XML_DECRYPTION_KEY_BASE64) so users can find and
update 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: 1a4bfdab-53b9-4184-906d-c1ca219ddc72
📒 Files selected for processing (4)
charts/plugin-br-bank-transfer/README.mdcharts/plugin-br-bank-transfer/templates/secrets.yamlcharts/plugin-br-bank-transfer/values-template.yamlcharts/plugin-br-bank-transfer/values.yaml
| | `JD_INCOMING_RAW_XML_ENCRYPTION_KEY` | Encryption key (hex-encoded 32-byte AES-256, 64 hex chars) | | ||
| | `RECIPIENT_DETAILS_ENCRYPTION_KEY` | Encryption key (hex-encoded 32-byte AES-256, 64 hex chars) | | ||
| | `JD_WEBHOOK_NOTIFICATION_RAW_XML_DECRYPTION_KEY_BASE64` | Decryption key (32-byte base64) | |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding upgrade/migration notes for this breaking change.
The documentation correctly reflects the renamed variables and hex-encoding format. However, since the PR summary indicates this is a breaking change requiring users to rename their values and convert from base64 to hex encoding, consider adding a brief "Upgrading" or "Migration Notes" section documenting:
- The renamed values (
*_BASE64→ non-suffixed) - The encoding change (base64 → hex-encoded 64-character string)
This would help users upgrading from previous chart versions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plugin-br-bank-transfer/README.md` around lines 191 - 193, Add a new
"Upgrading" or "Migration Notes" section to the README that documents the
breaking change: list the environment variable renames (e.g.,
JD_WEBHOOK_NOTIFICATION_RAW_XML_DECRYPTION_KEY_BASE64 →
JD_WEBHOOK_NOTIFICATION_RAW_XML_DECRYPTION_KEY, RECIPIENT_DETAILS_ENCRYPTION_KEY
and JD_INCOMING_RAW_XML_ENCRYPTION_KEY name changes if applicable), and clearly
state the encoding change from base64 to hex (32-byte AES-256 key must now be
hex-encoded as 64 hex characters). Provide a short migration recipe: how to
convert an existing base64 key to a hex string (one-line guidance) and instruct
users to rename the variables in their values/Secrets; reference the exact
variable names shown in the diff (JD_INCOMING_RAW_XML_ENCRYPTION_KEY,
RECIPIENT_DETAILS_ENCRYPTION_KEY,
JD_WEBHOOK_NOTIFICATION_RAW_XML_DECRYPTION_KEY_BASE64) so users can find and
update them.
Summary
The source code expects
JD_INCOMING_RAW_XML_ENCRYPTION_KEYandRECIPIENT_DETAILS_ENCRYPTION_KEY(hex-encoded), but the helm chart was creating env vars with_BASE64suffix which the app doesn't read.This caused the pod to crash with:
Root Cause
config.go): Expects env varJD_INCOMING_RAW_XML_ENCRYPTION_KEYwith hex-encoded 32-byte key (64 hex chars)secrets.yaml): Was creatingJD_INCOMING_RAW_XML_ENCRYPTION_KEY_BASE64Changes
*_BASE64to match source code expectationsvalues.yaml,values-template.yaml, andREADME.mdaccordinglyTesting
Deploy plugin-br-bank-transfer with the corrected helm chart and verify the pod starts successfully without encryption key errors.