feat(cardano-node): support custom mithril image overrides - #415
Conversation
219f588 to
45023fe
Compare
📝 WalkthroughWalkthroughThe chart version and Cardano Node application version were updated. The default node image tag and dedicated Mithril client image configuration were added. The Mithril snapshot init container now uses the dedicated image settings, runs as root, updates 🚥 Pre-merge checks | ✅ 4✅ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
a23f179 to
55314b4
Compare
![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
🤖 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/cardano-node/templates/statefulset.yaml`:
- Around line 37-39: Replace the init container’s root-based securityContext
with a non-root configuration, and add a pod-level securityContext under
spec.template.spec using fsGroup set to the mithril-client image’s non-root
group ID. Remove runAsUser: 0 and runAsGroup: 0 while preserving
persistent-volume write access.
- Line 148: Update the SNAPSHOT_DIGEST lookup in the snapshot download script to
handle an empty result or failed mithril-client fetch explicitly before
continuing. Preserve pipe failure detection with set -o pipefail, and emit a
clear error or exit when no digest is obtained so the later download is not
attempted with an empty value.
🪄 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: 6d622284-e9fc-4a15-a08a-29879300fdea
📒 Files selected for processing (3)
charts/cardano-node/Chart.yamlcharts/cardano-node/templates/statefulset.yamlcharts/cardano-node/values.yaml
| securityContext: | ||
| runAsUser: 0 | ||
| runAsGroup: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Init container runs as root — consider fsGroup alternative.
Explicitly setting runAsUser: 0 and runAsGroup: 0 degrades the container's security posture. If this is needed for persistent-volume write access, a pod-level securityContext.fsGroup can grant the container group ownership of mounted volumes without root.
🔒️ Suggested alternative using fsGroup
securityContext:
- runAsUser: 0
- runAsGroup: 0
+ runAsUser: 1000
+ runAsGroup: 1000
image: {{ .Values.mithril.image.repository | default .Values.image.repository }}:{{ .Values.mithril.image.tag | default .Values.image.tag }}And add at the pod level (under spec.template.spec):
securityContext:
fsGroup: 1000This makes the volume writable by the container's group without running as root. Adjust the UID/GID to match the mithril-client image's non-root user.
📝 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.
| securityContext: | |
| runAsUser: 0 | |
| runAsGroup: 0 | |
| securityContext: | |
| runAsUser: 1000 | |
| runAsGroup: 1000 |
🤖 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/cardano-node/templates/statefulset.yaml` around lines 37 - 39, Replace
the init container’s root-based securityContext with a non-root configuration,
and add a pod-level securityContext under spec.template.spec using fsGroup set
to the mithril-client image’s non-root group ID. Remove runAsUser: 0 and
runAsGroup: 0 while preserving persistent-volume write access.
| else | ||
| echo "Fetching latest snapshot digest from ${AGGREGATOR_ENDPOINT}..." | ||
| SNAPSHOT_DIGEST=$(mithril-client cardano-db snapshot list --json | jq -r '.[0].hash') | ||
| SNAPSHOT_DIGEST=$(mithril-client cardano-db snapshot list --json | grep -o '"hash":"[^"]*' | head -n 1 | cut -d'"' -f4) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if mithril-client is referenced elsewhere in the repo with JSON parsing
rg -n 'mithril-client.*--json' --type yaml
rg -n 'snapshot list' --type yamlRepository: blinklabs-io/helm-charts
Length of output: 162
🌐 Web query:
What is the JSON output format of "mithril-client cardano-db snapshot list --json"? Does it use compact or pretty-printed JSON?
💡 Result:
The mithril-client cardano-db snapshot list --json command produces a standard JSON-formatted output of available snapshots [1][2]. According to the project's source code, this output is generated using the serde_json::to_string function [3], which produces compact (minified) JSON, not pretty-printed JSON [3].
Citations:
- 1: https://mithril.network/doc/manual/develop/nodes/mithril-client/
- 2: https://cardano-community.github.io/guild-operators/Scripts/mithril-client/
- 3: IntersectMBO/mithril@f98f9d2
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant template and nearby script context
grep -n -A40 -B40 'SNAPSHOT_DIGEST=' charts/cardano-node/templates/statefulset.yaml
# Look for shell options or any digest validation nearby in the same file
rg -n 'pipefail|set -e|SNAPSHOT_DIGEST|Failed to fetch snapshot digest|hash' charts/cardano-node/templates/statefulset.yamlRepository: blinklabs-io/helm-charts
Length of output: 5694
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the surrounding section with line numbers for precise context
nl -ba charts/cardano-node/templates/statefulset.yaml | sed -n '110,180p'Repository: blinklabs-io/helm-charts
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba charts/cardano-node/templates/statefulset.yaml | sed -n '130,170p'Repository: blinklabs-io/helm-charts
Length of output: 202
Guard the snapshot digest lookup in charts/cardano-node/templates/statefulset.yaml:148. If the fetch pipeline returns nothing, SNAPSHOT_DIGEST stays empty and the later download fails with a vague error; set -o pipefail would also stop the script on a failed fetch.
🤖 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/cardano-node/templates/statefulset.yaml` at line 148, Update the
SNAPSHOT_DIGEST lookup in the snapshot download script to handle an empty result
or failed mithril-client fetch explicitly before continuing. Preserve pipe
failure detection with set -o pipefail, and emit a clear error or exit when no
digest is obtained so the later download is not attempted with an empty value.
4545fa6 to
b14d6ed
Compare
Adds support for specifying a dedicated image, tag, and pullPolicy for the mithril-snapshot initContainer. Sets default pullPolicy to Always to ensure mutable tags (like latest-unstable) are always checked on restart. Note: The built-in mithril-client v0.13.9 in ghcr.io/blinklabs-io/cardano-node:11.0.1-1 has a closed-channel stream race condition bug on GCS (Download: could not write 0 bytes to stream). Suggest using ghcr.io/input-output-hk/mithril-client:latest-unstable (or v2617.0+). Signed-off-by: Ales Verbic <verbotenj@blinklabs.io>
b14d6ed to
699fe78
Compare
2 participants
Adds support for specifying a dedicated image, tag, and pullPolicy for the mithril-snapshot initContainer.
Sets default to unstable (0.13.19+) due to GCS closed-channel stream bug.
Note: The built-in mithril-client v0.13.9 in ghcr.io/blinklabs-io/cardano-node:11.0.1-1 has a closed-channel stream race condition bug on GCS (Download: could not write 0 bytes to stream). Suggest using ghcr.io/input-output-hk/mithril-client:latest-unstable (or v2617.0+).
Summary by cubic
Adds Helm values to override the
mithrilclient image for the snapshot initContainer and hardens the init script for more reliable restores. Defaults toghcr.io/input-output-hk/mithril-client:unstablewith pull policyAlways, runs the init as non‑root, and adds stricter digest checks to avoid the GCS closed‑channel bug inmithril-client v0.13.9.New Features
.Values.mithril.image.{repository,tag,pullPolicy}for themithril-snapshotinitContainer (repository/tag fall back to node image/tag if unset); defaultghcr.io/input-output-hk/mithril-client:unstablewithAlwayspull policy.fsGroup: 1000; add/app/bintoPATH; auto‑resolve default genesis and ancillary keys formainnetandpreview|preprod.jq; parse snapshot list via shell, validate non‑empty digest, and fail fast; ensure a clean unpack directory to prevent Mithril “Unpack directory is not empty” errors.Dependencies
0.8.6,appVersionto11.0.1, and default node image toghcr.io/blinklabs-io/cardano-node:11.0.1-1.Written for commit 699fe78. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes