quickstart: preflight-check podman and jq before execution#215
quickstart: preflight-check podman and jq before execution#215ggiguash merged 2 commits intomicroshift-io:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds early host validation to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/quickstart.sh`:
- Around line 38-51: The unconditional check for jq (and missing check for curl)
should be gated to the same condition used when fetching the remote latest-tag:
wrap or move the dependency checks so they run only when [[ "${IMAGE}" !=
localhost/* ]] && [ "${TAG}" == "latest" ]; add a curl availability check
(command -v curl &>/dev/null) alongside jq and emit the same user-friendly
install hints if either is missing; update the quickstart.sh block that
currently tests jq to be executed only under that IMAGE/TAG condition so local
or pinned-tag runs do not fail unnecessarily.
- Around line 28-35: Validate podman presence and robustly parse its version
into podman_version using a regex (e.g., extract first X.Y.Z numeric sequence
from `podman --version`) instead of assuming field 3, then compare full semantic
versions (not only podman_major) against "4.0.0" using a reliable comparator
(e.g., a small version_compare function that uses sort -V or compares numeric
fields) to decide failure; update the error message to include the detected
version and a short note that the script requires ≥4.0.0 because it relies on
features used later (e.g., `podman run --replace`), and keep the variable names
podman_version and podman_major (if still used) to locate the change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f56f021-c472-42b3-a872-c243a6cfa8e7
📒 Files selected for processing (1)
src/quickstart.sh
|
@ggiguash thank you for great feedback, PR rebased. |
Add check_prerequisites() to fail fast with actionable install instructions when podman (or an outdated version <4) or jq is missing, rather than hitting a cryptic command-not-found mid-run. Package manager detection covers dnf, apt-get, and zypper.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/quickstart.sh (1)
33-37:⚠️ Potential issue | 🟡 MinorHarden podman version parsing before numeric compare.
Line 34 assumes a fixed token position, and Line 37 does integer comparison without validating numeric input. A malformed version string can bypass the intended guard or emit test errors.
Suggested hardening
function check_podman_version() { - local podman_version - podman_version="$(podman --version | awk '/^podman version /{print $3}')" - local podman_major - podman_major="$(echo "${podman_version}" | cut -d. -f1)" - if [ -z "${podman_major}" ] || [ "${podman_major}" -lt "${PODMAN_VMAJOR}" ]; then + local podman_version_output + podman_version_output="$(podman --version)" + local podman_version + podman_version="$(awk 'match($0, /[0-9]+([.][0-9]+)*/) { print substr($0, RSTART, RLENGTH); exit }' <<<"${podman_version_output}")" + local podman_major + podman_major="${podman_version%%.*}" + if [[ -z "${podman_major}" || ! "${podman_major}" =~ ^[0-9]+$ ]] || (( podman_major < PODMAN_VMAJOR )); then echo "ERROR: podman ${podman_version:-unknown} is too old (minimum required: ${PODMAN_VMAJOR}.0)" echo "Please upgrade podman and try again." exit 1 fi }#!/bin/bash # Read-only verification: demonstrate current parsing fragility with variant version strings. set -u parse_current() { local out="$1" local podman_version podman_version="$(awk '/^podman version /{print $3}' <<<"$out")" local podman_major podman_major="$(echo "${podman_version}" | cut -d. -f1)" printf 'input=%q -> podman_version=%q podman_major=%q | ' "$out" "$podman_version" "$podman_major" if [ -z "${podman_major}" ] || [ "${podman_major}" -lt "4" ] 2>/dev/null; then echo "rejected" else echo "accepted" fi } parse_current "podman version 4.9.3" parse_current "podman version v4.9.3" parse_current "podman version unknown"
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 11d21bed-b038-44a7-bb7a-091da8094d73
📒 Files selected for processing (1)
src/quickstart.sh
Add check_prerequisites() to fail fast with actionable install instructions when podman version is outdated (<4.0) or jq is missing, rather than hitting a cryptic command-not-found mid-run. Package manager detection covers dnf, apt-get, and zypper.
Summary by CodeRabbit