Skip to content

feat(helm)!: Kata Containers as the DinD runtime — drop Sysbox (closes #116)#129

Merged
Yambr merged 6 commits into
mainfrom
feat/kata-runtime
May 20, 2026
Merged

feat(helm)!: Kata Containers as the DinD runtime — drop Sysbox (closes #116)#129
Yambr merged 6 commits into
mainfrom
feat/kata-runtime

Conversation

@Yambr

@Yambr Yambr commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Makes Kata Containers (kata-qemu) the single supported DinD runtime for the computer-use-server Helm chart, and removes Sysbox entirely. Closes #116.

Why: Sysbox no longer works on containerd 2.x (RKE2 / k3s / kubeadm ≥ 1.34). Docker acquired Nestybox; public Sysbox releases are frozen on containerd 1.x with no upstream fix. Rather than carry Sysbox as a second option that is broken on every modern cluster, the chart now targets Kata exclusively. This upstreams the configuration we run in production on a containerd 2.x cluster.

⚠️ Breaking change. The chart defaults changed (see below). Existing Sysbox deployments must migrate to Kata — install kata-deploy and follow docs/kata-runtime.md.

Changes

Chart — defaults flipped to Kata:

Value Old New
orchestrator.runtimeClassName sysbox-runc kata-qemu
dind.privileged null true
dind.storageDriver overlay2 fuse-overlayfs
dind.kataInit.enabled false true
persistence.varLibDocker.persistentVolume.enabled false true
  • dind.kataInit — chart-templated entrypoint wrapper (ConfigMap, not a custom image): installs fuse-overlayfs, creates /dev/fuse, formats+mounts the Block PVC, runs the cgroup-v2 PID-1 evacuation shim.
  • /var/lib/docker is a Block-mode PVC — required under Kata for xattr-dependent workloads (virtio-fs drops security.capability xattrs, CVE-2021-20263).
  • runtimeClassName: "" (stock runc + privileged) is retained as an explicitly unsupported local-testing escape hatch with a loud NOTES.txt warning.
  • All Sysbox references removed from the chart, templates, schema, NOTES, keywords, and CI. Chart version 0.1.00.3.0.
  • CI (helm.yml): the default render is asserted to be the Kata path; the runc fallback is asserted to opt out of every Kata knob.

Docs:

  • docs/kata-runtime.md — install kata-deploy → configure → verify → troubleshoot, plus a tradeoffs table (Sysbox kept only as a historical comparison row).
  • docs/future-architecture/adr/0011-kata-as-first-class-dind-runtime.md — ADR.
  • kubernetes.md / chart README / examples — Kata is the only documented runtime; examples/helm/kata/ removed (it is now the chart default, covered by standalone/).
  • docs/future-architecture/ left as-is (historical context; ADR-0011 records the move).

Testing

  • helm lint — default (Kata), standalone, with-open-webui all pass
  • helm template — default render asserts runtimeClassName: kata-qemu, privileged: true, the init wrapper, volumeMode: Block, fuse-overlayfs; the runc fallback opts out of all of it
  • kubeconform -strict — 21/21 resources valid (default + runc-fallback renders)
  • shellcheck on the rendered dind-entrypoint.sh — clean
  • tests/test-no-cyrillic.sh, tests/test-project-structure.sh pass
  • N/A — no Dockerfile / Compose changes; chart + docs only

Not exercisable in CI: the runtime behavior inside a Kata guest (dockerd start with fuse-overlayfs, mknod/mkfs/cgroup-evacuation, xattr preservation, the iptables-needs-privileged claim) requires a Kata-capable cluster. These are validated in a production deployment; CI here covers chart rendering only.

ADRs reviewed

  • ADR-0004 (pluggable runtime via RuntimeClass) — this PR adds ADR-0011, which supersedes its Phase-9 Kata timeline.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Kata Containers (kata-qemu) is now the default DinD runtime; optional Kata init workflow added
    • Optional block-mode PVC support for the inner Docker daemon
  • Documentation

    • New Kata runtime runbook, troubleshooting, and updated Kubernetes/Helm READMEs, NOTES, and examples reflecting Kata prerequisites
  • Configuration Changes

    • Default storage driver → fuse-overlayfs; added dind.privileged and dind.kataInit controls; chart version bumped to 0.3.0

Review Change Stack

Nikolai Iambroskin and others added 2 commits May 20, 2026 19:06
Sysbox no longer works on containerd 2.x (RKE2/k3s/kubeadm >= 1.34) — its
public releases are frozen on containerd 1.x and there is no upstream fix.
This adds Kata Containers (kata-qemu) as a supported DinD runtime.

Chart changes (all gated — Sysbox installs render unchanged):
- dind.privileged: explicit override of the runtime/privileged coupling.
  null default = legacy auto-derivation. Kata needs true (dockerd needs
  CAP_NET_ADMIN/RAW for iptables NAT; safe — caps confined to the microVM).
- dind.kataInit: chart-templated entrypoint wrapper (ConfigMap, not a custom
  image) — installs fuse-overlayfs, mknod /dev/fuse, formats+mounts the Block
  PVC, runs the cgroup-v2 PID-1 evacuation shim.
- persistence.varLibDocker.persistentVolume: optional Block-mode PVC for
  /var/lib/docker — required under Kata for xattr-dependent workloads
  (virtio-fs drops security.capability xattrs, CVE-2021-20263).
- values.schema.json, NOTES.txt warnings, examples/helm/kata/values.yaml.
- CI: helm.yml gains a Kata render matrix + a backward-compat regression
  guard asserting the Sysbox path picks up no Kata machinery.

Closes #116

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- docs/kata-runtime.md: install kata-deploy, configure the chart, what the
  kataInit wrapper does, verification checklist, troubleshooting table, and a
  Sysbox vs Kata vs runc+privileged tradeoffs comparison.
- ADR-0011: records making Kata first-class now (deviates from ADR-0004's
  Phase-9 timeline, forced by the Sysbox EOL).
- kubernetes.md / chart README / DOCKER.md / README.md: present Sysbox and
  Kata as the two supported runtimes instead of Sysbox-only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b6ba379f-39bb-4134-9d8f-5f30a42a9668

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9eaba and 7a3b30d.

📒 Files selected for processing (2)
  • docs/kata-runtime.md
  • helm/computer-use-server/values.schema.json

📝 Walkthrough

Walkthrough

This PR migrates the Helm chart from Sysbox to Kata Containers (kata-qemu) as the first-class DinD runtime, adding configuration/schema changes, Helm helpers, an init ConfigMap/script, block‑PVC support, deployment wiring, CI/NOTES validation, ADR, runbook, examples, and documentation updates.

Changes

Kata Containers as First-Class DinD Runtime

Layer / File(s) Summary
Configuration Schema and Values Defaults
helm/computer-use-server/Chart.yaml, helm/computer-use-server/values.schema.json, helm/computer-use-server/values.yaml
Chart metadata/version/keywords updated; defaults and schema now target kata-qemu, add dind.privileged, dind.kataInit, default dind.storageDriver: fuse-overlayfs, and persistence.varLibDocker.persistentVolume block PVC options.
Runtime Resolution and Storage Detection Helpers
helm/computer-use-server/templates/_helpers.tpl
dindPrivileged now honors explicit .Values.dind.privileged with fallback to orchestrator.runtimeClassName derivation; added varLibDockerIsPVC.
Kata Guest Initialization ConfigMap
helm/computer-use-server/templates/configmap-dind-init.yaml
Conditional ConfigMap embeds dind-entrypoint.sh to install fuse-overlayfs, ensure /dev/fuse, format/mount block PVC to /var/lib/docker when present, evacuate cgroup‑v2 PID‑1, then exec dockerd.
Persistent Volume Claim for Docker Storage
helm/computer-use-server/templates/pvc-var-lib-docker.yaml
New template conditionally creates a block-mode PVC for /var/lib/docker when enabled and no existing claim is provided.
Deployment Pod Template and Container Wiring
helm/computer-use-server/templates/deployment.yaml
Pod template adds config checksum when kata init enabled, mounts dind-init script, switches DinD command to the init wrapper when enabled, and conditionally exposes var-lib-docker as a block device (volumeDevices) or emptyDir fallback.
Post-Deploy Validation, Messaging, and CI Workflow
helm/computer-use-server/templates/NOTES.txt, .github/workflows/helm.yml, .github/workflows/release.yml
NOTES.txt gains Kata-specific warnings/checklist; Helm lint workflow templates example values to assert Kata default render (runtimeClassName, privileged, dind-entrypoint.sh, Block PVC, fuse-overlayfs) and a separate runc fallback render that omits Kata markers; release workflow text updated.
Helm Chart README: Runtime Selection and Values Reference
helm/computer-use-server/README.md
README rewritten to position Kata-first: prerequisites, runtime matrix (kata-qemu vs empty runc fallback), dind.privileged behavior, dind.kataInit guidance, storage/persistence docs, and Kata troubleshooting.
Kubernetes Deployment: Runtime Setup and Architecture
docs/kubernetes.md
Added Runtime section (kata-deploy, kata-qemu) and Quick start updates; Architecture diagram now uses kata-qemu and Block PVC mapping for /var/lib/docker; prerequisites require block-volume-capable StorageClass.
Kata Runtime Deployment Guide and Troubleshooting
docs/kata-runtime.md
New runbook covering prerequisites (/dev/kvm, PSA), kata-deploy/RuntimeClass verification, optional heavy RuntimeClass, Helm config defaults for Kata DinD, init wrapper behavior, verification commands, troubleshooting table, and tradeoffs.
Architecture Decision Record
docs/future-architecture/adr/0011-kata-as-first-class-dind-runtime.md
ADR-0011 records the decision to support Kata for containerd 2.x and documents the config/init/PVC defaults and alternatives.
Example Values and Deployment Guides
examples/helm/standalone/values.yaml, examples/helm/README.md, examples/helm/with-open-webui/README.md
Examples updated to include Kata prerequisites and Block PVC guidance; standalone values file adds pre-install Kata runbook comments.
Security Documentation and Top-Level Clarifications
README.md, docs/DOCKER.md
Top-level security notes clarified: Compose uses stock runc (shares host kernel); point to Kata and gVisor for stronger isolation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 A quantum hop from Sysbox to the Kata realm,
Where microVMs shelter DinD at the helm—
fuse-overlayfs dances, /dev/fuse awakens,
Block PVCs mount whilst the pod never forsakes 'em.
containerd 2.x, at last, breaks free! 🚀

🚥 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 accurately summarizes the main change: replacing Sysbox with Kata Containers as the DinD runtime in the Helm chart, with appropriate breaking-change indicator.
Linked Issues check ✅ Passed The PR fully addresses all coding objectives from issue #116: Kata runtime support via runtimeClassName switch, fuse-overlayfs storage driver configuration, dind.kataInit wrapper, Block PVC persistence, schema validation guards, and complete documentation (runtime guide, ADR, updated READMEs).
Out of Scope Changes check ✅ Passed All changes are in scope: Helm chart refactoring to Kata defaults, documentation updates, CI validation, and complete removal of Sysbox references. No unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 feat/kata-runtime

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/helm.yml (1)

32-35: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Set explicit least-privilege permissions for the lint job.

lint currently inherits default token permissions. Please pin this job to read-only permissions to avoid unnecessary write scope.

Suggested patch
 jobs:
   lint:
     name: helm lint + template
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:
       - uses: actions/checkout@v6
🤖 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 @.github/workflows/helm.yml around lines 32 - 35, The lint job currently
inherits default token permissions; to enforce least-privilege add an explicit
permissions block under the lint job in the Helm workflow (job name "lint") that
restricts the GITHUB_TOKEN to read-only, e.g. add permissions: contents: read
(or the minimal read scopes you require) directly beneath the "lint:" stanza so
the job cannot perform write actions.
🧹 Nitpick comments (3)
examples/helm/kata/values.yaml (1)

21-23: ⚡ Quick win

Consider emphasizing placeholder replacement.

The mcpApiKey placeholder "change-me-32-hex-chars" is clearly marked, but users might overlook it. While line 19 mentions replacing placeholders, consider adding a more prominent comment near the value itself, such as:

  # ⚠️ REPLACE THIS - Generate a real key: openssl rand -hex 32
  mcpApiKey: "change-me-32-hex-chars"

This reduces the risk of deploying with an obvious/weak key.

🤖 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 `@examples/helm/kata/values.yaml` around lines 21 - 23, Add a prominent inline
warning comment directly above the secrets.mcpApiKey value to make replacement
explicit (e.g., "⚠️ REPLACE THIS - Generate a real key: openssl rand -hex 32")
so users won't miss the placeholder; update the block around the secrets.create
and mcpApiKey entries to include that comment and consider leaving the example
value unchanged but clearly marked as a placeholder using the mcpApiKey symbol
for easy location.
helm/computer-use-server/README.md (1)

167-175: ⚡ Quick win

Consider clarifying the auto-derivation logic.

The explanation "empty ⇒ true, set ⇒ false" describes the legacy Sysbox behavior, but the flow could be clearer for users configuring Kata. Consider restructuring to explain the logic more sequentially: "When dind.privileged: null, the chart sets privileged: false for any non-empty runtimeClassName (Sysbox path) and privileged: true when runtimeClassName is empty (stock runc fallback). For Kata, you must override with dind.privileged: true explicitly."

🤖 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 `@helm/computer-use-server/README.md` around lines 167 - 175, Rewrite the
paragraph describing how dind.privileged is derived from runtimeClassName to be
sequential and unambiguous: state that when dind.privileged is null the chart
auto-derives privileged as false if runtimeClassName is non-empty (Sysbox path)
and true if runtimeClassName is empty (stock-runc fallback), call out that this
is legacy Sysbox behavior, and explicitly instruct users that Kata requires
setting dind.privileged: true; also mention that the chart prints a warning in
NOTES.txt when runtimeClassName is empty and keep the cautionary sentence about
not shipping production clusters with the stock-runc+privileged escape hatch.
helm/computer-use-server/templates/pvc-var-lib-docker.yaml (1)

8-9: ⚡ Quick win

Use the shared PVC gate here too.

deployment.yaml and configmap-dind-init.yaml both rely on computer-use-server.varLibDockerIsPVC, but this template re-derives the condition from raw values. If that helper carries any extra Kata-only guard, this file will still provision a Block PVC that the pod never attaches.

♻️ Proposed fix
 {{- $pv := .Values.persistence.varLibDocker.persistentVolume | default dict }}
-{{- if and $pv.enabled (not $pv.existingClaim) }}
+{{- if and (eq (include "computer-use-server.varLibDockerIsPVC" .) "true") (not $pv.existingClaim) }}
🤖 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 `@helm/computer-use-server/templates/pvc-var-lib-docker.yaml` around lines 8 -
9, This template re-derives the PVC condition from
.Values.persistence.varLibDocker.persistentVolume ($pv.enabled /
$pv.existingClaim) instead of using the shared helper, causing a Block PVC to be
created when other templates expect computer-use-server.varLibDockerIsPVC;
change the conditional to call and rely on the existing helper
(computer-use-server.varLibDockerIsPVC) wherever the template currently checks
$pv.enabled and not $pv.existingClaim so the PVC creation logic is consistent
with deployment.yaml and configmap-dind-init.yaml.
🤖 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 `@docs/kata-runtime.md`:
- Line 39: Update the broken markdown link fragment: replace the incorrect
fragment "`#configure-the-helm-chart`" with the actual generated heading ID
"`#step-2--configure-the-helm-chart`" so the inline link correctly points to the
"Step 2 — Configure the Helm chart" heading in docs/kata-runtime.md.

In `@helm/computer-use-server/values.schema.json`:
- Around line 52-63: Add JSON Schema conditionals to enforce runtime-specific
combos: add an if/then that detects Kata runtime (e.g., runtimeClassName equals
the Kata identifier) and in the then require dind.privileged to be true and
ensure dind.kataInit.enabled may only be present/true under that same Kata
runtime; likewise add an if/then for sysbox-runc (runtimeClassName ==
"sysbox-runc") that rejects
persistence.varLibDocker.persistentVolume.enabled=true and rejects a non-empty
persistence.varLibDocker.existingClaim. Update the top-level schema near the
storageDriver/privileged/kataInit blocks to include these conditional rules so
invalid combinations (dind.privileged=false with Kata, kataInit used without
Kata, or persistentVolume/existingClaim with sysbox) fail schema validation.

---

Outside diff comments:
In @.github/workflows/helm.yml:
- Around line 32-35: The lint job currently inherits default token permissions;
to enforce least-privilege add an explicit permissions block under the lint job
in the Helm workflow (job name "lint") that restricts the GITHUB_TOKEN to
read-only, e.g. add permissions: contents: read (or the minimal read scopes you
require) directly beneath the "lint:" stanza so the job cannot perform write
actions.

---

Nitpick comments:
In `@examples/helm/kata/values.yaml`:
- Around line 21-23: Add a prominent inline warning comment directly above the
secrets.mcpApiKey value to make replacement explicit (e.g., "⚠️ REPLACE THIS -
Generate a real key: openssl rand -hex 32") so users won't miss the placeholder;
update the block around the secrets.create and mcpApiKey entries to include that
comment and consider leaving the example value unchanged but clearly marked as a
placeholder using the mcpApiKey symbol for easy location.

In `@helm/computer-use-server/README.md`:
- Around line 167-175: Rewrite the paragraph describing how dind.privileged is
derived from runtimeClassName to be sequential and unambiguous: state that when
dind.privileged is null the chart auto-derives privileged as false if
runtimeClassName is non-empty (Sysbox path) and true if runtimeClassName is
empty (stock-runc fallback), call out that this is legacy Sysbox behavior, and
explicitly instruct users that Kata requires setting dind.privileged: true; also
mention that the chart prints a warning in NOTES.txt when runtimeClassName is
empty and keep the cautionary sentence about not shipping production clusters
with the stock-runc+privileged escape hatch.

In `@helm/computer-use-server/templates/pvc-var-lib-docker.yaml`:
- Around line 8-9: This template re-derives the PVC condition from
.Values.persistence.varLibDocker.persistentVolume ($pv.enabled /
$pv.existingClaim) instead of using the shared helper, causing a Block PVC to be
created when other templates expect computer-use-server.varLibDockerIsPVC;
change the conditional to call and rely on the existing helper
(computer-use-server.varLibDockerIsPVC) wherever the template currently checks
$pv.enabled and not $pv.existingClaim so the PVC creation logic is consistent
with deployment.yaml and configmap-dind-init.yaml.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 55872eae-55ee-4231-b11e-4dfa8a5999d9

📥 Commits

Reviewing files that changed from the base of the PR and between 7b82d50 and ae4a191.

📒 Files selected for processing (17)
  • .github/workflows/helm.yml
  • README.md
  • docs/DOCKER.md
  • docs/future-architecture/adr/0011-kata-as-first-class-dind-runtime.md
  • docs/kata-runtime.md
  • docs/kubernetes.md
  • examples/helm/README.md
  • examples/helm/kata/values.yaml
  • helm/computer-use-server/Chart.yaml
  • helm/computer-use-server/README.md
  • helm/computer-use-server/templates/NOTES.txt
  • helm/computer-use-server/templates/_helpers.tpl
  • helm/computer-use-server/templates/configmap-dind-init.yaml
  • helm/computer-use-server/templates/deployment.yaml
  • helm/computer-use-server/templates/pvc-var-lib-docker.yaml
  • helm/computer-use-server/values.schema.json
  • helm/computer-use-server/values.yaml

Comment thread docs/kata-runtime.md Outdated
Comment thread helm/computer-use-server/values.schema.json
Nikolai Iambroskin and others added 2 commits May 20, 2026 19:42
Sysbox does not work on containerd 2.x and has no upstream fix. Rather than
carry it as a second, broken-on-modern-clusters option, the chart now targets
Kata Containers exclusively.

BREAKING CHANGE: the chart defaults changed. Existing Sysbox installs must
migrate to Kata (install kata-deploy; see docs/kata-runtime.md). Flipped
defaults:
- orchestrator.runtimeClassName: sysbox-runc -> kata-qemu
- dind.privileged: null -> true
- dind.storageDriver: overlay2 -> fuse-overlayfs
- dind.kataInit.enabled: false -> true
- persistence.varLibDocker.persistentVolume.enabled: false -> true

`runtimeClassName: ""` (stock runc + privileged) is retained as an explicitly
unsupported local-testing escape hatch, with a loud NOTES.txt warning.

All sysbox references removed from the chart, templates, schema, NOTES, and CI.
The helm.yml workflow now asserts the default render is the Kata path and
checks the runc fallback opts out of every Kata knob. Chart version 0.2.0 ->
0.3.0.

Closes #116

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrites kubernetes.md, the chart README, kata-runtime.md, and the helm
examples to present Kata Containers as the single supported DinD runtime
instead of one of two options.

- kubernetes.md: remove the containerd-1.x/2.x runtime-choice table; Kata is
  the only path.
- chart README: collapse "Runtime selection" to Kata + the unsupported runc
  fallback.
- kata-runtime.md: Sysbox kept only as a historical comparison row in the
  tradeoffs table, not as a chart option.
- examples/helm/kata/ removed — its config is now the chart default, so
  examples/helm/standalone/ covers it.

future-architecture/ docs are intentionally left as-is (historical context;
ADR-0011 already records the move to Kata).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Yambr Yambr changed the title feat(helm): Kata Containers as a first-class DinD runtime (closes #116) feat(helm)!: Kata Containers as the DinD runtime — drop Sysbox (closes #116) May 20, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
helm/computer-use-server/templates/deployment.yaml (1)

208-223: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add schema validation to prevent Block PVC without kataInit.

The block-mode PVC logic depends on kataInit.enabled being true: when enabled, the init wrapper formats the block device and mounts it at /var/lib/docker; when disabled, the block device is exposed but unmounted, causing dockerd to fail.

While kataInit.enabled defaults to true in values.yaml (mitigating the issue in typical deployments), the schema allows independently enabling persistence.varLibDocker.persistentVolume without enforcing kataInit.enabled: true. Add a schema validation to require kataInit.enabled: true when the Block PVC is enabled, or update the varLibDockerIsPVC helper to guard against this misconfiguration.

🤖 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 `@helm/computer-use-server/templates/deployment.yaml` around lines 208 - 223,
The deployment's block PVC path can be enabled via
persistence.varLibDocker.persistentVolume while kataInit.enabled may be false,
causing dockerd to fail; update validation or the helper so this
misconfiguration is prevented: either add a chart schema rule that requires
kataInit.enabled: true whenever persistence.varLibDocker.persistentVolume (or
the varLibDocker block-mode condition used by the varLibDockerIsPVC helper) is
true, or modify the varLibDockerIsPVC template helper to also check
.Values.kataInit.enabled and return "false" unless both the PVC is block-mode
and kataInit.enabled is true, ensuring the block device is not exposed without
the init wrapper.
🧹 Nitpick comments (1)
helm/computer-use-server/templates/_helpers.tpl (1)

89-100: 💤 Low value

Auto-derive fallback for null returns false for Kata, contradicting documented requirements.

When dind.privileged is null and runtimeClassName is set (e.g., kata-qemu), this helper returns false. However, the values.yaml documentation states Kata requires privileged=true for iptables NAT.

This is currently safe because values.yaml defaults privileged to true (so the null branch is never hit with Kata defaults), but the auto-derive logic is misleading and could cause failures if someone explicitly sets privileged: null with Kata.

Consider inverting the fallback or documenting that null is only intended for the runc fallback path.

Option: clarify the comment to match behavior
 {{/*
 Whether the inner dind container must run privileged.
 Resolution order:
   1. dind.privileged explicitly set (true/false) => use it verbatim.
      The Kata default is `true`: dockerd requires CAP_NET_ADMIN/RAW for
      iptables NAT, and the caps stay confined to the microVM.
   2. dind.privileged is null => auto-derive from runtimeClassName:
-       runtimeClassName empty => true  (runc fallback — dockerd needs it)
-       runtimeClassName set   => false
+       runtimeClassName empty => true  (runc fallback — dockerd needs privileged)
+       runtimeClassName set   => false (legacy Sysbox behavior; Kata users must set explicit true)
 */}}
🤖 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 `@helm/computer-use-server/templates/_helpers.tpl` around lines 89 - 100, The
helper computer-use-server.dindPrivileged currently auto-derives
.Values.dind.privileged as false when .Values.dind.privileged is null and
.Values.orchestrator.runtimeClassName is set, which breaks Kata (it requires
privileged=true); update the helper so the null/default branch returns true
(i.e., when .Values.dind.privileged is not set, default privileged to true
regardless of runtimeClassName) by changing the conditional branches in the
computer-use-server.dindPrivileged template to return true instead of false for
the currently falling-through case that uses
.Values.orchestrator.runtimeClassName.
🤖 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.

Outside diff comments:
In `@helm/computer-use-server/templates/deployment.yaml`:
- Around line 208-223: The deployment's block PVC path can be enabled via
persistence.varLibDocker.persistentVolume while kataInit.enabled may be false,
causing dockerd to fail; update validation or the helper so this
misconfiguration is prevented: either add a chart schema rule that requires
kataInit.enabled: true whenever persistence.varLibDocker.persistentVolume (or
the varLibDocker block-mode condition used by the varLibDockerIsPVC helper) is
true, or modify the varLibDockerIsPVC template helper to also check
.Values.kataInit.enabled and return "false" unless both the PVC is block-mode
and kataInit.enabled is true, ensuring the block device is not exposed without
the init wrapper.

---

Nitpick comments:
In `@helm/computer-use-server/templates/_helpers.tpl`:
- Around line 89-100: The helper computer-use-server.dindPrivileged currently
auto-derives .Values.dind.privileged as false when .Values.dind.privileged is
null and .Values.orchestrator.runtimeClassName is set, which breaks Kata (it
requires privileged=true); update the helper so the null/default branch returns
true (i.e., when .Values.dind.privileged is not set, default privileged to true
regardless of runtimeClassName) by changing the conditional branches in the
computer-use-server.dindPrivileged template to return true instead of false for
the currently falling-through case that uses
.Values.orchestrator.runtimeClassName.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3dd79e2c-f54e-4b47-a2b3-d5fc793a34cc

📥 Commits

Reviewing files that changed from the base of the PR and between ae4a191 and e342807.

📒 Files selected for processing (15)
  • .github/workflows/helm.yml
  • .github/workflows/release.yml
  • docs/kata-runtime.md
  • docs/kubernetes.md
  • examples/helm/README.md
  • examples/helm/standalone/values.yaml
  • examples/helm/with-open-webui/README.md
  • helm/computer-use-server/Chart.yaml
  • helm/computer-use-server/README.md
  • helm/computer-use-server/templates/NOTES.txt
  • helm/computer-use-server/templates/_helpers.tpl
  • helm/computer-use-server/templates/deployment.yaml
  • helm/computer-use-server/templates/pvc-var-lib-docker.yaml
  • helm/computer-use-server/values.schema.json
  • helm/computer-use-server/values.yaml
✅ Files skipped from review due to trivial changes (3)
  • .github/workflows/release.yml
  • examples/helm/standalone/values.yaml
  • docs/kata-runtime.md

…re not unsupported

- Remove the last Sysbox references from the chart README and kata-runtime.md
  (the "unlike Sysbox" aside and the Sysbox column in the tradeoffs table).
- The runc + privileged fallback is functional — it is not "unsupported", it
  is insecure (the inner daemon shares the host kernel). Reword values.yaml,
  README, NOTES.txt, schema, and kata-runtime.md accordingly so the wording
  describes the security tradeoff, not a non-working path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
helm/computer-use-server/values.yaml (1)

133-147: 💤 Low value

Clarify the null auto-derivation comment — the described fallback contradicts Kata requirements.

The comment on lines 145-146 states that null auto-derives to false when runtimeClassName is set (i.e., Kata). However, lines 140-143 explicitly state that privileged: true is required for Kata. If someone sets privileged: null expecting auto-derivation to "do the right thing" for Kata, they'll get false and hit the iptables permission error.

Consider rewording to make it clear that null auto-derivation is a legacy escape hatch that only works correctly for the runc fallback, not for Kata:

Suggested clarification
 # null  => auto-derive from orchestrator.runtimeClassName (empty => true,
-#   set => false). Only useful for the runc fallback.
+#   set => false). WARNING: this derivation does NOT work for Kata — Kata
+#   needs true. Only use null with the runc fallback (empty runtimeClassName).
🤖 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 `@helm/computer-use-server/values.yaml` around lines 133 - 147, Update the
comment for the privileged setting to clarify that null auto-derivation (based
on orchestrator.runtimeClassName) is a legacy escape hatch intended only for the
runc fallback and will not produce a safe value for Kata: explicitly state that
for Kata users privileged must be true (required for dockerd to have
CAP_NET_ADMIN/CAP_NET_RAW) and that setting privileged: null with a Kata
runtimeClassName will result in false and cause iptables permission failures;
reference the storageDriver and privileged keys and mention Kata/runc fallback
so readers know when to avoid relying on null auto-derivation.
🤖 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.

Nitpick comments:
In `@helm/computer-use-server/values.yaml`:
- Around line 133-147: Update the comment for the privileged setting to clarify
that null auto-derivation (based on orchestrator.runtimeClassName) is a legacy
escape hatch intended only for the runc fallback and will not produce a safe
value for Kata: explicitly state that for Kata users privileged must be true
(required for dockerd to have CAP_NET_ADMIN/CAP_NET_RAW) and that setting
privileged: null with a Kata runtimeClassName will result in false and cause
iptables permission failures; reference the storageDriver and privileged keys
and mention Kata/runc fallback so readers know when to avoid relying on null
auto-derivation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c22e46d-baad-4b1f-9629-2dd4b686777b

📥 Commits

Reviewing files that changed from the base of the PR and between e342807 and 0f9eaba.

📒 Files selected for processing (5)
  • docs/kata-runtime.md
  • helm/computer-use-server/README.md
  • helm/computer-use-server/templates/NOTES.txt
  • helm/computer-use-server/values.schema.json
  • helm/computer-use-server/values.yaml
✅ Files skipped from review due to trivial changes (2)
  • helm/computer-use-server/README.md
  • docs/kata-runtime.md

- values.schema.json: add an if/then guard so a Kata runtimeClassName
  (^kata) requires dind.privileged != false and dind.kataInit.enabled
  != false. Invalid Kata combos now fail schema validation instead of
  only failing later at render/runtime. The runc fallback (empty
  runtimeClassName) is unaffected. CodeRabbit's companion sysbox-runc
  guard is not applicable — Sysbox was removed in this PR.
- docs/kata-runtime.md: fix the broken link fragment
  (#configure-the-helm-chart -> #step-2--configure-the-helm-chart),
  flagged by markdownlint MD051.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Yambr

Yambr commented May 20, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed both CodeRabbit findings in 7a3b30d:

  1. Broken link fragment (docs/kata-runtime.md) — fixed #configure-the-helm-chart#step-2--configure-the-helm-chart.

  2. Schema does not enforce runtime combinations (values.schema.json) — added a top-level allOf if/then: when orchestrator.runtimeClassName matches ^kata, the schema now requires dind.privileged != false and dind.kataInit.enabled != false. Invalid Kata combinations fail helm lint/template instead of only failing later at install/runtime. Verified: the default and kata-qemu-heavy pass; --set dind.privileged=false and --set dind.kataInit.enabled=false are correctly rejected; the empty-runtimeClass runc fallback is unaffected.

    The companion sysbox-runc guard CodeRabbit suggested is not applicable — this PR removes Sysbox entirely, so sysbox-runc is no longer a valid runtimeClassName.

@Yambr Yambr merged commit 989ad59 into main May 20, 2026
15 checks passed
@Yambr Yambr deleted the feat/kata-runtime branch May 20, 2026 17:40
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.

Support Kata Containers runtime (sysbox EOL on containerd 2.x)

1 participant