LCORE-2874: Migrate the main branch of lightspeed-stack to RHOAI index + PyPI sdists#1931
Conversation
Jazzcort
changed the title
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (16)
🧰 Additional context used📓 Path-based instructions (1)**/*📄 CodeRabbit inference engine (Custom checks)
Files:
🔇 Additional comments (3)
WalkthroughThe PR adds a Konflux dependency resolver for RHOAI and PyPI packages, generates hashed requirements and build dependencies, patches Tekton prefetch lists, and adds Python 3.12 profile configuration. It also refreshes base images, RPM locks, dependency pins, hashes, build inputs, and runtime image metadata. ChangesKonflux dependency resolution and build refresh
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CLI as konflux_resolve.py
participant UV as uv pip compile
participant RHOAI as RhoaiIndex
participant PyPI as PypiClient
participant Outputs as Requirements and Tekton files
CLI->>UV: Resolve direct dependencies with prefer-index
UV-->>CLI: Resolved versions and index annotations
CLI->>RHOAI: Reclassify compatible RHOAI wheels and fetch hashes
RHOAI-->>CLI: Wheel metadata and hashes
CLI->>PyPI: Fetch dependency metadata and hashes
PyPI-->>CLI: Package requirements and hashes
CLI->>Outputs: Write hashed requirements and patch prefetch packages
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
|
After checking in with @syedriko, we should wait for the image update on registry.redhat.io instead of using image from quay.io which is a heck for an urgent release. 😁 I'll make this ready for review when they update the image on registry.redhat.io |
Jazzcort
force-pushed
the
update-rhai-base-image-with-better-grade
branch
from
d5b81b4 to
02d7a4f
Compare
|
@syedriko Got it! I was just playing around with the konflux build. I tried to follow the prefetch guideline here but had no luck to make it pass the |
Switch builder and runtime base images from registry.redhat.io/rhai/base-image-cpu-rhel9:3.2 to quay.io/aipcc/base-images/cpu:3.5.0-1782914735
Jazzcort
force-pushed
the
update-rhai-base-image-with-better-grade
branch
10 times, most recently
from
61e43a5 to
631153a
Compare
Jazzcort
force-pushed
the
update-rhai-base-image-with-better-grade
branch
from
631153a to
9217b4c
Compare
Jazzcort
changed the title
Jazzcort
changed the title
|
@tisnik The konflux build succeeded. It should be good to merge. This is a patch for the konflux build so any test failures are not relevant to this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/konflux_resolve.py`:
- Around line 573-591: Update _eval_single_marker so comparisons involving
python_version parse both operands as versions before applying the selected
operator, avoiding lexicographic ordering such as treating 3.12 as less than
3.9. Preserve existing string comparison behavior for non-version markers and
the current fallback behavior for unsupported operators or unresolved variables.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 70e121f4-2cfa-46ab-97ba-2594c605a193
📒 Files selected for processing (14)
.konflux/build-args-konflux.conf.konflux/profiles.toml.konflux/pypi_wheel_only.txt.konflux/requirements-build.txt.konflux/requirements.hashes.source.txt.konflux/requirements.hashes.wheel.pypi.txt.konflux/requirements.hashes.wheel.txt.konflux/requirements.hermetic.txt.konflux/requirements.overrides.txt.konflux/rpms.lock.yaml.tekton/lightspeed-stack-0-7-pull-request.yaml.tekton/lightspeed-stack-0-7-push.yamlpyproject.tomlscripts/konflux_resolve.py
💤 Files with no reviewable changes (1)
- .konflux/requirements.overrides.txt
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
- GitHub Check: E2E: server mode / ci / group 2
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
pyproject.tomlscripts/konflux_resolve.py
🧠 Learnings (1)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
scripts/konflux_resolve.py
🪛 ast-grep (0.44.1)
scripts/konflux_resolve.py
[warning] 204-204: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(pyproject_path, "rb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 914-914: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(output_path, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 925-925: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(yaml_path)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 932-932: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(yaml_path, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 950-950: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(profiles_path, "rb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 972-972: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(path)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 1151-1151: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(build_file)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 1166-1166: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(build_file, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 1269-1269: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(tmp_sdist_file, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 1293-1293: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(build_output, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[error] 1040-1040: Command coming from incoming request
Context: subprocess.run(cmd, capture_output=True, text=True, check=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 1273-1283: Command coming from incoming request
Context: subprocess.run(
[
"uv",
"run",
"pybuild-deps",
"compile",
f"--output-file={build_output}",
tmp_sdist_file,
],
check=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[warning] 417-417: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(url, timeout=30)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
[warning] 609-609: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(url, timeout=30)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
[error] 1040-1040: Use of unsanitized data to create processes
Context: subprocess.run(cmd, capture_output=True, text=True, check=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
🔇 Additional comments (15)
scripts/konflux_resolve.py (4)
1070-1114: 📐 Maintainability & Code Quality
reclassify_with_rhoaistill looks like dead code unless it’s wired into theuv_resolveflow. If it remains in use, align the input type withuv_resolve’s result shape before calling it.
1235-1240: 🩺 Stability & AvailabilityHandle the no-match path here If
RhoaiIndex.find_bestcan returnNone, add a guard or fallback before continuing so the package doesn't reach hashing with no platforms.
1206-1216: 🎯 Functional CorrectnessRHOAI detection should use the configured index URL
packages.redhat.comis too specific here; if the RHOAI index is hosted elsewhere, packages will be misclassified aspypi. Match againstrhoai_index_url(or its host) instead.
42-53: 🎯 Functional CorrectnessPotential version-collision bug in
parse_versionIf 4-segment versions are possible,_VERSION_REdrops everything beyond the third numeric component, so1.2.3.4and1.2.3.9collapse to the same sort key. That makesmax(..., key=parse_version)order-dependent and can widen exact matches. Capture trailing numeric segments if these versions are expected..konflux/build-args-konflux.conf (1)
1-3: LGTM!.konflux/profiles.toml (1)
1-12: LGTM!.konflux/pypi_wheel_only.txt (1)
1-5: LGTM!.konflux/requirements.hermetic.txt (1)
1-1: LGTM!.konflux/rpms.lock.yaml (1)
8-71: LGTM!Also applies to: 82-145
pyproject.toml (1)
189-189: LGTM!.konflux/requirements.hashes.source.txt (1)
5-137: LGTM!.konflux/requirements.hashes.wheel.pypi.txt (1)
1-1: Confirm this index-url-only file is intentionally wired.This file contains only an
--index-urldirective and no pins. Note it is not listed among therequirements_filesin either.tekton/lightspeed-stack-0-7-pull-request.yamlor.tekton/lightspeed-stack-0-7-push.yaml(which referencerequirements.hashes.wheel.txt,requirements.hashes.source.txt,requirements.hermetic.txt). Please confirm any PyPI-wheel-classified packages are not being silently dropped from the hermetic prefetch..konflux/requirements.hashes.wheel.txt (1)
141-148: Verifyhttpcore2andhttpx2are legitimate, expected dependencies.
httpcore2==2.5.0andhttpx2==2.5.0are unusual names alongside the standardhttpcore==1.0.9/httpx==0.28.1entries. Since these are binary-prefetched wheels, please confirm they are genuine transitive deps from the RHOAI index and not accidental/typosquat entries pulled into the lock..tekton/lightspeed-stack-0-7-pull-request.yaml (1)
56-56: Verify the prefetchbinary.packageslist stays in sync withrequirements.hashes.wheel.txt. In hermetic mode, any wheel-lock package missing frombinary.packagescan force a source build and break the pipeline; both Tekton lists should match and include the extra build wheels (maturin,pip,uv)..konflux/requirements-build.txt (1)
16-24: Duplicatehatchlingpins need downstream confirmationhatchlingappears at both1.26.3and1.31.0in the same build requirements file; make sure the build/prefetch path accepts multiple versions of the same package.
| def _eval_single_marker(expr: str, env: dict[str, str]) -> bool: | ||
| """Evaluate a single marker comparison like ``sys_platform == 'linux'``.""" | ||
| m = _MARKER_COMPARE_RE.match(expr) | ||
| if m is None: | ||
| return True | ||
|
|
||
| left_raw = m.group("left").strip("'\"") | ||
| right_raw = m.group("right").strip("'\"") | ||
| op = re.sub(r"\s+", " ", m.group("op")) | ||
|
|
||
| if left_raw in env: | ||
| lval, rval = env[left_raw], right_raw | ||
| elif right_raw in env: | ||
| lval, rval = left_raw, env[right_raw] | ||
| else: | ||
| return True | ||
|
|
||
| check = _MARKER_CMP_OPS.get(op) | ||
| return bool(check(lval, rval)) if check else True |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant function and nearby helpers.
rg -n "_eval_single_marker|_MARKER_CMP_OPS|python_version|parse_version" scripts/konflux_resolve.py
# Show the relevant section with line numbers.
sed -n '520,620p' scripts/konflux_resolve.py
# Run a small read-only probe to demonstrate the comparison behavior.
python3 - <<'PY'
from functools import cmp_to_key
def lex(a, b):
return (a > b) - (a < b)
pairs = [("3.12", "3.9"), ("3.10", "3.9"), ("3.9", "3.12")]
for a, b in pairs:
print(a, ">=", b, lex(a, b) >= 0)
print(a, "<", b, lex(a, b) < 0)
PYRepository: lightspeed-core/lightspeed-stack
Length of output: 1987
Compare python_version as a version, not a string. "_eval_single_marker" currently uses plain string ordering, so markers like python_version >= "3.9" can evaluate incorrectly on values such as "3.12", dropping valid dependencies. Parse version-valued markers before comparing.
🤖 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 `@scripts/konflux_resolve.py` around lines 573 - 591, Update
_eval_single_marker so comparisons involving python_version parse both operands
as versions before applying the selected operator, avoiding lexicographic
ordering such as treating 3.12 as less than 3.9. Preserve existing string
comparison behavior for non-version markers and the current fallback behavior
for unsupported operators or unresolved variables.
|
/retest |
1 similar comment
|
/retest |
Jazzcort
force-pushed
the
update-rhai-base-image-with-better-grade
branch
from
50610d2 to
be0cbc9
Compare
3 participants
Description
Switch builder and runtime base images from registry.redhat.io/rhai/base-image-cpu-rhel9:3.2 to quay.io/aipcc/base-images/cpu:3.5.0-1782914735.
Package preference order: RHOAI index wheel -> Source distribution -> PyPI wheel
Thanks @syedriko for the guidance on this one!
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
LCORE-1435
LCORE-2874
Checklist before requesting a review
Testing
Check Konflux console, those vulnerability warnings should go away.
Summary by CodeRabbit
Summary