Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/build-notebooks-TEMPLATE.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,10 @@ jobs:
# region Pytest image tests

# https://github.com/astral-sh/setup-uv
- name: Install the latest version of uv
- name: Install uv
uses: astral-sh/setup-uv@v7
with:
version: "latest"
version-file: uv.toml
python-version: "3.14"
enable-cache: true
cache-dependency-glob: "uv.lock"
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/code-quality.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ jobs:
- uses: actions/checkout@v6

# https://github.com/astral-sh/setup-uv
- name: Install the latest version of uv
- name: Install uv
uses: astral-sh/setup-uv@v7
with:
version: "latest"
version-file: uv.toml
python-version: "3.14"
enable-cache: true
cache-dependency-glob: "uv.lock"
Expand All @@ -44,10 +44,10 @@ jobs:
- uses: actions/checkout@v6

# https://github.com/astral-sh/setup-uv
- name: Install the latest version of uv
- name: Install uv
uses: astral-sh/setup-uv@v7
with:
version: "latest"
version-file: uv.toml
python-version: "3.14"
enable-cache: true
cache-dependency-glob: "uv.lock"
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ jobs:
- uses: actions/checkout@v6

# https://github.com/astral-sh/setup-uv
- name: Install the latest version of uv
- name: Install uv
uses: astral-sh/setup-uv@v7
with:
version: "latest"
version-file: uv.toml
python-version: "3.14"
enable-cache: true
cache-dependency-glob: "uv.lock"
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/piplock-renewal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ jobs:
python-version: '3.12'

- name: Install uv
run: pip install "uv==0.9.27"
uses: astral-sh/setup-uv@v7
with:
version-file: uv.toml

- name: Run make refresh-lock-files
run: |
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/security.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ jobs:
security-events: write
steps:

- name: Checkout code
uses: actions/checkout@v6

# https://github.com/astral-sh/setup-uv
- name: Install the latest version of uv
- name: Install uv
uses: astral-sh/setup-uv@v7
with:
version: "latest"
version-file: uv.toml
activate-environment: false
ignore-empty-workdir: true
enable-cache: false

- name: Checkout code
uses: actions/checkout@v6

# Trivy does not support pylock.toml https://github.com/aquasecurity/trivy/discussions/9408
- run: find . -name pyproject.toml -execdir uv lock \;

Expand Down
16 changes: 11 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@
# https://github.com/pre-commit/pre-commit-hooks?tab=readme-ov-file#hooks-available
repos:
# https://docs.astral.sh/uv/guides/integration/pre-commit/
- repo: https://github.com/astral-sh/uv-pre-commit
rev: 0.9.29
# Using a local hook instead of uv-pre-commit so it goes through ./uv,
# which handles version mismatches without requiring the exact system uv.
- repo: local
hooks:
- id: uv-lock
name: uv-lock
entry: ./uv lock --locked
language: system
files: '(^uv\.lock$|^pyproject\.toml$|^uv\.toml$)'
pass_filenames: false
# https://github.com/astral-sh/ruff-pre-commit
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.15.0
rev: v0.15.4
hooks:
- id: ruff
- id: ruff-check
types_or: [python, pyi]
args: [--fix]
files: 'ci/.*|tests/.*'
Expand All @@ -25,7 +31,7 @@ repos:
- id: pyright
name: Run Pyright on all files
# entry: /bin/bash -c 'find. -name "*.py" | xargs pyright --pythonversion 3.12'
entry: uv run pyright --pythonversion 3.14
entry: ./uv run pyright --pythonversion 3.14
pass_filenames: true
types_or: [python, pyi]
language: system
Expand Down
42 changes: 37 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,49 @@ Note: To ensure the GitHub Action runs successfully, users must add a `GH_ACCESS

#### Prepare Python + uv + pytest env

This project pins its uv version in `uv.toml` (`required-version`).
Use the `./uv` wrapper script at the repo root — it reads the pinned
version and runs it via `uvx`, so your system uv version doesn't matter:

```shell
# Linux
sudo dnf install python3.14
pip install --user uv
# MacOS
# macOS
brew install python@3.14 uv

uv venv --python $(which python3.14)
uv sync --locked
./uv venv --python $(which python3.14)
./uv sync --locked
```

<details>
<summary>Alternatives to <code>./uv</code></summary>

The `./uv` wrapper is the recommended way, but you can also
(replace `0.10.6` below with the version from `uv.toml`):

- **Use `uvx` directly** with an explicit version:
```shell
uvx uv@0.10.6 sync --locked
```
- **Use `uv tool run`** (equivalent, longer form):
```shell
uv tool run uv@0.10.6 sync --locked
```
- **Install the exact version** so `uv` works directly:
```shell
# Standalone installer (any OS)
curl -LsSf https://astral.sh/uv/0.10.6/install.sh | sh
# Or with pip
pip install uv==0.10.6
```

If your system uv matches the pinned version, you can use `uv` directly —
`required-version` in `uv.toml` will let it through. If it doesn't match,
uv exits with a clear error telling you which version is required.

</details>

#### Running Python selftests in Pytest
By completing configuration in previous section, you are able to run any tests that don't need to start a container using following command:

Expand Down Expand Up @@ -106,15 +138,15 @@ sudo dnf install podman
systemctl --user start podman.service
systemctl --user status podman.service
systemctl --user status podman.socket
DOCKER_HOST=unix:///run/user/$UID/podman/podman.sock uv run pytest tests/containers -m 'not openshift and not cuda and not rocm' --image quay.io/opendatahub/workbench-images@sha256:e98d19df346e7abb1fa3053f6d41f0d1fa9bab39e49b4cb90b510ca33452c2e4
DOCKER_HOST=unix:///run/user/$UID/podman/podman.sock ./uv run pytest tests/containers -m 'not openshift and not cuda and not rocm' --image quay.io/opendatahub/workbench-images@sha256:e98d19df346e7abb1fa3053f6d41f0d1fa9bab39e49b4cb90b510ca33452c2e4

# Mac OS
brew install podman
podman machine init
podman machine set --rootful=false
sudo podman-mac-helper install
podman machine start
uv run pytest tests/containers -m 'not openshift' --image quay.io/opendatahub/workbench-images@sha256:e98d19df346e7abb1fa3053f6d41f0d1fa9bab39e49b4cb90b510ca33452c2e4
./uv run pytest tests/containers -m 'not openshift' --image quay.io/opendatahub/workbench-images@sha256:e98d19df346e7abb1fa3053f6d41f0d1fa9bab39e49b4cb90b510ca33452c2e4
```

When using lima on macOS, it might be useful to give yourself access to rootful podman socket
Expand Down
9 changes: 6 additions & 3 deletions ci/generate_code.sh
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#!/usr/bin/env bash
set -Eeuxo pipefail

uv --version || pip install "uv==0.9.6"
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)"

uv run scripts/dockerfile_fragments.py
uv run manifests/tools/generate_kustomization.py
uv --version || pip install "uv==0.10.6"

"${REPO_ROOT}/uv" run scripts/dockerfile_fragments.py
"${REPO_ROOT}/uv" run manifests/tools/generate_kustomization.py
bash scripts/pylocks_generator.sh
Comment thread
coderabbitai[bot] marked this conversation as resolved.
20 changes: 14 additions & 6 deletions scripts/pylocks_generator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ MAIN_DIRS=("jupyter" "runtimes" "rstudio" "codeserver")
# CVE constraints file - applied to all lock file generations
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
ROOT_DIR="$(dirname "$SCRIPT_DIR")"
UV="${ROOT_DIR}/uv"
CVE_CONSTRAINTS_FILE="$ROOT_DIR/dependencies/cve-constraints.txt"

# ----------------------------
Expand Down Expand Up @@ -89,13 +90,18 @@ read_conf_value() {
# ----------------------------
# PRE-FLIGHT CHECK
# ----------------------------
if [[ ! -x "$UV" ]]; then
error "Expected uv wrapper at '$UV' but it is missing or not executable."
exit 1
fi

if ! command -v uv &>/dev/null; then
error "uv command not found. Please install uv: https://github.com/astral-sh/uv"
exit 1
fi

UV_MIN_VERSION="0.4.0"
UV_VERSION=$(uv --version 2>/dev/null | awk '{print $2}' || echo "0.0.0")
UV_VERSION=$("$UV" --version 2>/dev/null | awk '{print $2}' || echo "0.0.0")

version_ge() {
[ "$(printf '%s\n' "$2" "$1" | sort -V | head -n1)" = "$2" ]
Expand Down Expand Up @@ -273,7 +279,9 @@ for TARGET_DIR in "${TARGET_DIRS[@]}"; do
echo "➡️ Generating $(uppercase "$flavor") lock file..."
fi

# The behavior has changed in uv 0.9.17 (https://github.com/astral-sh/uv/pull/16956)
# Tag filtering was added in uv 0.9.16 (https://github.com/astral-sh/uv/pull/16956)
# but bypassed in --universal mode. uv 0.10.5 (https://github.com/astral-sh/uv/pull/18081)
# now filters wheels by requires-python and marker disjointness even in --universal mode.
# Documentation at https://docs.astral.sh/uv/reference/cli/#uv-pip-compile--python-platform says that
# `--python-platform linux` is alias for `x86_64-unknown-linux-gnu`; we cannot use this to get a multiarch pylock
# Let's use --universal temporarily, and in the future we can switch to using uv.lock
Expand All @@ -285,17 +293,17 @@ for TARGET_DIR in "${TARGET_DIRS[@]}"; do
# Build constraints flag if CVE constraints file exists
# Use relative path to avoid absolute paths in pylock.toml headers
# (which would differ between CI and local environments)
local constraints_flag=""
local -a constraints_flag=()
if [[ -f "$CVE_CONSTRAINTS_FILE" ]]; then
local relative_constraints
# Use Python for cross-platform relative path computation (realpath --relative-to is GNU-only)
relative_constraints=$(python3 -c "import os; print(os.path.relpath('$CVE_CONSTRAINTS_FILE', '$PWD'))")
constraints_flag="--constraints=$relative_constraints"
constraints_flag=(--constraints "$relative_constraints")
Comment on lines 299 to +301
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Make relative-path computation safe for apostrophes in paths.

Line 300 embeds shell-expanded paths inside Python single-quoted literals; this breaks if either path contains '. Pass paths as argv instead.

💡 Proposed fix
-      relative_constraints=$(python3 -c "import os; print(os.path.relpath('$CVE_CONSTRAINTS_FILE', '$PWD'))")
+      relative_constraints=$(python3 -c 'import os,sys; print(os.path.relpath(sys.argv[1], sys.argv[2]))' \
+        "$CVE_CONSTRAINTS_FILE" "$PWD")
📝 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.

Suggested change
# Use Python for cross-platform relative path computation (realpath --relative-to is GNU-only)
relative_constraints=$(python3 -c "import os; print(os.path.relpath('$CVE_CONSTRAINTS_FILE', '$PWD'))")
constraints_flag="--constraints=$relative_constraints"
constraints_flag=(--constraints "$relative_constraints")
# Use Python for cross-platform relative path computation (realpath --relative-to is GNU-only)
relative_constraints=$(python3 -c 'import os,sys; print(os.path.relpath(sys.argv[1], sys.argv[2]))' \
"$CVE_CONSTRAINTS_FILE" "$PWD")
constraints_flag=(--constraints "$relative_constraints")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/pylocks_generator.sh` around lines 299 - 301, The current python3 -c
call embeds shell-expanded $CVE_CONSTRAINTS_FILE and $PWD inside single quotes
which breaks on paths containing apostrophes; change the invocation so the shell
passes those paths as argv parameters and the inline Python reads sys.argv[1]
and sys.argv[2] and computes os.path.relpath on them, then assign that output to
relative_constraints and keep constraints_flag=(--constraints
"$relative_constraints"); ensure the shell side quotes the two positional
arguments when calling python3 so spaces and special chars are preserved.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai recheck, fixed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current state of the relative_constraints line in scripts/pylocks_generator.sh
cat scripts/pylocks_generator.sh | sed -n '295,305p'

Length of output: 497


@jiridanek, I can see that the array handling for constraints_flag has been fixed (great work on that!), but the apostrophe-in-path vulnerability on line 300 is still present:

Current code (line 300):

relative_constraints=$(python3 -c "import os; print(os.path.relpath('$CVE_CONSTRAINTS_FILE', '$PWD'))")

This still embeds $CVE_CONSTRAINTS_FILE and $PWD inside Python single-quoted strings, which will break if either path contains an apostrophe.

Required fix:

relative_constraints=$(python3 -c 'import os,sys; print(os.path.relpath(sys.argv[1], sys.argv[2]))' \
  "$CVE_CONSTRAINTS_FILE" "$PWD")

This passes the paths as command-line arguments to Python, avoiding string interpolation issues entirely.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai this is not worth fixing, rather, rewrite whole script to python

create github issue to do that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue to rewrite pylocks_generator.sh to Python

gh issue create \
  --repo opendatahub-io/notebooks \
  --title "Rewrite scripts/pylocks_generator.sh to Python" \
  --body "## Context

The current \`scripts/pylocks_generator.sh\` bash script is complex and has some edge cases that are difficult to handle correctly in bash (e.g., paths containing apostrophes).

Rewriting it in Python would:
- Make the code more maintainable
- Avoid shell quoting and escaping pitfalls
- Leverage Python's built-in path handling and safer subprocess management
- Improve readability and testability

## References

- Related PR: `#3034`
- Discussion: https://github.com/opendatahub-io/notebooks/pull/3034#discussion_r2864644935
- Requested by: `@jiridanek`

## Scope

Rewrite \`scripts/pylocks_generator.sh\` as a Python script while preserving all existing functionality." \
  --assignee jiridanek

Length of output: 55


@jiridanek, understood! I've created issue #3036 to track rewriting scripts/pylocks_generator.sh to Python. The issue has been assigned to you and includes links back to this PR and discussion.

The issue is available at: #3036

fi

set +e
# shellcheck disable=SC2086
uv pip compile pyproject.toml \
"$UV" pip compile pyproject.toml \
--output-file "$output" \
--format pylock.toml \
--generate-hashes \
Expand All @@ -309,7 +317,7 @@ for TARGET_DIR in "${TARGET_DIRS[@]}"; do
--no-emit-package odh-notebooks-meta-runtime-datascience-deps \
--no-emit-package odh-notebooks-meta-workbench-datascience-deps \
$UPGRADE_FLAG \
$constraints_flag \
"${constraints_flag[@]}" \
$index
local status=$?
set -e
Expand Down
40 changes: 40 additions & 0 deletions uv
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/usr/bin/env -S bash --norc --noprofile
# ./uv — run the project-pinned version of uv.
#
# Reads required-version from uv.toml and delegates via `uv tool run`.
# This avoids version mismatch errors when your system uv (e.g. Homebrew)
# differs from the version pinned for this project.
#
# Usage:
# ./uv sync
# ./uv run pytest
# ./uv pip compile ...
#
# The pinned version is cached by uvx after the first run.
# Parsing uses bash =~ instead of forking sed to avoid a subprocess.
#
# Bash with built-in regex is ~3x faster than Python for this task (hyperfine,
# 50 runs): bash+builtin 18.7ms, bash+sed 25.0ms, python 55.4ms.
set -Eeuo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

while IFS= read -r line; do
if [[ "$line" =~ ^required-version\ *=\ *\"==([^\"]*)\" ]]; then
version="${BASH_REMATCH[1]}"
break
fi
done < "${SCRIPT_DIR}/uv.toml"

if [[ -z "${version:-}" ]]; then
echo "error: could not read required-version from ${SCRIPT_DIR}/uv.toml" >&2
exit 1
fi

# Fast path: use the system uv directly if it already matches the pinned version
if current=$(uv --version 2>/dev/null) && [[ "$current" == "uv $version" || "$current" == "uv $version "* ]]; then
exec uv "$@"
fi

# Slow path: run the pinned version via uvx (downloaded and cached on first use)
exec uv tool run "uv@${version}" "$@"
1 change: 1 addition & 0 deletions uv.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
required-version = "==0.10.6"
Loading