Skip to content

fix(platform): DSPX-2933 align KAS root key env var with Viper config path#191

Merged
jp-ayyappan merged 4 commits into
mainfrom
DSPX-2933-a7k3m9x2p1
May 15, 2026
Merged

fix(platform): DSPX-2933 align KAS root key env var with Viper config path#191
jp-ayyappan merged 4 commits into
mainfrom
DSPX-2933-a7k3m9x2p1

Conversation

@jp-ayyappan
Copy link
Copy Markdown
Contributor

@jp-ayyappan jp-ayyappan commented May 6, 2026

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 path services.kas.root_key to the env var {PREFIX}_SERVICES_KAS_ROOT_KEY — the SERVICES_KAS_ segment was missing.

This PR updates:

  • charts/platform/templates/deployment.yaml — changes _KAS_ROOT_KEY_SERVICES_KAS_ROOT_KEY
  • tests/chart_platform_template_test.go — updates the unit test assertion from OPENTDF_KAS_ROOT_KEYOPENTDF_SERVICES_KAS_ROOT_KEY

This aligns the chart with the documentation updated in virtru-corp/data-security-platform#2826, which documents DSP_SERVICES_KAS_ROOT_KEY as the correct env var.

⚠️ Breaking Change Note

Deployments that rely on the chart's auto-injected root_key_secret env var will see the variable name change from {PREFIX}_KAS_ROOT_KEY to {PREFIX}_SERVICES_KAS_ROOT_KEY. Existing extraEnv overrides that already set DSP_SERVICES_KAS_ROOT_KEY are unaffected. Deployments using the old chart-injected variable should verify their KAS pods pick up the root key correctly after upgrading.

Test plan

  • Verify Test_KeyManagement_Enabled_With_RootKeySecret_Expect_EnvVar_Set passes with the updated assertion
  • Deploy to a dev cluster and confirm KAS starts successfully with key management enabled

Summary by CodeRabbit

  • Chores
    • Renamed the Kas root key environment variable to the services-prefixed form (OPENTDF_SERVICES_KAS_ROOT_KEY) for consistency.
  • Documentation
    • Updated chart README and values comments to document the injected services-prefixed root key and clarify it’s generated as a random 32-byte hex value (openssl rand 32 -hex).

jp-ayyappan and others added 2 commits May 6, 2026 09:33
… 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>
@jp-ayyappan jp-ayyappan requested a review from a team as a code owner May 6, 2026 13:34
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 47852daa-8c9e-4a3f-958c-fa3bb48801e9

📥 Commits

Reviewing files that changed from the base of the PR and between b67ea38 and 8633d5a.

📒 Files selected for processing (2)
  • charts/platform/README.md
  • charts/platform/values.yaml
✅ Files skipped from review due to trivial changes (1)
  • charts/platform/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • charts/platform/values.yaml

📝 Walkthrough

Walkthrough

The PR renames the KAS root key environment variable from _KAS_ROOT_KEY to _SERVICES_KAS_ROOT_KEY in the Helm deployment template, updates the corresponding unit test, and adjusts values.yaml and README documentation to reference the new injected env-var name.

Changes

KAS root key env rename

Layer / File(s) Summary
Deployment template
charts/platform/templates/deployment.yaml
Env var name changed from _KAS_ROOT_KEY to _SERVICES_KAS_ROOT_KEY within the kas config preview key management block.
Unit test assertion
tests/chart_platform_template_test.go
Test Test_KeyManagement_Enabled_With_RootKeySecret_Expect_EnvVar_Set updated to assert OPENTDF_SERVICES_KAS_ROOT_KEY instead of OPENTDF_KAS_ROOT_KEY.
Values comment
charts/platform/values.yaml
Comment/example for services.kas.config.root_key updated to state the generated root key is injected as {PREFIX}_SERVICES_KAS_ROOT_KEY and shows the openssl rand 32 -hex example.
Chart README
charts/platform/README.md
services.kas.root_key_secret description updated to mention the {PREFIX}_SERVICES_KAS_ROOT_KEY env-var injection and the generated-root-key mechanism.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • elizabethhealy

Poem

"A rabbit nibbles at a line,
Renames a key to fit the sign,
Tests wake up and nod in tune,
Docs and charts hum the new rune,
A tiny hop — the change is fine." 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: aligning the KAS root key environment variable name with Viper's configuration path mapping, supported by all four modified files and the PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch DSPX-2933-a7k3m9x2p1

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.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jp-ayyappan
Copy link
Copy Markdown
Contributor Author

Related PR

This chart fix is a companion to virtru-corp/data-security-platform#2826, which updated the admin guide documentation to reference DSP_SERVICES_KAS_ROOT_KEY as the correct environment variable. That doc PR did not update the underlying Helm chart template, which still generated the incorrect {PREFIX}_KAS_ROOT_KEY.

The bug affects both upstream OpenTDF and DSP — the Viper config path services.kas.root_key is identical in both opentdf/platform and virtru-corp/data-security-platform.

…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>
@jp-ayyappan
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between b7bce13 and b67ea38.

📒 Files selected for processing (2)
  • charts/platform/README.md
  • charts/platform/values.yaml

Comment thread charts/platform/values.yaml Outdated
@jp-ayyappan jp-ayyappan reopened this May 15, 2026
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>
@jp-ayyappan jp-ayyappan merged commit d1cb792 into main May 15, 2026
15 checks passed
@jp-ayyappan jp-ayyappan deleted the DSPX-2933-a7k3m9x2p1 branch May 15, 2026 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants