-
Notifications
You must be signed in to change notification settings - Fork 29
[DRAFT] fix(ci): resolve pre-existing CI failures on rhoai-3.3 #2184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rhoai-3.3
Are you sure you want to change the base?
Changes from all commits
fc5985c
e0c5f5f
6f2ea38
a27390d
9bf5093
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| --- | ||
| name: 'Install APT packages' | ||
| description: 'Fast apt-get install with slow repos removed' | ||
| inputs: | ||
| packages: | ||
| description: 'Space-separated list of packages to install' | ||
| required: true | ||
| update: | ||
| description: 'Run apt-get update before installing' | ||
| required: false | ||
| default: 'true' | ||
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: Configure apt for CI | ||
| shell: bash | ||
| run: | | ||
| # These are idempotent and fast — safe to run every time | ||
| sudo rm -f /var/lib/man-db/auto-update | ||
|
|
||
| # the Microsoft repo's kubelet does not provide /etc/systemd/system/kubelet.service.d/10-kubeadm.conf | ||
| # [Service] | ||
| # EnvironmentFile=-/var/lib/kubelet/kubeadm-flags.env | ||
| # ExecStart=/usr/bin/kubelet $KUBELET_KUBEADM_ARGS | ||
| # and otherwise fetching from microsoft-prod wastes time | ||
| sudo rm -f /etc/apt/sources.list.d/microsoft-prod.list | ||
| sudo rm -f /etc/apt/sources.list.d/azure-cli.list | ||
|
|
||
| cat <<'EOF' | sudo tee /etc/dpkg/dpkg.cfg.d/01_nodoc > /dev/null | ||
| path-exclude /usr/share/doc/* | ||
| path-exclude /usr/share/man/* | ||
| path-exclude /usr/share/locale/* | ||
| path-include /usr/share/locale/en* | ||
| EOF | ||
|
|
||
| cat <<'EOF' | sudo tee /etc/apt/apt.conf.d/99-ci-optimizations > /dev/null | ||
| Acquire::Queue-Mode "access"; | ||
| Acquire::Retries "3"; | ||
| APT::Install-Recommends "false"; | ||
| APT::Install-Suggests "false"; | ||
| APT::Get::Assume-Yes "true"; | ||
| APT::Get::Quiet "true"; | ||
| APT::Get::Show-Upgraded "false"; | ||
| APT::AutoRemove::SuggestsImportant "false"; | ||
| DPkg::Options:: "--force-unsafe-io"; | ||
| EOF | ||
|
|
||
| - name: Update package index | ||
| if: ${{ inputs.update == 'true' }} | ||
| shell: bash | ||
| run: | | ||
| sudo apt-get update | ||
|
|
||
| - name: Install packages | ||
| shell: bash | ||
| run: sudo apt-get install ${{ inputs.packages }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| --- | ||
| name: 'Playwright Browser Tests' | ||
| description: | | ||
| Run Playwright browser tests for workbench images. | ||
|
|
||
| Features: | ||
| - Runs Playwright tests inside the official Microsoft Playwright container | ||
| - Uses Podman to run the test container with proper isolation | ||
| - Supports testcontainers for container orchestration | ||
| - Uploads test reports as artifacts on pull requests | ||
|
|
||
| Requirements: | ||
| - Podman must be installed and running | ||
| - The workbench image must be available in the local Podman storage | ||
| - tests/browser directory must contain the Playwright test configuration | ||
|
|
||
| inputs: | ||
| test-target: | ||
| description: 'The workbench image to test (full image reference).' | ||
| required: true | ||
| podman-socket: | ||
| description: 'Path to Podman socket' | ||
| required: false | ||
| default: '/var/run/podman/podman.sock' | ||
| playwright-version: | ||
| description: 'Version of the Playwright container to use (e.g., v1.58.1-noble). If not provided, extracted from package.json5' | ||
| required: false | ||
| default: '' | ||
| working-directory: | ||
| description: 'Directory containing the Playwright tests' | ||
| required: false | ||
| default: 'tests/browser' | ||
| upload-report: | ||
| description: 'Whether to upload the Playwright report as an artifact' | ||
| required: false | ||
| default: 'false' | ||
| artifact-name: | ||
| description: 'Name for the uploaded artifact (required if upload-report is true)' | ||
| required: false | ||
| default: 'playwright-report' | ||
| artifact-retention-days: | ||
| description: 'Number of days to retain the artifact' | ||
| required: false | ||
| default: '30' | ||
| grep-pattern: | ||
| description: 'Grep pattern passed to --grep to filter tests (e.g. "@codeserver", "@smoke"). Empty string runs all tests.' | ||
| required: false | ||
| default: '' | ||
|
|
||
| outputs: | ||
| outcome: | ||
| description: 'The outcome of the Playwright tests (success or failure)' | ||
| value: ${{ steps.playwright.outcome }} | ||
|
|
||
| runs: | ||
| using: 'composite' | ||
| steps: | ||
| - name: Determine Playwright version | ||
| id: playwright-version | ||
| shell: bash | ||
| run: | | ||
| if [[ -n "${INPUT_PLAYWRIGHT_VERSION}" ]]; then | ||
| echo "version=${INPUT_PLAYWRIGHT_VERSION}" >> "$GITHUB_OUTPUT" | ||
| else | ||
| # Extract version from package.json5 (single source of truth) | ||
| # package.json5 has: "@playwright/test": "=1.59.1" | ||
| # Container tag format is: v1.59.1-noble | ||
| PKG_VERSION=$(grep -oP '"@playwright/test":\s*"=?\K[0-9.]+' "${WORKING_DIRECTORY}/package.json5") | ||
| if [[ -z "$PKG_VERSION" ]]; then | ||
| echo "::error::Failed to extract Playwright version from package.json5" | ||
| exit 1 | ||
| fi | ||
| echo "version=v${PKG_VERSION}-noble" >> "$GITHUB_OUTPUT" | ||
| echo "Extracted Playwright version: v${PKG_VERSION}-noble" | ||
| fi | ||
| env: | ||
| INPUT_PLAYWRIGHT_VERSION: ${{ inputs.playwright-version }} | ||
| WORKING_DIRECTORY: ${{ inputs.working-directory }} | ||
|
|
||
| - name: Run Playwright tests | ||
| id: playwright | ||
| shell: bash | ||
| # --ipc=host because Microsoft says so in Playwright docs | ||
| # --net=host because testcontainers connects to the Reaper container's exposed port | ||
| # we need to pass through the relevant environment variables | ||
| # DEBUG configures Node.js debuggers, sets different verbosity as needed | ||
| # CI=true is set on every CI nowadays | ||
| # PODMAN_SOCK should be mounted to /var/run/docker.sock, other likely mounting locations may not exist (mkdir -p) | ||
| # TEST_TARGET is the workbench image the test will run | ||
| # --volume(s) let us access docker socket and not clobber host's node_modules | ||
| run: | | ||
| podman run \ | ||
| --interactive --rm \ | ||
| --ipc=host \ | ||
| --net=host \ | ||
| --env "CI=true" \ | ||
| --env "NPM_CONFIG_fund=false" \ | ||
| --env "DEBUG=testcontainers:*" \ | ||
| --env "PODMAN_SOCK=/var/run/docker.sock" \ | ||
| --env "TEST_TARGET" \ | ||
| --volume "${PODMAN_SOCKET}":/var/run/docker.sock \ | ||
| --volume "${PWD}":/mnt \ | ||
| --volume /mnt/node_modules \ | ||
| "mcr.microsoft.com/playwright:${PLAYWRIGHT_VERSION}" \ | ||
| /bin/bash <<EOF | ||
| set -Eeuxo pipefail | ||
| cd /mnt | ||
| npm install -g pnpm@10.33.0 && pnpm install --frozen-lockfile | ||
| pnpm exec playwright test ${GREP_PATTERN:+--grep "$GREP_PATTERN"} | ||
| exit 0 | ||
| EOF | ||
| working-directory: ${{ inputs.working-directory }} | ||
| env: | ||
| # Only set TEST_TARGET if provided, otherwise playwright.config.ts uses its default | ||
| TEST_TARGET: ${{ inputs.test-target || '' }} | ||
| PODMAN_SOCKET: ${{ inputs.podman-socket }} | ||
| PLAYWRIGHT_VERSION: ${{ steps.playwright-version.outputs.version }} | ||
| GREP_PATTERN: ${{ inputs.grep-pattern }} | ||
|
|
||
| - name: Upload Playwright report | ||
| if: ${{ !cancelled() && inputs.upload-report == 'true' }} | ||
| uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 | ||
| with: | ||
| name: ${{ inputs.artifact-name }} | ||
| path: ${{ inputs.working-directory }}/playwright-report/ | ||
| retention-days: ${{ inputs.artifact-retention-days }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| --- | ||
| name: 'Setup uv and Python' | ||
| description: 'Install uv (version from pyproject.toml) and Python (version from .python-version) with caching' | ||
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: Read Python version from .python-version | ||
| id: python-version | ||
| shell: python3 {0} | ||
| run: | | ||
| import os, pathlib | ||
| version = pathlib.Path(".python-version").read_text().strip().split("\n")[0] | ||
| with open(os.environ["GITHUB_OUTPUT"], "a") as f: | ||
| print(f"version={version}", file=f) | ||
|
|
||
| # https://github.com/astral-sh/setup-uv | ||
| - name: Install uv and Python | ||
| uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 | ||
| with: | ||
| version-file: pyproject.toml | ||
| python-version: ${{ steps.python-version.outputs.version }} | ||
| enable-cache: true | ||
| cache-dependency-glob: "uv.lock" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,10 +2,16 @@ | |||||||||||||||||
| # 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.18 | ||||||||||||||||||
| # Using a local hook instead of uv-pre-commit so it uses the system uv | ||||||||||||||||||
| # installed by setup-uv, avoiding version mismatch with required-version. | ||||||||||||||||||
| - 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 | ||||||||||||||||||
|
Comment on lines
+11
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The Suggested adjustment - id: uv-lock
name: uv-lock
- entry: uv lock --locked
+ entry: bash -c 'set -euo pipefail; for f in "$@"; do d="$(dirname "$f")"; (cd "$d" && uv lock --locked); done' --
language: system
- files: '(^uv\.lock$|^pyproject\.toml$|^uv\.toml$)'
- pass_filenames: false
+ files: '(^|.*/)(uv\.lock|pyproject\.toml|uv\.toml)$'
+ pass_filenames: true📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| # https://github.com/astral-sh/ruff-pre-commit | ||||||||||||||||||
| - repo: https://github.com/astral-sh/ruff-pre-commit | ||||||||||||||||||
| rev: v0.14.10 | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Harden package input handling to prevent command/option injection.
Line 56 executes
inputs.packagesdirectly in the shell. In a reusable action, this can be abused via shell metacharacters or apt flags if any caller passes non-literal input.🔧 Proposed hardening
Expected result: callers should pass static package lists, not values derived from untrusted event payloads.
🤖 Prompt for AI Agents