Update RHOAI index to 3.4#1991
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughKonflux build inputs now target a CPU profile with refreshed base images, resolver inputs, and pinned build requirements. A new resolver script generates hashed requirements and patches Tekton inputs. The PR also adds RPM lock generation tooling and updates the lockfile for both architectures. ChangesKonflux build and lockfile updates
Sequence Diagram(s)sequenceDiagram
participant main as main()
participant uv as uv pip compile
participant rhoai as RhoaiIndex
participant pypi as PypiClient
participant write as write_hashed_requirements
participant patch as patch_tekton_packages
main->>uv: resolve pinned packages
uv-->>main: versions and index annotations
main->>rhoai: fetch compatible wheel hashes
main->>pypi: fetch hashes and requires_dist
main->>write: emit hashed requirements files
main->>patch: update Tekton packages lists
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
syedriko
force-pushed
the
syedriko-rhoai-3.4
branch
3 times, most recently
from
1ff754d to
e13805f
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: 10
🤖 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 @.konflux/build-args-konflux.conf:
- Around line 1-3: The DNF command build arg names are inconsistent between the
Konflux build-args config and the Containerfile, so the value from DNF_COMMAND
is never used. Update .konflux/build-args-konflux.conf and
deploy/lightspeed-stack/Containerfile to use the same arg names, either by
restoring BUILDER_DNF_COMMAND and RUNTIME_DNF_COMMAND in the config or by
renaming the Containerfile’s builder/runtime DNF args to DNF_COMMAND so they
match.
In @.konflux/requirements.hashes.wheel.txt:
- Around line 155-157: The multiline requirements entries are broken because
several package blocks in requirements.hashes.wheel.txt have a non-final --hash
line without the trailing continuation backslash. Update the affected entries
for jeepney==0.9.0, markdown-it-py==4.2.0, and oracledb==3.4.2 so every hash
line except the last one in each block ends with a backslash, matching the
existing multiline syntax used by the file.
In `@scripts/generate-rpm-lock.sh`:
- Around line 5-8: The RPM lock generator is still defaulting to the old
root-level inputs and lockfile plus the stale base image, so update
generate-rpm-lock.sh to use the committed Konflux files and current image
settings. In the script’s defaults and argument handling, point INPUT_FILE,
BUILD_ARGS_FILE, and OUTPUT_FILE at the .konflux artifacts, and make the
generator work with container-local basenames while copying the generated
lockfile back to the host path instead of writing directly into .konflux inside
the container. Also update the related command construction and path references
in the script’s main flow so rpm-lockfile-prototype uses the new defaults
consistently.
- Around line 35-40: The RPM lock generator currently requires -a and -g flags,
which breaks the make target that runs it without arguments. Update the argument
handling in generate-rpm-lock.sh, especially the usage/parse logic and the main
subscription-manager setup, so ACTIVATION_KEY and ORG_ID can fall back to
REDHAT_ACTIVATION_KEY and REDHAT_ORG_ID from the environment when flags are
omitted. Keep the existing CLI flags working, but allow make konflux-rpm-lock to
succeed by resolving the values from either args or environment before any
validation.
- Around line 113-118: The registry login in the generate-rpm-lock flow is
passing the password as a command-line argument, which exposes it in the process
list. Update the skopeo login call inside the
REGISTRY_USERNAME/REGISTRY_PASSWORD block to use stdin-based authentication
instead, and make sure the $CONTAINER_RUNTIME exec invocation is interactive so
the password can be piped securely. Keep the change localized to the registry
login logic that uses REGISTRY_HOST and skopeo login.
- Around line 93-97: The cleanup flow only deletes the local container state via
cleanup(), so the RHSM consumer remains registered. Update cleanup() to perform
a best-effort subscription-manager unregister inside the container before
calling $CONTAINER_RUNTIME rm -f on $CONTAINER_NAME, and keep the existing trap
cleanup EXIT so this runs on all exits.
In `@scripts/konflux_resolve.py`:
- Around line 1318-1325: The Tekton binary.packages list is missing
build-dependency wheels that are already added to
requirements.hashes.wheel.pypi*.txt, so the prefetched set can drift from what
the Containerfile installs. Update the wheel_package_names assembly in
konflux_resolve.py to include build_pkg_names alongside rhoai_wheel, pypi_wheel,
and bootstrap_packages, then let patch_tekton_packages use the expanded set when
rewriting the Tekton templates.
- Around line 901-907: The hash rendering in the requirements output should not
silently fall back to plain name==version when a hashed entry has no hashes.
Update the logic in the hashed artifact generation path around sorted_hashes
handling so that requirements.hashes.*.txt generation fails fast when hashes is
empty, instead of appending an unhashed requirement; use the surrounding hash
resolution flow in scripts/konflux_resolve.py to raise an error at source for
this contract violation.
- Around line 893-899: The hashed requirements generated by
write_hashed_requirements() are currently mixing sdist_hashes and wheel_hashes
for the pypi_sdist bucket, which lets requirements.hashes.source*.txt resolve
wheels instead of being source-only. Update the hash collection logic in
scripts/konflux_resolve.py so source requirements only include sdist artifact
hashes, either by filtering on artifact kind when building the hashes set or by
adding an explicit source-only mode in write_hashed_requirements(). Ensure the
behavior applies to the source requirements emitted for
requirements.hashes.source*.txt.
- Around line 989-1022: uv_resolve() is still hardcoded to a single x86_64
platform, which means dependency resolution ignores the configured
multi-platform build matrix. Update uv_resolve() to iterate over
config["platforms"], run the uv pip compile solve per platform instead of using
the fixed x86_64-manylinux_2_28 target, and merge the results so
platform-specific packages are preserved before hash fetching.
🪄 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: b6e4dd09-3796-446f-a674-a4847966b4a2
📒 Files selected for processing (19)
.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-6-pull-request.yaml.tekton/lightspeed-stack-0-6-push.yaml.tekton/lightspeed-stack-pull-request.yaml.tekton/lightspeed-stack-push.yamlMakefiledeploy/lightspeed-stack/Containerfilescripts/generate-rpm-lock.shscripts/konflux_requirements.shscripts/konflux_resolve.py
💤 Files with no reviewable changes (1)
- .konflux/rpms.lock.yaml
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Pylinter
- GitHub Check: unit_tests (3.13)
- GitHub Check: radon
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-05-12T15:14:34.788Z
Learnt from: syedriko
Repo: lightspeed-core/lightspeed-stack PR: 1727
File: scripts/konflux_requirements.sh:9-15
Timestamp: 2026-05-12T15:14:34.788Z
Learning: In this repo, the `.konflux/` directory is committed/tracked and is guaranteed to exist in a fresh clone. Therefore, shell scripts that write output under `.konflux/` (e.g., create files like `.konflux/<...>`) should not waste effort by calling `mkdir -p .konflux` first. Only add directory-creation logic if the script may run in an environment/repo state where `.konflux/` might not be present.
Applied to files:
scripts/konflux_requirements.shscripts/generate-rpm-lock.sh
📚 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.0)
scripts/konflux_resolve.py
[warning] 414-414: 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] 606-606: 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] 1026-1026: 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] 1231-1241: 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)
[error] 1268-1286: Command coming from incoming request
Context: subprocess.run(
[
uv,
"pip",
"compile",
build_output,
"--generate-hashes",
"--index-url",
"https://pypi.org/simple",
"--python-version",
python_version,
"--no-deps",
"--no-annotate",
"--emit-index-url",
"--universal",
],
capture_output=True,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 1026-1026: 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)
[warning] 203-203: 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] 908-908: 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] 919-919: 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] 926-926: 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] 944-944: 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] 966-966: 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] 1227-1227: 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] 1246-1246: 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)
[warning] 1261-1261: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(build_output)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 1288-1288: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(pypi_wheel_path, "a")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 1299-1299: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(rhoai_wheel_path)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 1309-1309: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(rhoai_wheel_path, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🔇 Additional comments (9)
.konflux/profiles.toml (1)
1-14: LGTM!.konflux/pypi_wheel_only.txt (1)
1-2: LGTM!.konflux/requirements-build.txt (1)
26-26: 🩺 Stability & Availability | ⚡ Quick winVerify
packaging==26.2is a published pin.This lockfile is copied into the image build context, so a missing or mistyped version will fail the build early. Please confirm the resolver emitted an intentional, published release.
.konflux/requirements.hermetic.txt (1)
1-3: LGTM!.konflux/requirements.overrides.txt (1)
1-2: LGTM!.tekton/lightspeed-stack-0-6-pull-request.yaml (1)
49-61: LGTM!.tekton/lightspeed-stack-0-6-push.yaml (1)
50-62: LGTM!.tekton/lightspeed-stack-pull-request.yaml (1)
54-65: LGTM!.tekton/lightspeed-stack-push.yaml (1)
50-61: LGTM!
| BUILDER_BASE_IMAGE=registry.redhat.io/rhai/base-image-cpu-rhel9:3.4.0-1777399554 | ||
| RUNTIME_BASE_IMAGE=registry.redhat.io/rhai/base-image-cpu-rhel9:3.4.0-1777399554 | ||
| DNF_COMMAND=dnf |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '\bDNF_COMMAND\b|\bBUILDER_DNF_COMMAND\b|\bRUNTIME_DNF_COMMAND\b|build-args-file' .konflux .tekton deploy scripts MakefileRepository: lightspeed-core/lightspeed-stack
Length of output: 2064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the build-args config and the pipeline wiring that consumes it.
sed -n '1,80p' .konflux/build-args-konflux.conf
printf '\n---\n'
sed -n '1,220p' .tekton/lightspeed-stack-pull-request.yaml
printf '\n---\n'
sed -n '220,560p' .tekton/lightspeed-stack-pull-request.yaml
printf '\n---\n'
sed -n '1,160p' deploy/lightspeed-stack/ContainerfileRepository: lightspeed-core/lightspeed-stack
Length of output: 28750
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the exact code path that reads build-args-file or build args.
rg -n -C 3 'build-args-file|build args|buildargs|build-args|buildArgs|BUILDER_DNF_COMMAND|RUNTIME_DNF_COMMAND|DNF_COMMAND' .tekton .konflux deploy scripts .Repository: lightspeed-core/lightspeed-stack
Length of output: 17965
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the build-args config and the pipeline wiring that consumes it.
sed -n '1,80p' .konflux/build-args-konflux.conf
printf '\n---\n'
sed -n '1,220p' .tekton/lightspeed-stack-pull-request.yaml
printf '\n---\n'
sed -n '220,560p' .tekton/lightspeed-stack-pull-request.yaml
printf '\n---\n'
sed -n '1,160p' deploy/lightspeed-stack/ContainerfileRepository: lightspeed-core/lightspeed-stack
Length of output: 28750
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the exact code path that reads build-args-file or build args.
rg -n -C 3 'build-args-file|build args|buildargs|build-args|buildArgs|BUILDER_DNF_COMMAND|RUNTIME_DNF_COMMAND|DNF_COMMAND' .tekton .konflux deploy scripts .Repository: lightspeed-core/lightspeed-stack
Length of output: 17965
DNF_COMMAND never reaches the Containerfile args
.konflux/build-args-konflux.conf sets DNF_COMMAND, but deploy/lightspeed-stack/Containerfile still consumes BUILDER_DNF_COMMAND and RUNTIME_DNF_COMMAND. This build-args file has no effect as written; either keep the old arg names here or rename the Containerfile args to match.
🤖 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 @.konflux/build-args-konflux.conf around lines 1 - 3, The DNF command build
arg names are inconsistent between the Konflux build-args config and the
Containerfile, so the value from DNF_COMMAND is never used. Update
.konflux/build-args-konflux.conf and deploy/lightspeed-stack/Containerfile to
use the same arg names, either by restoring BUILDER_DNF_COMMAND and
RUNTIME_DNF_COMMAND in the config or by renaming the Containerfile’s
builder/runtime DNF args to DNF_COMMAND so they match.
| jeepney==0.9.0 \ | ||
| --hash=sha256:959e10a8d7c33bea23aa0320c421afd6303ce469cb68b1fe9a4f00921fe92de1 | ||
| --hash=sha256:61b4b68331c4f1900566fe55e09f6a7ca849ce0a90cd070935b4f6ecefa3b86e |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
failed = False
for path in sorted(Path(".konflux").glob("requirements.hashes*.txt")):
lines = path.read_text().splitlines()
for i, line in enumerate(lines[:-1], start=1):
if line.lstrip().startswith("--hash=") and not line.rstrip().endswith("\\"):
if lines[i].lstrip().startswith("--hash="):
failed = True
print(f"{path}:{i}: missing trailing \\\\ before another --hash line")
if failed:
raise SystemExit(1)
PYRepository: lightspeed-core/lightspeed-stack
Length of output: 443
🏁 Script executed:
#!/bin/bash
sed -n '150,200p' .konflux/requirements.hashes.wheel.txt
echo '---'
sed -n '240,255p' .konflux/requirements.hashes.wheel.txtRepository: lightspeed-core/lightspeed-stack
Length of output: 3985
Fix the broken hash continuations in .konflux/requirements.hashes.wheel.txt.
jeepney==0.9.0, markdown-it-py==4.2.0, and oracledb==3.4.2 each have a non-final --hash= line missing the trailing \, which breaks the multiline requirements syntax used by the hermetic install.
🤖 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 @.konflux/requirements.hashes.wheel.txt around lines 155 - 157, The multiline
requirements entries are broken because several package blocks in
requirements.hashes.wheel.txt have a non-final --hash line without the trailing
continuation backslash. Update the affected entries for jeepney==0.9.0,
markdown-it-py==4.2.0, and oracledb==3.4.2 so every hash line except the last
one in each block ends with a backslash, matching the existing multiline syntax
used by the file.
| DEFAULT_BASE_IMAGE="registry.redhat.io/rhai/base-image-cpu-rhel9:3.3" | ||
| BUILD_ARGS_FILE="build.args" | ||
| INPUT_FILE="rpms.in.yaml" | ||
| OUTPUT_FILE="rpms.lock.yaml" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Point the generator at the committed Konflux inputs and lockfile.
The PR updates .konflux/rpms.lock.yaml, but the script defaults to root-level rpms.lock.yaml and build.args, then falls back to the stale 3.3 base image. Also, passing -O .konflux/rpms.lock.yaml would make rpm-lockfile-prototype write inside /workdir/.konflux/, which is not created in the container. Use host paths for copy-out and container-local basenames for generation.
Proposed fix
-DEFAULT_BASE_IMAGE="registry.redhat.io/rhai/base-image-cpu-rhel9:3.3"
-BUILD_ARGS_FILE="build.args"
-INPUT_FILE="rpms.in.yaml"
-OUTPUT_FILE="rpms.lock.yaml"
+DEFAULT_BASE_IMAGE="registry.redhat.io/rhai/base-image-cpu-rhel9:3.4"
+BUILD_ARGS_FILE=".konflux/build-args-konflux.conf"
+INPUT_FILE=".konflux/rpms.in.yaml"
+OUTPUT_FILE=".konflux/rpms.lock.yaml"
@@
echo "Creating workdir and copying files..."
$CONTAINER_RUNTIME exec "$CONTAINER_NAME" mkdir -p "$WORKDIR"
-$CONTAINER_RUNTIME cp "$INPUT_FILE" "$CONTAINER_NAME:$WORKDIR/$(basename "$INPUT_FILE")"
+CONTAINER_INPUT_FILE="$(basename "$INPUT_FILE")"
+CONTAINER_OUTPUT_FILE="$(basename "$OUTPUT_FILE")"
+$CONTAINER_RUNTIME cp "$INPUT_FILE" "$CONTAINER_NAME:$WORKDIR/$CONTAINER_INPUT_FILE"
$CONTAINER_RUNTIME cp "$REDHAT_REPO_FILE" "$CONTAINER_NAME:$WORKDIR/$(basename "$REDHAT_REPO_FILE")"
@@
/root/.local/bin/rpm-lockfile-prototype \
--image "'"$BASE_IMAGE"'" \
- --outfile "'"$OUTPUT_FILE"'" \
- "'"$(basename "$INPUT_FILE")"'"
+ --outfile "'"$CONTAINER_OUTPUT_FILE"'" \
+ "'"$CONTAINER_INPUT_FILE"'"
'
echo "Copying output file from container..."
-$CONTAINER_RUNTIME cp "$CONTAINER_NAME:$WORKDIR/$OUTPUT_FILE" "$OUTPUT_FILE"
+$CONTAINER_RUNTIME cp "$CONTAINER_NAME:$WORKDIR/$CONTAINER_OUTPUT_FILE" "$OUTPUT_FILE"Also applies to: 21-33, 127-143
🤖 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/generate-rpm-lock.sh` around lines 5 - 8, The RPM lock generator is
still defaulting to the old root-level inputs and lockfile plus the stale base
image, so update generate-rpm-lock.sh to use the committed Konflux files and
current image settings. In the script’s defaults and argument handling, point
INPUT_FILE, BUILD_ARGS_FILE, and OUTPUT_FILE at the .konflux artifacts, and make
the generator work with container-local basenames while copying the generated
lockfile back to the host path instead of writing directly into .konflux inside
the container. Also update the related command construction and path references
in the script’s main flow so rpm-lockfile-prototype uses the new defaults
consistently.
| usage() { | ||
| echo "Usage: $0 -a ACTIVATION_KEY -g ORG_ID [-i BASE_IMAGE] [-f INPUT_FILE] [-O OUTPUT_FILE]" | ||
| echo "" | ||
| echo "Required:" | ||
| echo " -a ACTIVATION_KEY Red Hat activation key for subscription-manager" | ||
| echo " -g ORG_ID Red Hat organization ID for subscription-manager" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep make konflux-rpm-lock usable.
Makefile invokes this script without arguments, but these lines exit unless -a and -g are passed. Support environment-variable fallbacks so the documented make target can run as REDHAT_ACTIVATION_KEY=... REDHAT_ORG_ID=... make konflux-rpm-lock.
Proposed fix
- echo " -a ACTIVATION_KEY Red Hat activation key for subscription-manager"
- echo " -g ORG_ID Red Hat organization ID for subscription-manager"
+ echo " -a ACTIVATION_KEY Red Hat activation key for subscription-manager"
+ echo " -g ORG_ID Red Hat organization ID for subscription-manager"
@@
echo "Environment variables:"
+ echo " REDHAT_ACTIVATION_KEY Red Hat activation key for subscription-manager"
+ echo " REDHAT_ORG_ID Red Hat organization ID for subscription-manager"
echo " REGISTRY_USERNAME Username for registry authentication (skopeo login)"
@@
-ACTIVATION_KEY=""
-ORG_ID=""
+ACTIVATION_KEY="${REDHAT_ACTIVATION_KEY:-}"
+ORG_ID="${REDHAT_ORG_ID:-}"
@@
if [[ -z "$ACTIVATION_KEY" || -z "$ORG_ID" ]]; then
- echo "Error: Both activation key (-a) and organization ID (-g) are required."
+ echo "Error: activation key and organization ID are required via -a/-g or REDHAT_ACTIVATION_KEY/REDHAT_ORG_ID."
usage
fiAlso applies to: 54-72
🤖 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/generate-rpm-lock.sh` around lines 35 - 40, The RPM lock generator
currently requires -a and -g flags, which breaks the make target that runs it
without arguments. Update the argument handling in generate-rpm-lock.sh,
especially the usage/parse logic and the main subscription-manager setup, so
ACTIVATION_KEY and ORG_ID can fall back to REDHAT_ACTIVATION_KEY and
REDHAT_ORG_ID from the environment when flags are omitted. Keep the existing CLI
flags working, but allow make konflux-rpm-lock to succeed by resolving the
values from either args or environment before any validation.
| cleanup() { | ||
| echo "Cleaning up container..." | ||
| $CONTAINER_RUNTIME rm -f "$CONTAINER_NAME" 2>/dev/null || true | ||
| } | ||
| trap cleanup EXIT |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Red Hat subscription-manager unregister command consumer cleanup documentation
💡 Result:
To manage the lifecycle of a Red Hat Subscription Management (RHSM) registration, you should use the subscription-manager command-line tool. The primary commands for unregistering and cleaning up a system are as follows: 1. subscription-manager unregister: This is the standard command to unregister a system from the Subscription Management service [1][2]. It performs two primary actions: it implicitly removes all currently attached subscriptions (returning entitlements to the pool) and removes the system's consumer identity from the management server [2]. 2. subscription-manager clean: This command is used to remove all local subscription and identity data (certificates and configurations) from the local system [3][2]. Importantly, this action does not affect the system's status on the remote management server; it only cleans the local environment [2][4]. This is typically used when local subscription information is corrupted or lost [2]. If you intend to fully decommission a system's registration, it is common practice to perform these steps in sequence: # subscription-manager remove --all # subscription-manager unregister # subscription-manager clean While unregister handles the server-side removal and the removal of attached subscriptions, the clean command ensures no residual local identity data remains [3][2][5]. [3][1][2]
Citations:
- 1: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/10/html/automatically_installing_rhel/changing-a-subscription-service
- 2: https://github.com/candlepin/subscription-manager/blob/main/man/subscription-manager.8
- 3: https://access.redhat.com/articles/433903
- 4: https://access.redhat.com/sites/default/files/attachments/rh_sm_command_cheatsheet_1214_jcs_print.pdf
- 5: https://access.redhat.com/solutions/253273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant script and surrounding context.
file="scripts/generate-rpm-lock.sh"
wc -l "$file"
sed -n '1,170p' "$file"Repository: lightspeed-core/lightspeed-stack
Length of output: 5397
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="scripts/generate-rpm-lock.sh"
wc -l "$file"
sed -n '1,170p' "$file"Repository: lightspeed-core/lightspeed-stack
Length of output: 5397
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate all cleanup traps and RHSM registration calls in the repo.
rg -n --hidden --glob '!**/.git/**' 'subscription-manager register|subscription-manager unregister|trap cleanup EXIT|cleanup\(\)' scriptsRepository: lightspeed-core/lightspeed-stack
Length of output: 371
Unregister the RHSM consumer before deleting the container. rm -f only removes local container state; add a best-effort subscription-manager unregister in cleanup() so the registered consumer is removed from RHSM too.
🤖 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/generate-rpm-lock.sh` around lines 93 - 97, The cleanup flow only
deletes the local container state via cleanup(), so the RHSM consumer remains
registered. Update cleanup() to perform a best-effort subscription-manager
unregister inside the container before calling $CONTAINER_RUNTIME rm -f on
$CONTAINER_NAME, and keep the existing trap cleanup EXIT so this runs on all
exits.
| if [[ -n "$REGISTRY_USERNAME" && -n "$REGISTRY_PASSWORD" ]]; then | ||
| echo "Logging into registry with skopeo..." | ||
| REGISTRY_HOST=$(echo "$BASE_IMAGE" | cut -d'/' -f1) | ||
| $CONTAINER_RUNTIME exec "$CONTAINER_NAME" skopeo login "$REGISTRY_HOST" \ | ||
| --username "$REGISTRY_USERNAME" \ | ||
| --password "$REGISTRY_PASSWORD" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🌐 Web query:
skopeo login --password-stdin documentation
💡 Result:
The --password-stdin flag for the skopeo login command is used to securely provide a registry password via standard input (stdin) rather than typing it interactively or passing it as a command-line argument [1][2][3]. This is particularly useful for scripting and automation to avoid exposing credentials in shell history [1][4]. Usage To use this flag, provide the username via the --username (or -u) flag and pipe the password into the command [1][3]. Example using a file: $ skopeo login -u testuser --password-stdin < testpassword.txt docker.io Login Succeeded! Example using a variable: $ echo
Citations:
- 1: https://github.com/containers/skopeo/blob/main/docs/skopeo-login.1.md
- 2: https://man.archlinux.org/man/skopeo-login.1.en
- 3: http://rpm.pbone.net/manpage_idpl_86253690_numer_1_nazwa_skopeo-login.html
- 4: https://manpages.ubuntu.com/manpages/jammy/man1/skopeo-login.1.html
- 5: https://manpages.debian.org/bookworm/skopeo/skopeo-login.1.en.html
🏁 Script executed:
# Inspect the relevant script context and any existing skopeo login usage.
set -e
git ls-files scripts/generate-rpm-lock.sh
echo '---'
ast-grep outline scripts/generate-rpm-lock.sh --view expanded
echo '---'
sed -n '1,180p' scripts/generate-rpm-lock.shRepository: lightspeed-core/lightspeed-stack
Length of output: 5444
🏁 Script executed:
# Check whether the script already uses stdin-based login elsewhere or has related runtime flags.
set -e
rg -n "skopeo login|password-stdin|CONTAINER_RUNTIME exec" scripts/generate-rpm-lock.shRepository: lightspeed-core/lightspeed-stack
Length of output: 982
Avoid passing the registry password in argv. Pipe it to skopeo login and use --password-stdin with exec -i so the secret does not appear in the process list.
Suggested fix
- $CONTAINER_RUNTIME exec "$CONTAINER_NAME" skopeo login "$REGISTRY_HOST" \
+ printf '%s\n' "$REGISTRY_PASSWORD" | $CONTAINER_RUNTIME exec -i "$CONTAINER_NAME" skopeo login "$REGISTRY_HOST" \
--username "$REGISTRY_USERNAME" \
- --password "$REGISTRY_PASSWORD"
+ --password-stdin📝 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 "$REGISTRY_USERNAME" && -n "$REGISTRY_PASSWORD" ]]; then | |
| echo "Logging into registry with skopeo..." | |
| REGISTRY_HOST=$(echo "$BASE_IMAGE" | cut -d'/' -f1) | |
| $CONTAINER_RUNTIME exec "$CONTAINER_NAME" skopeo login "$REGISTRY_HOST" \ | |
| --username "$REGISTRY_USERNAME" \ | |
| --password "$REGISTRY_PASSWORD" | |
| if [[ -n "$REGISTRY_USERNAME" && -n "$REGISTRY_PASSWORD" ]]; then | |
| echo "Logging into registry with skopeo..." | |
| REGISTRY_HOST=$(echo "$BASE_IMAGE" | cut -d'/' -f1) | |
| printf '%s\n' "$REGISTRY_PASSWORD" | $CONTAINER_RUNTIME exec -i "$CONTAINER_NAME" skopeo login "$REGISTRY_HOST" \ | |
| --username "$REGISTRY_USERNAME" \ | |
| --password-stdin |
🤖 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/generate-rpm-lock.sh` around lines 113 - 118, The registry login in
the generate-rpm-lock flow is passing the password as a command-line argument,
which exposes it in the process list. Update the skopeo login call inside the
REGISTRY_USERNAME/REGISTRY_PASSWORD block to use stdin-based authentication
instead, and make sure the $CONTAINER_RUNTIME exec invocation is interactive so
the password can be piped securely. Keep the change localized to the registry
login logic that uses REGISTRY_HOST and skopeo login.
| for key in ("sdist_hashes", "wheel_hashes"): | ||
| for sha in info.get(key, []): | ||
| if sha: | ||
| hashes.add(sha) | ||
| # wheel_files entries are filenames, not hashes — but the info dict | ||
| # may carry per-file hashes via wheel_hashes already. Nothing extra | ||
| # to extract here. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and inspect the target range plus nearby writer/function definitions.
git ls-files scripts/konflux_resolve.py deploy/lightspeed-stack/Containerfile
echo '--- outline scripts/konflux_resolve.py ---'
ast-grep outline scripts/konflux_resolve.py --view expanded || true
echo '--- target slice scripts/konflux_resolve.py ---'
sed -n '840,930p' scripts/konflux_resolve.py | cat -n
echo '--- Containerfile slice ---'
sed -n '60,85p' deploy/lightspeed-stack/Containerfile | cat -nRepository: lightspeed-core/lightspeed-stack
Length of output: 8466
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all references to the hash writer and the source bucket to understand the data flow.
rg -n "write_hashed_requirements|requirements\.hashes\.source|pypi_sdist|wheel_hashes|sdist_hashes|wheel_files" scripts/konflux_resolve.py deploy -SRepository: lightspeed-core/lightspeed-stack
Length of output: 3784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and inspect the target range plus nearby writer/function definitions.
git ls-files scripts/konflux_resolve.py deploy/lightspeed-stack/Containerfile
echo '--- outline scripts/konflux_resolve.py ---'
ast-grep outline scripts/konflux_resolve.py --view expanded || true
echo '--- target slice scripts/konflux_resolve.py ---'
sed -n '840,930p' scripts/konflux_resolve.py | cat -n
echo '--- Containerfile slice ---'
sed -n '60,85p' deploy/lightspeed-stack/Containerfile | cat -nRepository: lightspeed-core/lightspeed-stack
Length of output: 8466
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all references to the hash writer and the source bucket to understand the data flow.
rg -n "write_hashed_requirements|requirements\.hashes\.source|pypi_sdist|wheel_hashes|sdist_hashes|wheel_files" scripts/konflux_resolve.py deploy -SRepository: lightspeed-core/lightspeed-stack
Length of output: 3784
Keep requirements.hashes.source*.txt sdist-only.
write_hashed_requirements() emits both sdist_hashes and wheel_hashes for buckets["pypi_sdist"], so requirements.hashes.source*.txt can still resolve to a wheel when installed in deploy/lightspeed-stack/Containerfile:73. Filter hashes by artifact kind, or add an explicit source-only mode to the writer.
🤖 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 893 - 899, The hashed requirements
generated by write_hashed_requirements() are currently mixing sdist_hashes and
wheel_hashes for the pypi_sdist bucket, which lets
requirements.hashes.source*.txt resolve wheels instead of being source-only.
Update the hash collection logic in scripts/konflux_resolve.py so source
requirements only include sdist artifact hashes, either by filtering on artifact
kind when building the hashes set or by adding an explicit source-only mode in
write_hashed_requirements(). Ensure the behavior applies to the source
requirements emitted for requirements.hashes.source*.txt.
| sorted_hashes = sorted(hashes) | ||
| if sorted_hashes: | ||
| lines.append(f"{name}=={version} \\\n") | ||
| hash_lines = [f" --hash=sha256:{h}" for h in sorted_hashes] | ||
| lines.append(" \\\n".join(hash_lines) + "\n") | ||
| else: | ||
| lines.append(f"{name}=={version}\n") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail generation when a hashed output has no hashes.
requirements.hashes.*.txt is a contract artifact for the hermetic pipeline, but Lines 901-907 quietly fall back to plain name==version when hash lookup failed upstream. That turns a transient metadata problem into an invalid lockfile instead of failing regeneration at the source.
Proposed fix
- sorted_hashes = sorted(hashes)
- if sorted_hashes:
- lines.append(f"{name}=={version} \\\n")
- hash_lines = [f" --hash=sha256:{h}" for h in sorted_hashes]
- lines.append(" \\\n".join(hash_lines) + "\n")
- else:
- lines.append(f"{name}=={version}\n")
+ sorted_hashes = sorted(hashes)
+ if not sorted_hashes:
+ raise RuntimeError(
+ f"No hashes collected for {name}=={version} while writing {output_path}"
+ )
+ lines.append(f"{name}=={version} \\\n")
+ hash_lines = [f" --hash=sha256:{h}" for h in sorted_hashes]
+ lines.append(" \\\n".join(hash_lines) + "\n")📝 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.
| sorted_hashes = sorted(hashes) | |
| if sorted_hashes: | |
| lines.append(f"{name}=={version} \\\n") | |
| hash_lines = [f" --hash=sha256:{h}" for h in sorted_hashes] | |
| lines.append(" \\\n".join(hash_lines) + "\n") | |
| else: | |
| lines.append(f"{name}=={version}\n") | |
| sorted_hashes = sorted(hashes) | |
| if not sorted_hashes: | |
| raise RuntimeError( | |
| f"No hashes collected for {name}=={version} while writing {output_path}" | |
| ) | |
| lines.append(f"{name}=={version} \\\n") | |
| hash_lines = [f" --hash=sha256:{h}" for h in sorted_hashes] | |
| lines.append(" \\\n".join(hash_lines) + "\n") |
🤖 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 901 - 907, The hash rendering in the
requirements output should not silently fall back to plain name==version when a
hashed entry has no hashes. Update the logic in the hashed artifact generation
path around sorted_hashes handling so that requirements.hashes.*.txt generation
fails fast when hashes is empty, instead of appending an unhashed requirement;
use the surrounding hash resolution flow in scripts/konflux_resolve.py to raise
an error at source for this contract violation.
| def uv_resolve( | ||
| python_version: str, | ||
| rhoai_index_url: str, | ||
| suffix: str, | ||
| ) -> dict[str, dict[str, Any]]: | ||
| """Run ``uv pip compile --index-strategy prefer-index`` to resolve deps. | ||
|
|
||
| Returns ``{normalized_name: {"version": str, "index": str}}`` | ||
| where *index* is the URL of the index the package was resolved from. | ||
| """ | ||
| overrides_file = os.path.join( | ||
| KONFLUX_DIR, | ||
| f"requirements.overrides{suffix}.txt" if suffix else "requirements.overrides.txt", | ||
| ) | ||
| uv = UV_BINARY if os.path.isfile(UV_BINARY) else "uv" | ||
| cmd = [ | ||
| uv, | ||
| "pip", | ||
| "compile", | ||
| "pyproject.toml", | ||
| "--python-platform", | ||
| "x86_64-manylinux_2_28", | ||
| "--python-version", | ||
| python_version, | ||
| "--refresh", | ||
| "--index", | ||
| rhoai_index_url, | ||
| "--default-index", | ||
| "https://pypi.org/simple/", | ||
| "--index-strategy", | ||
| "prefer-index", | ||
| "--emit-index-annotation", | ||
| "--no-sources", | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and nearby symbols first.
ast-grep outline scripts/konflux_resolve.py --view expanded || true
# Find where config["platforms"] and python-platform are used.
rg -n 'python-platform|platforms|aarch64|x86_64-manylinux_2_28|uv_resolve\(' scripts/konflux_resolve.py .tekton deploy -g '!**/.git/**' || true
# Show the relevant section with line numbers.
sed -n '960,1060p' scripts/konflux_resolve.pyRepository: lightspeed-core/lightspeed-stack
Length of output: 12534
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect surrounding config definition / consumers.
rg -n '"platforms"|platforms:' . -g '!**/.git/**' || true
# Show the Tekton/Containerfile context mentioned in the comment if present.
rg -n 'arch:|aarch64|x86_64' .tekton deploy -g '!**/.git/**' || trueRepository: lightspeed-core/lightspeed-stack
Length of output: 2892
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the main resolution flow and how uv_resolve feeds later steps.
sed -n '1128,1215p' scripts/konflux_resolve.py
# Read the README platform description around the referenced section.
sed -n '930,955p' README.md
# Show the Tekton platform parameter blocks around the cited lines.
sed -n '20,70p' .tekton/lightspeed-stack-push.yamlRepository: lightspeed-core/lightspeed-stack
Length of output: 8377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1148,1210p' scripts/konflux_resolve.pyRepository: lightspeed-core/lightspeed-stack
Length of output: 2574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1148,1210p' scripts/konflux_resolve.pyRepository: lightspeed-core/lightspeed-stack
Length of output: 2574
Resolve deps for every configured platform. uv_resolve() still hardcodes x86_64-manylinux_2_28, while the rest of the pipeline builds for both x86_64 and aarch64. That makes the package set come from a single x86_64 solve, so platform-specific dependencies can be dropped before the per-platform hash fetch runs. Drive this from config["platforms"] and merge the per-platform solves instead of pinning x86_64 here.
🤖 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 989 - 1022, uv_resolve() is still
hardcoded to a single x86_64 platform, which means dependency resolution ignores
the configured multi-platform build matrix. Update uv_resolve() to iterate over
config["platforms"], run the uv pip compile solve per platform instead of using
the fixed x86_64-manylinux_2_28 target, and merge the results so
platform-specific packages are preserved before hash fetching.
| wheel_package_names = ( | ||
| list(buckets["rhoai_wheel"].keys()) | ||
| + list(buckets["pypi_wheel"].keys()) | ||
| + [normalize_name(p) for p in bootstrap_packages] | ||
| ) | ||
| for tekton_file in tekton_files: | ||
| if os.path.exists(tekton_file): | ||
| patch_tekton_packages(tekton_file, wheel_package_names) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep Tekton binary.packages in sync with requirements.hashes.wheel.pypi*.txt.
Step 8 appends build-dependency wheels such as flit-core, hatchling, and poetry-core into .konflux/requirements.hashes.wheel.pypi.txt, but Step 9 only serializes rhoai_wheel + pypi_wheel + bootstrap_packages into the Tekton binary.packages field. That leaves the prefetched wheel set narrower than what Containerfile Line 73 now installs from requirements.hashes.wheel.pypi.txt. Include build_pkg_names in wheel_package_names before patching the Tekton templates.
🤖 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 1318 - 1325, The Tekton
binary.packages list is missing build-dependency wheels that are already added
to requirements.hashes.wheel.pypi*.txt, so the prefetched set can drift from
what the Containerfile installs. Update the wheel_package_names assembly in
konflux_resolve.py to include build_pkg_names alongside rhoai_wheel, pypi_wheel,
and bootstrap_packages, then let patch_tekton_packages use the expanded set when
rewriting the Tekton templates.
3c60333 to
9219429
Compare
syedriko
force-pushed
the
syedriko-rhoai-3.4
branch
from
9219429 to
3d0a889
Compare
1 participant
Description
Update RHOAI index to 3.4
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
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Chores