Add IronCore-in-a-Box integration test targets#722
Conversation
Introduce `make iiab-tests` which clones ironcore-in-a-box, builds a local libvirt-provider image, and runs the iiab test suite using the native LIBVIRT_PROVIDER_CONFIG_DIR / LIBVIRT_PROVIDER_IMAGE_TAG overlay support. IIAB_BRANCH is configurable for coordinated cross-repo work. Use a separate kind cluster (iiab-libvirt-provider-test) to avoid conflicting with an existing ironcore-in-a-box dev cluster. Add IIAB_LOCAL flag to skip git operations and preserve the directory on clean. Add iiab-tests-local Makefile target for local checkouts. Signed-off-by: Felix Riegger <felix.riegger@sap.com>
📝 WalkthroughWalkthroughThis PR introduces complete infrastructure for running IronCore-in-a-Box integration tests. It adds Make targets and configuration variables, a Bash test orchestrator script that manages repository checkout/building/cleanup, ChangesIronCore-in-a-Box Integration Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
🚥 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 docstrings
🧪 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: 4
🤖 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 @.github/workflows/iiab-tests.yml:
- Line 19: The workflow hardcodes IIAB_BRANCH in the run step which risks
breakage when that feature branch is removed; update the run invocation of "make
iiab-tests" to either omit the IIAB_BRANCH assignment so the Makefile default is
used or set IIAB_BRANCH to a stable branch name (e.g., main) instead;
specifically edit the line containing the run: make iiab-tests
IIAB_BRANCH=enh/allow-image-and-config-overwrites2 to remove the "=enh/..."
suffix or replace it with a stable branch, ensuring the CI uses the Makefile
default or a permanent branch.
- Around line 14-15: Update the "Check out libvirt-provider" step to pin
actions/checkout to the immutable SHA
actions/checkout@0c366fd6a839edf440554fa01a7085ccba70ac98 and add with:
persist-credentials: false to avoid credential persistence; also stop hardcoding
IIAB_BRANCH by making it configurable (for example switch IIAB_BRANCH to come
from a workflow input or environment variable with the current branch name as
the default) so downstream uses of IIAB_BRANCH are not tied to
enh/allow-image-and-config-overwrites2.
In `@hack/iiab-tests.sh`:
- Around line 24-26: The removal of ${IIAB_DIR} with rm -rf is unsafe if
IIAB_DIR is unset or points to a critical path; add a guard that validates
IIAB_DIR before deleting: ensure IIAB_DIR is non-empty, not “/” or the root
device, and resolves via realpath to a directory under an expected parent (e.g.,
a known workspace or $HOME prefix) or matches a safe pattern (contains “iiab” or
similar), and if the check fails print an error and exit non‑zero instead of
running rm -rf. Use the existing IIAB_DIR variable and the rm -rf invocation as
the target to protect.
- Around line 34-39: When IIAB_LOCAL is set and IIAB_DIR exists the script
currently skips initializing git submodules, which can cause `make test` to
fail; after the existing directory check for IIAB_DIR (the IIAB_LOCAL branch)
invoke git submodule sync --quiet --recursive and git submodule update --init
--recursive (inside the IIAB_DIR) and fail the script if those commands return
non‑zero so submodules are guaranteed initialized before running make test;
reference the IIAB_LOCAL and IIAB_DIR variables when adding this logic and
ensure errors are reported and exit with non-zero status on failure.
🪄 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: 6c6664aa-a063-496c-bf10-a2c5523c7d26
📒 Files selected for processing (4)
.github/workflows/iiab-tests.yml.gitignoreMakefilehack/iiab-tests.sh
| - name: Check out libvirt-provider | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the current immutable commit SHA for actions/checkout v6?
💡 Result:
The current immutable commit SHA for actions/checkout v6 is not explicitly stated in the sources returned by this research session; however, the v6 major tag is mutable and points to the latest v6 release commit at call time. For example, v6.0.2 is a specific release, but its underlying full commit SHA is not present in the retrieved pages/snippets, so I can’t provide a verified full-length immutable SHA for “v6” from the evidence gathered here.[1][2] If you want, I can run one more targeted lookup specifically on the v6 tag page (e.g., the “v6” tag object) to extract the exact full commit SHA that the v6 tag currently points to.
Citations:
- 1: https://github.com/actions/checkout/tree/v6.0.2
- 2: https://github.com/actions/checkout/releases/tag/v6.0.2
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/iiab-tests.yml"
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,120p'Repository: ironcore-dev/libvirt-provider
Length of output: 179
🌐 Web query:
actions/checkout persist-credentials default value
💡 Result:
The default value of actions/checkout input persist-credentials is true (i.e., credentials are persisted unless you set persist-credentials: false). [1][2] Evidence: the actions/checkout documentation lists the input and indicates persist-credentials defaults to “true” (shown as “Default: true” in the inputs definition). [1] Additionally, the commit that introduced the input states “Whether to persist the token in the git config” with “Default: true”. [1] The README further explains that the auth token is persisted in the local git config by default and can be opted out by setting persist-credentials: false. [3]
Citations:
- 1: actions/checkout@c170eef
- 2: https://github.com/actions/checkout/tree/refs/heads/main
- 3: https://github.com/actions/checkout/?tab=readme-ov-file
🌐 Web query:
GitHub actions/checkout tag v6 immutable commit SHA (the commit the v6 tag points to)
💡 Result:
The Git tag v6 for the GitHub Action actions/checkout currently points to commit SHA 0c366fd6a839edf440554fa01a7085ccba70ac98 (i.e., the commit for release v6.0.0). [1] Note: different v6 sub-tags (for example, v6.0.0, v6.0.1, v6.0.2) resolve to different SHAs; the mutable major tag v6 maps to whichever commit GitHub advanced it to in the actions/checkout repository’s tag refs. [2][3]
Citations:
- 1: https://github.com/actions/checkout/tree/v6.0.0
- 2: https://github.com/actions/checkout/releases
- 3: https://github.com/actions/checkout/tree/v6.0.2
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/iiab-tests.yml"
echo "== Head (1-80) of $FILE =="
awk '{printf "%5d:%s\n", NR, $0}' "$FILE" | sed -n '1,80p'
echo "== Grep for IIAB_BRANCH =="
rg -n "IIAB_BRANCH" "$FILE" || true
echo "== Checkout step context =="
awk 'NR>=1 && NR<=120 {print}' "$FILE" | sed -n '/- name: Check out libvirt-provider/,+8p'Repository: ironcore-dev/libvirt-provider
Length of output: 1183
Pin actions/checkout to an immutable SHA and disable credential persistence.
- In
.github/workflows/iiab-tests.yml(line 15),uses: actions/checkout@v6is mutable; pin toactions/checkout@0c366fd6a839edf440554fa01a7085ccba70ac98. - In the same step, set
with: persist-credentials: false(default istrue, so credentials get persisted today). - In
.github/workflows/iiab-tests.yml(line 19),IIAB_BRANCHis hardcoded toenh/allow-image-and-config-overwrites2, which can break CI if the branch changes; make it configurable.
Proposed fix
- name: Check out libvirt-provider
- uses: actions/checkout@v6
+ uses: actions/checkout@0c366fd6a839edf440554fa01a7085ccba70ac98
+ with:
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 14-15: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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/iiab-tests.yml around lines 14 - 15, Update the "Check out
libvirt-provider" step to pin actions/checkout to the immutable SHA
actions/checkout@0c366fd6a839edf440554fa01a7085ccba70ac98 and add with:
persist-credentials: false to avoid credential persistence; also stop hardcoding
IIAB_BRANCH by making it configurable (for example switch IIAB_BRANCH to come
from a workflow input or environment variable with the current branch name as
the default) so downstream uses of IIAB_BRANCH are not tied to
enh/allow-image-and-config-overwrites2.
| - name: Install test dependencies | ||
| run: sudo apt-get update && sudo apt-get install -yqq sshpass | ||
| - name: Run iiab tests | ||
| run: make iiab-tests IIAB_BRANCH=enh/allow-image-and-config-overwrites2 |
There was a problem hiding this comment.
Avoid hardcoding a feature branch in CI.
Line 19 pins tests to enh/allow-image-and-config-overwrites2; this can break once that branch is deleted/renamed. Prefer default IIAB_BRANCH from Makefile or set it to a stable branch.
Proposed fix
- name: Run iiab tests
- run: make iiab-tests IIAB_BRANCH=enh/allow-image-and-config-overwrites2
+ run: make iiab-tests📝 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.
| run: make iiab-tests IIAB_BRANCH=enh/allow-image-and-config-overwrites2 | |
| - name: Run iiab tests | |
| run: make iiab-tests |
🤖 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/iiab-tests.yml at line 19, The workflow hardcodes
IIAB_BRANCH in the run step which risks breakage when that feature branch is
removed; update the run invocation of "make iiab-tests" to either omit the
IIAB_BRANCH assignment so the Makefile default is used or set IIAB_BRANCH to a
stable branch name (e.g., main) instead; specifically edit the line containing
the run: make iiab-tests IIAB_BRANCH=enh/allow-image-and-config-overwrites2 to
remove the "=enh/..." suffix or replace it with a stable branch, ensuring the CI
uses the Makefile default or a permanent branch.
| echo "==> Removing ${IIAB_DIR}..." | ||
| rm -rf "${IIAB_DIR}" | ||
| fi |
There was a problem hiding this comment.
Add a safety guard before deleting IIAB_DIR.
Line 25 can delete an arbitrary path if IIAB_DIR is accidentally set to something unsafe. Add explicit path validation before rm -rf.
Proposed fix
else
echo "==> Removing ${IIAB_DIR}..."
- rm -rf "${IIAB_DIR}"
+ case "${IIAB_DIR}" in
+ ""|"/")
+ echo "ERROR: refusing to remove unsafe IIAB_DIR='${IIAB_DIR}'" >&2
+ exit 1
+ ;;
+ esac
+ rm -rf "${IIAB_DIR}"
fi
}📝 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.
| echo "==> Removing ${IIAB_DIR}..." | |
| rm -rf "${IIAB_DIR}" | |
| fi | |
| echo "==> Removing ${IIAB_DIR}..." | |
| case "${IIAB_DIR}" in | |
| ""|"/") | |
| echo "ERROR: refusing to remove unsafe IIAB_DIR='${IIAB_DIR}'" >&2 | |
| exit 1 | |
| ;; | |
| esac | |
| rm -rf "${IIAB_DIR}" | |
| 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/iiab-tests.sh` around lines 24 - 26, The removal of ${IIAB_DIR} with rm
-rf is unsafe if IIAB_DIR is unset or points to a critical path; add a guard
that validates IIAB_DIR before deleting: ensure IIAB_DIR is non-empty, not “/”
or the root device, and resolves via realpath to a directory under an expected
parent (e.g., a known workspace or $HOME prefix) or matches a safe pattern
(contains “iiab” or similar), and if the check fails print an error and exit
non‑zero instead of running rm -rf. Use the existing IIAB_DIR variable and the
rm -rf invocation as the target to protect.
| if [ -n "${IIAB_LOCAL}" ]; then | ||
| echo "==> Using local ironcore-in-a-box at ${IIAB_DIR}" | ||
| if [ ! -d "${IIAB_DIR}" ]; then | ||
| echo "ERROR: IIAB_LOCAL is set but ${IIAB_DIR} does not exist" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Initialize submodules in local mode too.
Line 34 path only checks directory existence; if submodules are not initialized, Line 58 make test may fail. Run submodule sync/init/update in the local branch as well.
Proposed fix
if [ -n "${IIAB_LOCAL}" ]; then
echo "==> Using local ironcore-in-a-box at ${IIAB_DIR}"
if [ ! -d "${IIAB_DIR}" ]; then
echo "ERROR: IIAB_LOCAL is set but ${IIAB_DIR} does not exist" >&2
exit 1
fi
+ cd "${IIAB_DIR}" && git submodule update --init --recursive
else📝 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.
| if [ -n "${IIAB_LOCAL}" ]; then | |
| echo "==> Using local ironcore-in-a-box at ${IIAB_DIR}" | |
| if [ ! -d "${IIAB_DIR}" ]; then | |
| echo "ERROR: IIAB_LOCAL is set but ${IIAB_DIR} does not exist" >&2 | |
| exit 1 | |
| fi | |
| if [ -n "${IIAB_LOCAL}" ]; then | |
| echo "==> Using local ironcore-in-a-box at ${IIAB_DIR}" | |
| if [ ! -d "${IIAB_DIR}" ]; then | |
| echo "ERROR: IIAB_LOCAL is set but ${IIAB_DIR} does not exist" >&2 | |
| exit 1 | |
| fi | |
| cd "${IIAB_DIR}" && git submodule update --init --recursive |
🤖 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/iiab-tests.sh` around lines 34 - 39, When IIAB_LOCAL is set and IIAB_DIR
exists the script currently skips initializing git submodules, which can cause
`make test` to fail; after the existing directory check for IIAB_DIR (the
IIAB_LOCAL branch) invoke git submodule sync --quiet --recursive and git
submodule update --init --recursive (inside the IIAB_DIR) and fail the script if
those commands return non‑zero so submodules are guaranteed initialized before
running make test; reference the IIAB_LOCAL and IIAB_DIR variables when adding
this logic and ensure errors are reported and exit with non-zero status on
failure.
2 participants
Proposed Changes
Introduce
make iiab-testswhich clones ironcore-in-a-box, builds a local libvirt-provider image, and runs the iiab test suite using the native LIBVIRT_PROVIDER_CONFIG_DIR / LIBVIRT_PROVIDER_IMAGE_TAG overlay support. IIAB_BRANCH is configurable for coordinated cross-repo work.Use a separate kind cluster (iiab-libvirt-provider-test) to avoid conflicting with an existing ironcore-in-a-box dev cluster. Add IIAB_LOCAL flag to skip git operations and preserve the directory on clean. Add iiab-tests-local Makefile target for local checkouts.
Contributes to #719
Summary by CodeRabbit
New Features
Chores