From 0f59b6fb7a996debcb350f6bf10a18e5ba276a62 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sat, 5 Sep 2026 12:03:42 -0700 Subject: [PATCH] ci(e2e): refine changed-test selection and runner lifecycle --- .github/e2e-stack/assert_tests_ran.py | 29 +++++-- .github/e2e-stack/secrets_to_env.py | 44 ++++++---- .github/e2e-stack/up.sh | 8 +- .github/workflows/test-e2e-changed.yml | 80 ++++++++++++++++--- .../test_e2e_changed_gate.py | 55 +++++++++++++ tests/e2e/CONTRIBUTING.md | 12 ++- tests/e2e/gateway/stage_mirror_ci_config.yml | 4 - 7 files changed, 186 insertions(+), 46 deletions(-) create mode 100644 tests/code_coverage_tests/test_e2e_changed_gate.py diff --git a/.github/e2e-stack/assert_tests_ran.py b/.github/e2e-stack/assert_tests_ran.py index 092a9db7256..7fd3f7c7c33 100644 --- a/.github/e2e-stack/assert_tests_ran.py +++ b/.github/e2e-stack/assert_tests_ran.py @@ -5,15 +5,28 @@ from typing import Final def main() -> int: - report: Final = ET.parse(Path(sys.argv[1])).getroot() - suites: Final = tuple(report.iter("testsuite")) - collected: Final = sum(int(suite.get("tests", "0")) for suite in suites) - skipped: Final = sum(int(suite.get("skipped", "0")) for suite in suites) - executed: Final = collected - skipped - _ = sys.stdout.write(f"executed {executed} of {collected} collected tests ({skipped} skipped)\n") - if executed > 0: + selected: Final = tuple(sys.argv[2:]) + try: + report: Final = ET.parse(Path(sys.argv[1])).getroot() + except (ET.ParseError, OSError): + _ = sys.stdout.write("::error::could not read the test execution report\n") + return 1 + cases: Final = tuple(report.iter("testcase")) + passed: Final = frozenset( + case.get("file") for case in cases if all(case.find(tag) is None for tag in ("skipped", "failure", "error")) + ) + missing: Final = tuple(path for path in selected if path not in passed) + for path in selected: + collected: Final = sum(case.get("file") == path for case in cases) + skipped: Final = sum(case.get("file") == path and case.find("skipped") is not None for case in cases) + _ = sys.stdout.write(f"{path}: {collected} collected, {skipped} skipped\n") + if ( + selected + and not missing + and not any(case.find(tag) is not None for case in cases for tag in ("failure", "error")) + ): return 0 - _ = sys.stdout.write("::error::every selected test was skipped, so nothing was verified\n") + _ = sys.stdout.write("::error::every selected file must execute a passing test, with no failures or errors\n") return 1 diff --git a/.github/e2e-stack/secrets_to_env.py b/.github/e2e-stack/secrets_to_env.py index d2d6675d690..99e717b271d 100644 --- a/.github/e2e-stack/secrets_to_env.py +++ b/.github/e2e-stack/secrets_to_env.py @@ -1,29 +1,41 @@ +import os import re import sys from pathlib import Path from typing import Final -from pydantic import TypeAdapter +from pydantic import TypeAdapter, ValidationError -secrets_adapter: TypeAdapter[dict[str, str]] = TypeAdapter(dict[str, str]) -SECRET_NAME: Final = re.compile(r"KEY|SECRET|TOKEN|PASS|CREDENTIAL|LICENSE|AUTH") +secrets_adapter: Final[TypeAdapter[dict[str, str]]] = TypeAdapter(dict[str, str]) +ENV_NAME: Final = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") def main() -> int: - env_path = Path(sys.argv[1]) - secrets = {key: value.rstrip("\r\n") for key, value in secrets_adapter.validate_json(sys.stdin.read()).items()} - unwritable = tuple( - key for key, value in secrets.items() if "'" in value or "\n" in value or "\r" in value - ) - if unwritable: - _ = sys.stderr.write(f"values contain characters unsafe for both bash and dotenv: {', '.join(unwritable)}\n") + env_path: Final = Path(sys.argv[1]) + try: + secrets: Final = { + key: value.rstrip("\r\n") for key, value in secrets_adapter.validate_json(sys.stdin.read()).items() + } + except (ValidationError, UnicodeError): + _ = sys.stderr.write("expected a JSON object containing string environment values\n") + return 1 + if any( + ENV_NAME.fullmatch(key) is None or any(char in value for char in "'\n\r\0") for key, value in secrets.items() + ): + _ = sys.stderr.write("environment names or values cannot be represented in both bash and dotenv\n") + return 1 + for value in secrets.values(): + if value: + _ = sys.stdout.write(f"::add-mask::{value.replace('%', '%25')}\n") + sys.stdout.flush() + lines: Final = tuple(f"{key}='{value}'" for key, value in secrets.items() if value) + try: + with os.fdopen(os.open(env_path, os.O_WRONLY | os.O_APPEND | os.O_CREAT | os.O_NOFOLLOW, 0o600), "w") as handle: + os.fchmod(handle.fileno(), 0o600) + _ = handle.write("\n".join(lines) + "\n") + except OSError: + _ = sys.stderr.write("could not write the environment file\n") return 1 - lines = tuple(f"{key}='{value}'" for key, value in secrets.items() if value) - with env_path.open("a") as handle: - _ = handle.write("\n".join(lines) + "\n") - for key, value in secrets.items(): - if value and SECRET_NAME.search(key): - _ = sys.stdout.write(f"::add-mask::{value}\n") return 0 diff --git a/.github/e2e-stack/up.sh b/.github/e2e-stack/up.sh index 06fce35a896..e891add341d 100755 --- a/.github/e2e-stack/up.sh +++ b/.github/e2e-stack/up.sh @@ -1,5 +1,6 @@ #!/usr/bin/env bash set -euo pipefail +umask 077 REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" STACK_DIR="${E2E_STACK_DIR:-${RUNNER_TEMP:-/tmp}/litellm-e2e-stack}" @@ -8,9 +9,9 @@ LOGS_DIR="${STACK_DIR}/logs" PIDS_DIR="${STACK_DIR}/pids" POSTGRES_IMAGE="${E2E_POSTGRES_IMAGE:-postgres:16.6}" -VALKEY_IMAGE="${E2E_VALKEY_IMAGE:-valkey/valkey:8.1}" +VALKEY_IMAGE="${E2E_VALKEY_IMAGE:-valkey/valkey:8.1.4@sha256:81db6d39e1bba3b3ff32bd3a1b19a6d69690f94a3954ec131277b9a26b95b3aa}" JAEGER_IMAGE="${E2E_JAEGER_IMAGE:-jaegertracing/jaeger:2.10.0}" -NGINX_IMAGE="${E2E_NGINX_IMAGE:-nginx:1.29-alpine}" +NGINX_IMAGE="${E2E_NGINX_IMAGE:-nginx:1.29.1-alpine@sha256:42a516af16b852e33b7682d5ef8acbd5d13fe08fecadc7ed98605ba5e3b26ab8}" LB_PORT="${E2E_LB_PORT:-4000}" GATEWAY_PORT_1="${E2E_GATEWAY_PORT_1:-4010}" @@ -28,6 +29,8 @@ JAEGER_QUERY_PORT="${E2E_JAEGER_QUERY_PORT:-16686}" MASTER_KEY="${LITELLM_MASTER_KEY:-sk-e2e-$(openssl rand -hex 16)}" mkdir -p "${CERTS_DIR}" "${LOGS_DIR}" "${PIDS_DIR}" +chmod 700 "${STACK_DIR}" "${LOGS_DIR}" "${PIDS_DIR}" +chmod 755 "${CERTS_DIR}" log() { printf 'e2e-stack: %s\n' "$*"; } @@ -38,7 +41,6 @@ wait_for() { until eval "${check}"; do if ((SECONDS >= deadline)); then log "timed out waiting for ${label}" - tail -n 60 "${LOGS_DIR}"/*.log 2>/dev/null || true exit 1 fi sleep 2 diff --git a/.github/workflows/test-e2e-changed.yml b/.github/workflows/test-e2e-changed.yml index e08660d0412..b802e9173f5 100644 --- a/.github/workflows/test-e2e-changed.yml +++ b/.github/workflows/test-e2e-changed.yml @@ -26,27 +26,26 @@ jobs: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} - SMOKE_TESTS: tests/e2e/access_control + HEAD_SHA: ${{ github.event.pull_request.head.sha }} OWN_LANE: '^tests/e2e/(ui|claude_code|load)/|^tests/e2e/batches/test_managed_files_enforcement_e2e\.py$' run: | + gh api "repos/${REPO}/pulls/${PR_NUMBER}" \ + --jq 'select(.head.sha == env.HEAD_SHA and .changed_files < 3000) | .head.sha' \ + | grep -Fxq "${HEAD_SHA}" files="$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/files" --paginate \ --jq '.[] | select(.status != "removed") | .filename')" + gh api "repos/${REPO}/pulls/${PR_NUMBER}" --jq '.head.sha' | grep -Fxq "${HEAD_SHA}" tests="$(printf '%s\n' "${files}" \ | grep -E '^tests/e2e/([A-Za-z0-9_.-]+/)*test_[A-Za-z0-9_.-]+\.py$' \ | grep -vE "${OWN_LANE}" \ | sort -u | tr '\n' ' ' | sed 's/ $//')" || true - if [ -z "${tests}" ] && printf '%s\n' "${files}" | grep -vE "${OWN_LANE}" \ - | grep -qE '^(tests/e2e/|\.github/e2e-stack/|\.github/workflows/test-e2e-changed\.yml$)'; then - tests="${SMOKE_TESTS}" - echo "harness or stack changed without a test file; running the smoke suite" - fi echo "tests=${tests}" >> "${GITHUB_OUTPUT}" if [ -n "${tests}" ]; then echo "any=true" >> "${GITHUB_OUTPUT}" echo "selected e2e tests: ${tests}" else echo "any=false" >> "${GITHUB_OUTPUT}" - echo "no e2e changes; nothing to run" + echo "no changed e2e test files supported by this stack; nothing to run" fi run: @@ -88,6 +87,7 @@ jobs: uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 with: persist-credentials: false + ref: ${{ github.sha }} - name: Set up Python uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 @@ -120,14 +120,24 @@ jobs: run: uv run --no-sync playwright install --with-deps chromium - name: Configure AWS credentials + id: aws uses: aws-actions/configure-aws-credentials@e7f100cf4c008499ea8adda475de1042d6975c7b # v6.2.0 with: role-to-assume: ${{ vars.E2E_AWS_ROLE_TO_ASSUME }} aws-region: us-east-1 role-session-name: litellm-e2e-changed-${{ github.run_id }} + role-duration-seconds: 900 + output-env-credentials: false + output-credentials: true - name: Fetch provider credentials from AWS Secrets Manager + env: + AWS_ACCESS_KEY_ID: ${{ steps.aws.outputs.aws-access-key-id }} + AWS_SECRET_ACCESS_KEY: ${{ steps.aws.outputs.aws-secret-access-key }} + AWS_SESSION_TOKEN: ${{ steps.aws.outputs.aws-session-token }} + AWS_DEFAULT_REGION: us-east-1 run: | + umask 077 aws secretsmanager get-secret-value --secret-id litellm-e2e-changed-provider-keys \ --query SecretString --output text \ | uv run --no-sync python .github/e2e-stack/secrets_to_env.py tests/e2e/.env @@ -138,7 +148,12 @@ jobs: - name: Boot the stage-mirror stack id: boot - run: bash .github/e2e-stack/up.sh + run: | + umask 077 + if ! bash .github/e2e-stack/up.sh > "${RUNNER_TEMP}/e2e-boot.log" 2>&1; then + echo "::error::stage-mirror stack failed to boot; raw logs are not published" + exit 1 + fi - name: Export stack environment run: | @@ -149,13 +164,16 @@ jobs: - name: Run the selected tests three times with retries off env: TESTS: ${{ needs.detect.outputs.tests }} + E2E_FIXTURE_MODE: live run: | + umask 077 read -r -a test_files <<< "${TESTS}" for pass in 1 2 3; do report="${RUNNER_TEMP}/e2e-pass-${pass}.xml" echo "::group::pass ${pass} of 3" set +e - uv run --no-sync pytest "${test_files[@]}" --reruns 0 -v -rA --tb=short -p no:cacheprovider --junitxml="${report}" + uv run --no-sync pytest "${test_files[@]}" --rootdir=. --reruns 0 -v -p no:cacheprovider \ + -o junit_family=xunit1 --junitxml="${report}" > "${RUNNER_TEMP}/e2e-pass-${pass}.log" 2>&1 status=$? set -e echo "::endgroup::" @@ -163,13 +181,49 @@ jobs: echo "::error::the selected files collected no runnable tests, so nothing was verified" exit 1 fi + if ! uv run --no-sync python .github/e2e-stack/assert_tests_ran.py "${report}" "${test_files[@]}"; then + echo "::error::pass ${pass} of 3 did not verify every selected file" + exit 1 + fi if [ "${status}" != "0" ]; then echo "::error::pass ${pass} of 3 failed with exit code ${status}" exit "${status}" fi - uv run --no-sync python .github/e2e-stack/assert_tests_ran.py "${report}" + echo "pass ${pass} of 3 passed" done - - name: Show stack logs on failure - if: failure() && steps.boot.conclusion != 'skipped' - run: tail -n 200 "${RUNNER_TEMP}/litellm-e2e-stack/logs"/*.log + - name: Stop the stack + if: always() && steps.boot.outcome != 'skipped' + run: bash .github/e2e-stack/down.sh + + - name: Remove credentials and raw output + if: always() + run: | + rm -f tests/e2e/.env "${RUNNER_TEMP}/e2e-boot.log" "${RUNNER_TEMP}"/e2e-pass-*.log "${RUNNER_TEMP}"/e2e-pass-*.xml + rm -rf "${RUNNER_TEMP}/litellm-e2e-stack" + + gate: + name: e2e-changed-tests + needs: [detect, run] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Require three successful passes when tests changed + env: + DETECT_RESULT: ${{ needs.detect.result }} + ANY_TESTS: ${{ needs.detect.outputs.any }} + RUN_RESULT: ${{ needs.run.result }} + run: | + if [ "${DETECT_RESULT}" != "success" ]; then + echo "::error::changed-test detection did not succeed" + exit 1 + fi + if [ "${ANY_TESTS}" = "false" ]; then + echo "no changed e2e test files supported by this stack; nothing to run" + exit 0 + fi + if [ "${ANY_TESTS}" != "true" ] || [ "${RUN_RESULT}" != "success" ]; then + echo "::error::selected e2e tests require an approved, successful run; fork PRs must run from a reviewed same-repository branch" + exit 1 + fi diff --git a/tests/code_coverage_tests/test_e2e_changed_gate.py b/tests/code_coverage_tests/test_e2e_changed_gate.py new file mode 100644 index 00000000000..7a82a8ebb60 --- /dev/null +++ b/tests/code_coverage_tests/test_e2e_changed_gate.py @@ -0,0 +1,55 @@ +import subprocess +import sys +import xml.etree.ElementTree as ET +from pathlib import Path +from typing import Final + +import pytest + +GATE: Final = Path(__file__).resolve().parents[2] / ".github/e2e-stack/assert_tests_ran.py" +SELECTED: Final = ("tests/e2e/access_control/test_a.py", "tests/e2e/access_control/test_b.py") + + +@pytest.mark.parametrize( + ("second_outcome", "expected_status"), + (("passed", 0), ("skipped", 1), ("failure", 1), ("error", 1), ("deselected", 1)), +) +def test_each_changed_file_must_run(tmp_path: Path, second_outcome: str, expected_status: int) -> None: + suite: Final = ET.Element("testsuite") + _ = ET.SubElement(suite, "testcase", file=SELECTED[0]) + if second_outcome != "deselected": + second: Final = ET.SubElement(suite, "testcase", file=SELECTED[1]) + if second_outcome != "passed": + _ = ET.SubElement(second, second_outcome) + report: Final = tmp_path / "report.xml" + ET.ElementTree(suite).write(report) + + result: Final = subprocess.run([sys.executable, str(GATE), str(report), *SELECTED], capture_output=True, text=True) + + assert result.returncode == expected_status, result.stdout + + +@pytest.mark.parametrize("outcome", ("failure", "error")) +def test_passing_case_does_not_hide_a_failure_in_the_same_file(tmp_path: Path, outcome: str) -> None: + suite: Final = ET.Element("testsuite") + _ = ET.SubElement(suite, "testcase", file=SELECTED[0]) + failed: Final = ET.SubElement(suite, "testcase", file=SELECTED[0]) + _ = ET.SubElement(failed, outcome) + report: Final = tmp_path / "report.xml" + ET.ElementTree(suite).write(report) + + result: Final = subprocess.run( + [sys.executable, str(GATE), str(report), SELECTED[0]], capture_output=True, text=True + ) + + assert result.returncode == 1 + + +@pytest.mark.parametrize("contents", ("", "')) +def test_missing_execution_evidence_fails(tmp_path: Path, contents: str) -> None: + report: Final = tmp_path / "report.xml" + _ = report.write_text(contents) + + result: Final = subprocess.run([sys.executable, str(GATE), str(report), *SELECTED], capture_output=True, text=True) + + assert result.returncode == 1 diff --git a/tests/e2e/CONTRIBUTING.md b/tests/e2e/CONTRIBUTING.md index 1e88f6505cf..d51e729fd1c 100644 --- a/tests/e2e/CONTRIBUTING.md +++ b/tests/e2e/CONTRIBUTING.md @@ -54,9 +54,17 @@ Some suites need extra services the bare proxy does not start. The `logging/` OT ### The pull request check -Every PR that adds or modifies a `tests/e2e/**/test_*.py` file (outside `ui/`, `claude_code/`, `load/`, and `batches/test_managed_files_enforcement_e2e.py`, which have their own lanes or need a differently configured proxy) runs exactly those files three times, with retries off, against a stage-mirror stack booted on a GitHub Actions runner: migrations, a control-plane backend, two gateway processes behind an nginx load balancer, Postgres, Jaeger, and a TLS cluster-mode Valkey, wired the way stage is deployed. A PR that only touches the harness or the stack itself runs the `access_control/` suite as a smoke instead. Three green passes are the bar because the check exists to catch a flaky test before it reaches the release gate, so a red pass is a failure to fix, not a retry candidate. A pass that executes nothing is also red: each pass writes a JUnit report and fails when every collected test was skipped, so a skipped-out file cannot pass on paper. The same stack boots on a laptop with `bash .github/e2e-stack/up.sh`: it reads provider keys from `tests/e2e/.env`, writes the pytest environment to `${E2E_STACK_DIR:-/tmp/litellm-e2e-stack}/stack.env`, every port is overridable through `E2E_*_PORT` variables, and `down.sh` tears it all down +Every same-repository PR that adds, modifies, or renames a `tests/e2e/**/test_*.py` file runs those changed files three times with pytest retries off. The stage-mirror stack has a control-plane backend, two gateways behind nginx, Postgres, Jaeger, and TLS cluster-mode Valkey. Documentation, harness, configuration, deleted-file, and workflow-only changes do not start the stack or request environment approval. The `ui/`, `claude_code/`, and `load/` directories and `batches/test_managed_files_enforcement_e2e.py` remain outside this check because they use separate tooling or need a differently configured stack -Credentials: everything the runner writes into `tests/e2e/.env` comes from two AWS Secrets Manager secrets in us-east-1, `litellm-e2e-changed-provider-keys` and `litellm-e2e-changed-license`, and the first holds only the names that the tests and `tests/e2e/gateway/stage_mirror_ci_config.yml` read. The cloud credentials in it are dedicated to this lane and can do only what the tests do: the AWS pair belongs to the `litellm-e2e-changed` IAM user, whose inline policy allows Bedrock inference, the one guardrail the tests use, the batch job APIs, the batch S3 bucket, and assuming the batch test's role, and the Vertex key belongs to a service account holding only `roles/aiplatform.user` on the Vertex project. The provider API keys carry no lane-specific spend cap, since a cap that trips mid-month would fail every run until it resets, so the reviewer's approval of the `e2e-changed` environment is the control on how a PR's tests use them. Rotating any value is a single `aws secretsmanager put-secret-value` on the secret, with no workflow change +Every selected file must execute at least one passing test in each pass, and any test failure, collection error, or entirely skipped or deselected file fails the check. A failed pass stops the run without retrying. The final `e2e-changed-tests` job succeeds only when no supported test files changed or the approved run completed all three passes. Fork PRs with selected tests fail this gate until a maintainer brings the reviewed change onto a same-repository branch + +Repository admins must require the `e2e-changed-tests` status check for merging and configure the `e2e-changed` environment with required reviewers, self-review disabled, and admin bypass disabled. Each push cancels the previous run; a new run that selects tests needs a fresh approval. Reviewers must inspect the entire executable PR diff, including application code, dependencies, tests, and workflow helpers, before approving the exact revision. Approved code executes with provider credentials, so environment approval is a trust decision about that code + +Credentials come from the existing AWS Secrets Manager secrets in us-east-1, `litellm-e2e-changed-provider-keys` and `litellm-e2e-changed-license`. The OIDC role must trust only `repo:BerriAI/litellm:environment:e2e-changed` with audience `sts.amazonaws.com` and have read access only to these secrets. The short-lived reader credentials are scoped to the fetch step. Provider credentials must cover the selected suites, including Datadog credentials when logging or MCP tests need them; missing credentials fail the run. Keep provider credentials dedicated to this lane with only the permissions those tests need + +Fetched values are masked before use, and credential files and raw output are private to the runner. Public logs contain selected file names, counts, and pass status; raw pytest output, reports, and stack logs are not uploaded or printed. The workflow removes them and the credential files during cleanup. To diagnose a failed pass, reproduce the selected files locally with the appropriate credentials and inspect the local logs + +To reproduce the CI topology on a dedicated machine, `bash .github/e2e-stack/up.sh` reads `tests/e2e/.env`, writes `stack.env` under `${E2E_STACK_DIR:-/tmp/litellm-e2e-stack}`, and `bash .github/e2e-stack/down.sh` stops it. Keep this directory private and remove its credential files and logs after use ### Record and replay diff --git a/tests/e2e/gateway/stage_mirror_ci_config.yml b/tests/e2e/gateway/stage_mirror_ci_config.yml index 21664c2d0a1..6b3756b5f9f 100644 --- a/tests/e2e/gateway/stage_mirror_ci_config.yml +++ b/tests/e2e/gateway/stage_mirror_ci_config.yml @@ -52,10 +52,6 @@ model_list: litellm_params: model: gemini/gemini-2.5-flash api_key: os.environ/GEMINI_API_KEY - - model_name: gemini-2.5-flash - litellm_params: - model: gemini/gemini-2.5-flash - api_key: os.environ/GEMINI_API_KEY mcp_servers: devin: