Skip to content

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

Open
jp-ayyappan wants to merge 3 commits intomainfrom
DSPX-2933-a7k3m9x2p1
Open

fix(platform): DSPX-2933 align KAS root key env var with Viper config path#191
jp-ayyappan wants to merge 3 commits intomainfrom
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 use the services-prefixed form (OPENTDF_SERVICES_KAS_ROOT_KEY) for consistency.
  • Documentation
    • Updated chart documentation and values comments to describe the injected services-prefixed root key and note it is generated as a random 32-byte hex value.

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

Review Change Stack

📝 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 amends values.yaml and README documentation to reflect the new injected env-var name.

Changes

KAS root key env rename

Layer / File(s) Summary
Deployment template
charts/platform/templates/deployment.yaml
Environment variable name changed from _KAS_ROOT_KEY to _SERVICES_KAS_ROOT_KEY in the kas config preview key management block.
Test update
tests/chart_platform_template_test.go
Test assertion in Test_KeyManagement_Enabled_With_RootKeySecret_Expect_EnvVar_Set updated to check for OPENTDF_SERVICES_KAS_ROOT_KEY instead of OPENTDF_KAS_ROOT_KEY.
Values doc
charts/platform/values.yaml
Comment updated to document that the KAS root key is injected as {PREFIX}_SERVICES_KAS_ROOT_KEY when key_management is enabled.
README
charts/platform/README.md
services.kas.root_key_secret description updated to mention the {PREFIX}_SERVICES_KAS_ROOT_KEY env-var injection and root-key generation note.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • elizabethhealy

Poem

"A rabbit hops through code so neat,
Renaming keys with tiny feet,
OPENTDF now wears a longer name,
Tests and charts all speak the same,
A little hop, a stable beat." 🐇

🚥 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 to match Viper's config path mapping, which is the core issue fixed across the deployment template and test files.
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

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

alg: rsa:2048
# -- Key needed when key_management feature is enabled (openssl rand 32 -hex)
# -- Key needed when key_management feature is enabled. Injected as `{PREFIX}_SERVICES_KAS_ROOT_KEY` env var (openssl rand 32 -hex)
# openssl rand 32 -hex | kubectl create secret generic kas-root-key --from-file=root_key=/dev/stdin
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Unescaped | in the helm-docs continuation line breaks the generated README table.

helm-docs appends plain # lines to the preceding # -- description, so the shell pipe in openssl rand 32 -hex | kubectl ... lands verbatim inside the Markdown table cell and creates a spurious extra column (confirmed by markdownlint MD056 at README line 217 — the data after the | is silently truncated).

Escape the pipe so it renders as a literal character:

🐛 Proposed fix
-# openssl rand 32 -hex | kubectl create secret generic kas-root-key --from-file=root_key=/dev/stdin
+# openssl rand 32 -hex \| kubectl create secret generic kas-root-key --from-file=root_key=/dev/stdin
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# openssl rand 32 -hex | kubectl create secret generic kas-root-key --from-file=root_key=/dev/stdin
# openssl rand 32 -hex \| kubectl create secret generic kas-root-key --from-file=root_key=/dev/stdin
🤖 Prompt for 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.

In `@charts/platform/values.yaml` at 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.

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.

1 participant