Conversation
Answers, per cluster, whether an agent can stage a script in a writable directory and execute it — and whether that directory sits inside the git worktree the harness commits and pushes. The original concern was that skills mount read-only and possibly noexec, so executable content would be unusable. Measured on CRI-O 1.35: the ImageVolume is read-only at the superblock but noexec does not reach the container, because CRI-O sets it on the lowerdir and overlayfs does not inherit MS_NOEXEC. So execution is not the constraint — placement is. Agents do not execute from the mount anyway; they write a script somewhere writable and run it from there. Given a skill that said only "write the script to a file and run it", the model wrote into the git worktree and left /tmp empty. The watcher did not commit it only because its extension allowlist happens to exclude .sh; it does include .md, .json, .yaml and .txt. ADR 0007 records the resulting convention and why it belongs in the harness rather than in each SkillCard. The probe runs standalone against any cluster — no CRDs, controller or LLM required — and also probes a real Sandbox pod when one exists, so the two can be compared. Refs konveyor#70 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Fabian von Feilitzsch <fabian@fabianism.us>
📝 WalkthroughWalkthroughAdds ADR 0007 and documentation for staging scripts from read-only ImageVolume skills into writable executable directories. Adds standalone and sandbox Kubernetes probes that collect mount, runtime, git-worktree, and execution results. ChangesSkill execution staging
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant run-probe.sh
participant Kubernetes
participant ProbePod
participant probe.sh
Operator->>run-probe.sh: start probe
run-probe.sh->>Kubernetes: collect cluster facts and ImageVolume gate
run-probe.sh->>Kubernetes: create probe pod
Kubernetes-->>ProbePod: start pod
run-probe.sh->>ProbePod: execute probe.sh
probe.sh->>probe.sh: test staging and skill mounts
probe.sh-->>run-probe.sh: return RESULT diagnostics
run-probe.sh->>Kubernetes: locate optional Sandbox pod
run-probe.sh->>Kubernetes: execute probe in Sandbox
run-probe.sh-->>Operator: summarize and compare verdicts
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
hack/probe/run-probe.sh (1)
201-255: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd trap-based cleanup so an interrupted run doesn't leak the probe pod.
If the script is interrupted (Ctrl-C) or fails between
start_probe_podand the final cleanup,${PROBE_POD}is left running since deletion only happens at the tail ofmain.♻️ Suggested fix
main() { mkdir -p "${RESULTS_DIR}" RAW_OUT="${RESULTS_DIR}/probe-raw.txt" : > "${RAW_OUT}" + + if [ "${KEEP}" != "1" ]; then + trap 'kube delete pod "${PROBE_POD}" --ignore-not-found --wait=false >/dev/null 2>&1 || true' EXIT + fi🤖 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 `@hack/probe/run-probe.sh` around lines 201 - 255, Add trap-based cleanup for the probe pod created by start_probe_pod, so interruptions or failures before main’s final cleanup still delete ${PROBE_POD} unless PROBE_KEEP=1. Reuse the existing kube delete behavior and ensure the trap is installed before pod creation while preserving the current normal cleanup and keep-pod behavior.docs/adr/0007-skill-script-execution-and-staging.md (1)
51-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd language identifiers to fenced code blocks.
markdownlint (MD040) flags these three fences as missing a language tag.
📝 Suggested fix
-``` +```text 1267 1259 0:180 / /opt/skills/probe-skill rw,relatime - overlay overlay ro,seclabel,lowerdir=...(apply the same `text` tag to the fences at lines 96 and 119.) </details> Also applies to: 96-99, 119-124 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/adr/0007-skill-script-execution-and-staging.mdaround lines 51 - 53,
Update the three fenced code blocks in the ADR, including the blocks around the
mount-output examples, to specify the text language identifier after each
opening fence. Apply the same text tag consistently to the fences near the
reported locations without changing their contents.</details> <!-- cr-comment:v1:c1921ccd982d04f42806bf83 --> _Source: Linters/SAST tools_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@hack/probe/probe-pod.yaml:
- Around line 16-45: Add a pod-level securityContext to the imagevolume-probe
Pod spec that satisfies Kubernetes restricted Pod Security requirements,
including non-root execution, seccomp RuntimeDefault, and dropping all Linux
capabilities. Keep the existing container command, mounts, and volumes
unchanged.In
@README.md:
- Around line 59-65: Update the ImageVolume prerequisites in README.md at lines
59-65 to state that Kubernetes 1.33+ has ImageVolume beta enabled but not by
default, and change the containerd requirement from 2.0+ to 2.1+. Update the
containerd warning in hack/probe/run-probe.sh at lines 103-105 from 2.0+ to
2.1+; no other prerequisite changes are needed.
Nitpick comments:
In@docs/adr/0007-skill-script-execution-and-staging.md:
- Around line 51-53: Update the three fenced code blocks in the ADR, including
the blocks around the mount-output examples, to specify the text language
identifier after each opening fence. Apply the same text tag consistently to the
fences near the reported locations without changing their contents.In
@hack/probe/run-probe.sh:
- Around line 201-255: Add trap-based cleanup for the probe pod created by
start_probe_pod, so interruptions or failures before main’s final cleanup still
delete ${PROBE_POD} unless PROBE_KEEP=1. Reuse the existing kube delete behavior
and ensure the trap is installed before pod creation while preserving the
current normal cleanup and keep-pod behavior.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `537f57b2-2a65-449b-a3b0-721aa1d20939` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7acc1371552ffc33120e2e3a92f10d019e809de3 and afec590b34a3680d3b52e302325ad721a3876853. </details> <details> <summary>📒 Files selected for processing (7)</summary> * `.gitignore` * `README.md` * `changes/unreleased/70-skill-exec-probe.yaml` * `docs/adr/0007-skill-script-execution-and-staging.md` * `hack/probe/probe-pod.yaml` * `hack/probe/probe.sh` * `hack/probe/run-probe.sh` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| apiVersion: v1 | ||
| kind: Pod | ||
| metadata: | ||
| name: imagevolume-probe | ||
| labels: | ||
| app.kubernetes.io/name: imagevolume-probe | ||
| app.kubernetes.io/component: diagnostic | ||
| spec: | ||
| restartPolicy: Never | ||
| # Override the stub entrypoint: we want a bare long-running process, not the | ||
| # stub's banner logic, so the probe measures the environment and nothing else. | ||
| containers: | ||
| - name: agent | ||
| image: quay.io/konveyor/agentic-controller-agent:e2e | ||
| command: ["sleep", "infinity"] | ||
| volumeMounts: | ||
| - name: skill | ||
| mountPath: /opt/skills/probe-skill | ||
| readOnly: true | ||
| - name: workspace | ||
| mountPath: /workspace | ||
| volumes: | ||
| # The mount under test. Any skill image works -- we are measuring how the | ||
| # kubelet/CRI mounts an ImageVolume, not the contents. | ||
| - name: skill | ||
| image: | ||
| reference: quay.io/konveyor/skills:maven-migration | ||
| # Matches the controller's workspace volume (agentrun_controller.go:332). | ||
| - name: workspace | ||
| emptyDir: {} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add restricted-compatible securityContext so the probe can schedule on hardened/PSA-restricted clusters.
No securityContext is set, so this pod will fail admission on any namespace enforcing the Kubernetes "restricted" Pod Security Standard — exactly the kind of hardened cluster ADR 0007 calls out as the one scenario ("BLOCKED_NO_EXEC_SURFACE") that would need re-testing.
🔒 Suggested fix
spec:
restartPolicy: Never
+ securityContext:
+ runAsNonRoot: true
+ seccompProfile:
+ type: RuntimeDefault
containers:
- name: agent
image: quay.io/konveyor/agentic-controller-agent:e2e
command: ["sleep", "infinity"]
+ securityContext:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop: ["ALL"]
volumeMounts:📝 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.
| apiVersion: v1 | |
| kind: Pod | |
| metadata: | |
| name: imagevolume-probe | |
| labels: | |
| app.kubernetes.io/name: imagevolume-probe | |
| app.kubernetes.io/component: diagnostic | |
| spec: | |
| restartPolicy: Never | |
| # Override the stub entrypoint: we want a bare long-running process, not the | |
| # stub's banner logic, so the probe measures the environment and nothing else. | |
| containers: | |
| - name: agent | |
| image: quay.io/konveyor/agentic-controller-agent:e2e | |
| command: ["sleep", "infinity"] | |
| volumeMounts: | |
| - name: skill | |
| mountPath: /opt/skills/probe-skill | |
| readOnly: true | |
| - name: workspace | |
| mountPath: /workspace | |
| volumes: | |
| # The mount under test. Any skill image works -- we are measuring how the | |
| # kubelet/CRI mounts an ImageVolume, not the contents. | |
| - name: skill | |
| image: | |
| reference: quay.io/konveyor/skills:maven-migration | |
| # Matches the controller's workspace volume (agentrun_controller.go:332). | |
| - name: workspace | |
| emptyDir: {} | |
| apiVersion: v1 | |
| kind: Pod | |
| metadata: | |
| name: imagevolume-probe | |
| labels: | |
| app.kubernetes.io/name: imagevolume-probe | |
| app.kubernetes.io/component: diagnostic | |
| spec: | |
| restartPolicy: Never | |
| securityContext: | |
| runAsNonRoot: true | |
| seccompProfile: | |
| type: RuntimeDefault | |
| # Override the stub entrypoint: we want a bare long-running process, not the | |
| # stub's banner logic, so the probe measures the environment and nothing else. | |
| containers: | |
| - name: agent | |
| image: quay.io/konveyor/agentic-controller-agent:e2e | |
| command: ["sleep", "infinity"] | |
| securityContext: | |
| allowPrivilegeEscalation: false | |
| capabilities: | |
| drop: ["ALL"] | |
| volumeMounts: | |
| - name: skill | |
| mountPath: /opt/skills/probe-skill | |
| readOnly: true | |
| - name: workspace | |
| mountPath: /workspace | |
| volumes: | |
| # The mount under test. Any skill image works -- we are measuring how the | |
| # kubelet/CRI mounts an ImageVolume, not the contents. | |
| - name: skill | |
| image: | |
| reference: quay.io/konveyor/skills:maven-migration | |
| # Matches the controller's workspace volume (agentrun_controller.go:332). | |
| - name: workspace | |
| emptyDir: {} |
🧰 Tools
🪛 Checkov (3.3.8)
[medium] 16-45: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 16-45: Minimize the admission of root containers
(CKV_K8S_23)
🤖 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 `@hack/probe/probe-pod.yaml` around lines 16 - 45, Add a pod-level
securityContext to the imagevolume-probe Pod spec that satisfies Kubernetes
restricted Pod Security requirements, including non-root execution, seccomp
RuntimeDefault, and dropping all Linux capabilities. Keep the existing container
command, mounts, and volumes unchanged.
Source: Linters/SAST tools
| - Kubernetes 1.33+ (ImageVolume beta, enabled by default) | ||
| - OpenShift 4.20+ | ||
| - Agent Sandbox v0.5.x | ||
| - A container runtime that implements ImageVolume: CRI-O 1.31+ or containerd | ||
| 2.0+. cri-dockerd does not implement it. | ||
|
|
||
| For a local cluster to develop against, see [docs/local-development.md](docs/local-development.md). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files =="
git ls-files | rg '(^README\.md$|hack/probe/run-probe\.sh$|docs/local-development\.md$)' || true
echo "== README relevant lines =="
if [ -f README.md ]; then
nl -ba README.md | sed -n '50,70p'
fi
echo "== run-probe relevant lines =="
if [ -f hack/probe/run-probe.sh ]; then
nl -ba hack/probe/run-probe.sh | sed -n '90,115p'
fi
echo "== broader ImageVolume mentions =="
rg -n "ImageVolume|containerd 2\.|containerd >= 2\.0|needs >= 2\.0|CRI-O|Kubernetes 1\.33" README.md hack/probe/run-probe.sh docs 2>/dev/null || trueRepository: konveyor/agentic-controller
Length of output: 284
🌐 Web query:
Kubernetes v1.33 beta announcement containerd ImageVolume v2.1.0
💡 Result:
In Kubernetes v1.33, released in April 2025, the ImageVolume feature graduated to beta [1][2]. This feature, introduced via KEP-4639, enables pods to mount OCI images or artifacts directly as volumes [1][3]. Regarding containerd, support for the ImageVolume feature was implemented as part of the containerd v2.1.0 release [4][1]. Specifically: - Alpha Support: Initial support for the alpha version of the ImageVolume feature was included in containerd v2.1.0 [1]. - Beta Graduation & SubPath Support: While Kubernetes v1.33 brought the ImageVolume feature to beta (enabling subPath and subPathExpr support), containerd's support for these advanced beta requirements (such as subPath support) was integrated in later developments following the initial v2.1.0 release [5][1][2]. Because the feature remains disabled by default in Kubernetes due to the requirement for full runtime support, users must explicitly enable the ImageVolume feature gate on both the API server and the kubelet [1][6]. As of the v1.33 release, users were advised to ensure their container runtime environment was updated to a version with appropriate support [1].
Citations:
- 1: https://kubernetes.io/blog/2025/04/29/kubernetes-v1-33-image-volume-beta/
- 2: https://www.devopschat.co/articles/kubernetes-v133-image-volumes-graduate-to-beta
- 3: Mount OCI Artifacts containerd/containerd#11381
- 4: ImageVolume alpha feature not supported by containerd k3s-io/k3s#10877
- 5: [KEP-4639] Support image volume mount subpath containerd/containerd#11578
- 6: https://kubernetes.ltd/docs/tasks/configure-pod-container/image-volumes/
🌐 Web query:
kubernetes 1.33 image volumes beta enabled by default Kubernetes v1.33
💡 Result:
In Kubernetes v1.33, the Image Volumes feature graduated to beta, but it is not enabled by default [1][2][3]. To use Image Volumes in Kubernetes v1.33, you must manually enable the ImageVolume feature gate on your API server and kubelet [4][3]. The feature remains disabled by default primarily because not all container runtimes have full support for it [1][5]. While in beta, the feature introduced support for subPath and subPathExpr in volumeMounts, allowing users to mount specific subdirectories of an image volume [1][3][5]. Additionally, new kubelet metrics were added to track usage and monitor mount success or failure [5].
Citations:
- 1: https://kubernetes.io/blog/2025/04/29/kubernetes-v1-33-image-volume-beta/
- 2: https://www.bookstack.cn/read/kubernetes-1.33-en/c19d23720b57372b.md
- 3: https://blog.abhimanyu-saharan.com/posts/kubernetes-v1-33-image-volumes-graduate-to-beta
- 4: https://kubernetes.ltd/docs/tasks/configure-pod-container/image-volumes/
- 5: https://www.devopschat.co/articles/kubernetes-v133-image-volumes-graduate-to-beta
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== README relevant lines =="
if [ -f README.md ]; then
sed -n '50,70p' README.md
fi
echo "== run-probe relevant lines =="
if [ -f hack/probe/run-probe.sh ]; then
sed -n '90,115p' hack/probe/run-probe.sh
fi
echo "== broader ImageVolume mentions =="
rg -n "ImageVolume|containerd 2\.|containerd >= 2\.0|needs >= 2\.0|CRI-O|Kubernetes 1\.33" README.md hack/probe/run-probe.sh docs 2>/dev/null || trueRepository: konveyor/agentic-controller
Length of output: 5634
Correct the Kubernetes and containerd prerequisites for ImageVolume.
ImageVolume is beta in Kubernetes 1.33 but is not enabled by default, and containerd support is at 2.1, so both README.md#L59-L65 and the containerd warning in hack/probe/run-probe.sh#L104 are now incorrect. Update the README default-enablement wording and bump both “containerd 2.0+” references to “containerd 2.1+”.
📍 Affects 2 files
README.md#L59-L65(this comment)hack/probe/run-probe.sh#L103-L105
🤖 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 `@README.md` around lines 59 - 65, Update the ImageVolume prerequisites in
README.md at lines 59-65 to state that Kubernetes 1.33+ has ImageVolume beta
enabled but not by default, and change the containerd requirement from 2.0+ to
2.1+. Update the containerd warning in hack/probe/run-probe.sh at lines 103-105
from 2.0+ to 2.1+; no other prerequisite changes are needed.
1 participant
Spike work for #70 — what it takes for skills to carry executable content.
What this is
A probe that answers, for a given cluster, whether an agent can stage a script in a writable directory and execute it, and whether that directory sits inside the git worktree the harness commits and pushes. Plus ADR 0007 recording the convention that falls out of it.
Runs standalone — no CRDs, controller, or LLM needed:
It also probes a real Sandbox pod when one exists, so the hand-written and controller-created cases can be compared. Divergence between them would itself be a finding.
What we found
The original worry was that skills mount read-only and possibly
noexec, making executable content unusable. That turns out to be mostly the wrong question.Measured on CRI-O 1.35 / k8s 1.34:
/opt/skills/<name>rw,relatimero,seclabel,.../tmprw,relatimerwCRI-O does pass
ro,noexec,nosuid,nodevtoStore().MountImage(), but it then layers an overlay on top and overlayfs doesn't inheritMS_NOEXEC. Read-only is enforced, at the superblock. So execution isn't the constraint — placement is.And agents don't execute from the mount anyway. They write a script somewhere writable and run it from there. Which raises the question the ADR actually answers: where?
Given a skill that said only "write the script to a file, make it executable, run it", the model wrote to
/workspace/repo/verify.shand left/tmpempty. That's inside the git worktree the harness pushes. The watcher didn't commit it, but only because its extension allowlist happens to exclude.sh— it does include.md,.json,.yamland.txt, which is exactly what an agent reaches for when writing notes and plans.ADR 0007 records the convention (stage to
/tmp) and argues it belongs in the harness rather than in every SkillCard, since it's a property of the execution environment rather than of any skill.Notes
The local dev environment used to run this is being kept out of the PR for now.
Summary by CodeRabbit
New Features
Documentation