randomparity / randomparity/kdive

Preflight runtime-dir drift guard compares env-resolved constants, so a moved knob reds just ci

Open Beginner friendly
#2,585 0 comments 0 reactions 0 assignees View on GitHub
area:build-install effort:S priority:P2 risk:night-safe status:ready type:bug type:test
Dominant language
Python
Stars
0
Forks
0
Avg merge
1h 26m
Merged PRs (30d)
311

Description

## Problem

`test_host_runtime_dirs_cover_every_hardcoded_provider_path` compares the preflight script's
declared runtime dirs against constants that are **not** literals: two of the three resolve through
`config.require(...)` at import time and therefore follow whatever the developer has set in the
environment. An operator who legitimately points `KDIVE_LIBVIRT_ROOTFS_ROOT` or
`KDIVE_LIBVIRT_CONSOLE_ROOT` elsewhere gets a red `just ci` on code they did not touch.

This is the same defect #2549 fixed in `tests/scripts/test_live_workflow_shape.py`, in a different
file. #2549's own fix is the pattern: compare against `.default`, the env-independent
source of truth the script literal actually mirrors.

The test's failure message makes the contradiction visible: it says the value *is hardcoded in src*
while printing a value that came from the environment.

## Evidence

- `tests/scripts/test_live_vm_preflight.py:296-309` — the assertion loop:

```python
declared = set(_script_runtime_dirs())
for const in (runtime_paths._CONSOLE_DIR, runtime_paths._PCAP_DIR, storage.ROOTFS_DIR):
assert const in declared, f"{const} is hardcoded in src but not preflighted"
```

The docstring states the premise the code relies on — "these paths are constants in src, so they
cannot be pointed elsewhere" — and two of the three do not meet it.

- `src/kdive/providers/shared/runtime_paths.py:20` — `_CONSOLE_DIR = config.require(LIBVIRT_CONSOLE_ROOT)`
- `src/kdive/providers/local_libvirt/lifecycle/storage.py` — `ROOTFS_DIR = config.require(LIBVIRT_ROOTFS_ROOT)`
- `src/kdive/providers/shared/runtime_paths.py:21` — `_PCAP_DIR = "/var/lib/kdive/pcap"` is a genuine
literal and is not affected.

Reproduced directly, both arms, against a clean tree:

```
$ KDIVE_LIBVIRT_ROOTFS_ROOT=/custom/other-root pytest ...::test_host_runtime_dirs_cover_every_hardcoded_provider_path
E AssertionError: /custom/other-root is hardcoded in src but not preflighted
E assert '/custom/other-root' in {'/var/lib/kdive/console', '/var/lib/kdive/pcap', '/var/lib/kdive/rootfs'}

$ KDIVE_LIBVIRT_CONSOLE_ROOT=/custom/console-root pytest ... -> FAILED
$ pytest ... -> 1 passed
```

Prior art for the fix, already merged: commit `8999b024a` (#2549) replaced `storage.ROOTFS_DIR` with
`LIBVIRT_ROOTFS_ROOT.default` in `test_live_workflow_shape.py` for exactly this reason, and
`test_live_workflow_shape.py:113-114` carries a comment warning against comparing to the resolved
constant. That warning did not reach this file.

## Expected

The test's verdict does not depend on the developer's environment. With any combination of
`KDIVE_LIBVIRT_ROOTFS_ROOT` and `KDIVE_LIBVIRT_CONSOLE_ROOT` set, it still passes on an unmodified
tree, and still fails when a genuinely new hardcoded runtime dir is added to `src` without being
added to the preflight script — which is the drift it exists to catch.

## Proposed approach

1. Compare against `LIBVIRT_CONSOLE_ROOT.default` and `LIBVIRT_ROOTFS_ROOT.default` rather than the
resolved `runtime_paths._CONSOLE_DIR` and `storage.ROOTFS_DIR`. Keep `_PCAP_DIR` as-is; it is a
real literal.
2. Fix the assertion message, which currently asserts the value is hardcoded while printing an
environment-supplied one.
3. Verify the guard still bites: add a hardcoded runtime dir in `src` without declaring it in the
preflight script, observe red, revert. Then re-run both env arms above and observe green. A fix
that only makes the test stop failing has removed a guard rather than corrected it.

This is the last site of this shape. One search over `tests/scripts/` and `tests/guards/` for
comparisons against `config.require`-resolved constants returns only this one; the two sites in
`test_live_workflow_shape.py` already use `.default`.

## Out of scope

- Changing `runtime_paths._CONSOLE_DIR` or `ROOTFS_DIR` themselves, or whether they should be
env-overridable. They are, deliberately; only the test's comparison is wrong.
- The preflight script's own contents.

## Provenance

Found by the implementer of #2549 while fixing the sibling occurrence, reported as adjacent and
deliberately excluded from PR #2575 as out of the approved scope. Independently reproduced before
filing. Filed at the repository operator's explicit request.

Contributor guide

Open the contributing guide

Research direction

Start in tests/scripts/test_live_vm_preflight.py:296-309 and compare its assertion with the already-merged pattern in tests/scripts/test_live_workflow_shape.py:113-114. Run the focused preflight test with both KDIVE_LIBVIRT_ROOTFS_ROOT and KDIVE_LIBVIRT_CONSOLE_ROOT overridden, then verify it still fails when an undeclared hardcoded runtime directory is introduced and passes after that change is reverted.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing-qa
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
90/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.