fix(platform): DSPX-2933 align KAS root key env var with Viper config path#191
Conversation
… path Update the deployment template to use DSP_SERVICES_KAS_ROOT_KEY instead of DSP_KAS_ROOT_KEY. The Viper config path is services.kas.root_key, so the correct environment variable override is DSP_SERVICES_KAS_ROOT_KEY. Also update the corresponding unit test assertion. Co-authored-by: CoopAgent <coopagent@users.noreply.github.com>
…nv var Co-authored-by: CoopAgent <coopagent@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR renames the KAS root key environment variable from ChangesKAS root key env rename
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Code Review
This pull request renames the KAS root key environment variable from _KAS_ROOT_KEY to _SERVICES_KAS_ROOT_KEY in the Helm chart deployment template and updates the corresponding test case to reflect this change. I have no feedback to provide.
Related PRThis chart fix is a companion to virtru-corp/data-security-platform#2826, which updated the admin guide documentation to reference The bug affects both upstream OpenTDF and DSP — the Viper config path |
…ecret
Update the root_key_secret comment to reference the correct injected env
var name {PREFIX}_SERVICES_KAS_ROOT_KEY and regenerate README via helm-docs.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/platform/values.yaml`:
- Line 482: The README table breaks because the comment line containing "openssl
rand 32 -hex | kubectl create secret generic kas-root-key
--from-file=root_key=/dev/stdin" includes an unescaped pipe; edit the comment in
charts/platform/values.yaml and escape the pipe by replacing "|" with "\|" so
helm-docs emits a literal pipe in the generated Markdown (preserving the rest of
the line exactly) to prevent the table column split.
🪄 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: CHILL
Plan: Pro
Run ID: efea1f80-2cd5-4fbb-84a9-ff9146cf323b
📒 Files selected for processing (2)
charts/platform/README.mdcharts/platform/values.yaml
Unescaped | in the continuation line was creating a spurious extra column in the generated README Markdown table. Replace with \| so helm-docs emits a literal pipe character. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Summary
The Helm deployment template currently injects the KAS root key secret as the environment variable
{PREFIX}_KAS_ROOT_KEY(e.g.DSP_KAS_ROOT_KEY). However, Viper maps the YAML config pathservices.kas.root_keyto the env var{PREFIX}_SERVICES_KAS_ROOT_KEY— theSERVICES_KAS_segment was missing.This PR updates:
charts/platform/templates/deployment.yaml— changes_KAS_ROOT_KEY→_SERVICES_KAS_ROOT_KEYtests/chart_platform_template_test.go— updates the unit test assertion fromOPENTDF_KAS_ROOT_KEY→OPENTDF_SERVICES_KAS_ROOT_KEYThis aligns the chart with the documentation updated in virtru-corp/data-security-platform#2826, which documents
DSP_SERVICES_KAS_ROOT_KEYas the correct env var.Deployments that rely on the chart's auto-injected
root_key_secretenv var will see the variable name change from{PREFIX}_KAS_ROOT_KEYto{PREFIX}_SERVICES_KAS_ROOT_KEY. ExistingextraEnvoverrides that already setDSP_SERVICES_KAS_ROOT_KEYare unaffected. Deployments using the old chart-injected variable should verify their KAS pods pick up the root key correctly after upgrading.Test plan
Test_KeyManagement_Enabled_With_RootKeySecret_Expect_EnvVar_Setpasses with the updated assertionSummary by CodeRabbit