ci: run pdp-tester via Docker runtime instead of k3d - #319
Conversation
The pdp-tester job fails at the "Start k3d cluster" step on every PR: Downloading k3d@v5.4.6 ... curl: (22) ... error: 404 Failed to install k3d AbsaOSS/k3d-action@v2.4.0 defaults to k3d v5.4.6, whose GitHub release has no checksums.txt; k3d's install.sh now SHA256-verifies the downloaded binary against that file, so the download 404s. This is environmental (the job passed on main in May) and now breaks pdp-tester on all branches. Switch the job to the pdp-tester Docker runtime backend (aiodocker), which launches the PDP as a local container with no k3d/Helm/kubectl -- mirroring pdp-tester's own `pdp-tester-docker` CI job. LOCAL_IMAGE pins it to the locally built permitio/pdp-v2:next image (loaded from the build artifact) so nothing is pulled from Docker Hub. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the pdp-tester GitHub Actions job to avoid k3d/Kubernetes setup failures by running pdp-tester against the locally-built PDP image using the Docker runtime backend.
Changes:
- Replace k3d + Helm deployment flow with
pdp-tester’s--docker --localexecution path. - Install
pdp-testerwith its Docker extra (pip install -e ".[docker]") and run tests againstpermitio/pdp-v2:nextloaded from the workflow artifact. - Add an always-run post-mortem step that lists PDP-managed containers and tails their logs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set -o pipefail | ||
| python -m pdp_tester --docker --local --tag next --skip-generate 2>&1 | tee /tmp/tester.log | ||
| if grep -q "test cases failed" /tmp/tester.log; then | ||
| echo "::error::Some test cases failed!" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Good catch — fixed in d6c7cc4. The tester's exit status is now captured explicitly with || status=$? so -e no longer aborts the step before /tmp/tester.log is parsed. We always run the grep check (emitting the Some test cases failed! annotation) and then fail on a non-zero exit status with a distinct annotation.
🔍 Vulnerabilities of
|
| digest | sha256:ca635a014e3f3aed42c5e673918d3bf74367bafc465090271a44d8bbe2c31150 |
| vulnerabilities | |
| platform | linux/amd64 |
| size | 218 MB |
| packages | 247 |
📦 Base Image python:3.10-alpine3.22
| also known as |
|
| digest | sha256:c8f94b3bb77e6ea9015ccd091b7f8aec1b1fcbca95159675235d9a93788797cd |
| vulnerabilities |
Description
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
The default GitHub Actions shell runs with -e, so with pipefail a non-zero exit from pdp_tester aborted the step before the log was parsed, meaning the ::error:: annotation never ran. Capture the exit status explicitly so we always parse the log and fail consistently. Addresses Copilot review comment on PR #319. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses the code review of the Docker-runtime pdp-tester job: - timeout-minutes: 15 on the job. The old k3d path was bounded by `kubectl wait --timeout=600s`; the tester's own max_running_time only applies in interval mode, so run-once CI had no cap and a hung run would hold the runner until GitHub's 360-minute default. - Cache pip deps on setup-python (keyed on pdp-tester/pyproject.toml) so runs don't re-download from PyPI and survive transient outages. - Post-mortem step: hoist the label filter to a variable, iterate over container names directly (dropping the per-container `docker inspect | sed`), and truncate at the daemon with `docker logs --tail 200`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
zeevmoney
left a comment
There was a problem hiding this comment.
Automated review — PER-15361 (pdp-tester CI: k3d -> Docker runtime)
What this PR does: Replaces the pdp-tester CI job's Kubernetes path (k3d cluster + image import + Helm deploy + kubectl wait/kubectl logs + teardown) with the pdp-tester aiodocker Docker-runtime backend: set up Python 3.12, pip install -e ".[docker]", then python -m pdp_tester --docker --local --tag next --skip-generate against the locally-built permitio/pdp-v2:next image. It also adds a timeout-minutes: 15 job cap, pip dependency caching, explicit non-zero-exit handling, and a docker-based post-mortem log dump. Motivation: the AbsaOSS/k3d-action installer 404s (its default k3d v5.4.6 ships no checksums.txt).
Verdict: APPROVE. No Postable finding (severity rule: none postable -> approve). The migration is correct and well-isolated: the removed k3d/Helm/kubectl steps leave no dangling references (only explanatory comments mention them), pdp-tester is a leaf job (nothing has needs: pdp-tester), and the env wiring maps 1:1 to the old Helm --sets — permit.apiUrl -> API_URL, tests.startTimeout=180 -> START_TIMEOUT, pdp.image + pdp.localTags[0]=next -> LOCAL_IMAGE + --tag next + --local, tests.skipGenerate=true -> --skip-generate, PDP_TESTER_API_KEY -> TOKEN. The Docker-runtime path is genuinely exercised: pdp-tester is green in CI (it launches the PDP as a local container and asserts on both a test cases failed log line and a non-zero exit status).
Findings
No postable findings.
Informational
| # | Sev | Ref | Category | Description |
|---|---|---|---|---|
| I1 | INFO | pytests (CI) | Mergeability / coordination | pytests is RED on this branch, but not because of this diff: it is the repo-wide aioresponses / aiohttp-3.14 breakage (34 failures on clean main too). The fix (horizon/tests/conftest.py) currently rides in the sibling PR #318; until it lands on main (or is included here), #319 stays blocked on an unrelated failing check. |
| I2 | INFO | docker-scout (CI) | Pre-existing | The docker-scout high/critical gate is red from residual base-image CVEs; unrelated to this CI-only change (this branch does not touch image contents or requirements.txt). |
| I3 | INFO | security/snyk (CI) | Infra | The Snyk check failed on a quota limit ("used your limit of private tests"), not a vulnerability finding. |
| I4 | INFO | .github/workflows/tests.yml:152 | Prior review (resolved) | Copilot's earlier flag that the default -e shell would abort before /tmp/tester.log is parsed was addressed in d6c7cc4 — the step now captures status=$? so -e no longer aborts, always runs the grep check, and fails distinctly on a non-zero tester exit. Verified in the current code; not re-raised. |
| I5 | INFO | .github/workflows/tests.yml (uses:) | Supply-chain | The PR removes third-party AbsaOSS/k3d-action@v2.4.0 and adds first-party actions/setup-python@v5 at the repo's standard unpinned @vN tag. No PDP-repo SHA-pinning rule exists (no workflow pins actions to a SHA), so the change conforms to convention and net-reduces third-party action surface. |
| I6 | INFO | cross-PR (#318) | Duplication | This same migration is also bundled into PR #318 (commit 75d974b) as an older copy lacking this PR's timeout-minutes / pip-cache / exit-status hardening. Both edit the same file and will conflict; de-conflict by letting one PR own the migration (flagged on #318). |
Blast radius: pdp-tester is a leaf job (no dependents). The helm/kubectl usage in deploy_sidecar.yml and test_release.yml is unrelated (PDP chart deploy / release smoke test) and untouched. tests.yml is invoked by release.yml via secrets: inherit, so PDP_TESTER_API_KEY / CLONE_REPO_TOKEN still reach the job — behavior unchanged there.
Isolation / scope: Well-isolated — one file, one purpose, no drive-by changes. The only entanglement is external (the duplicate copy in #318).
aiohttp 3.14 added a required `stream_writer` kwarg to ClientResponse.__init__, which the test mock library aioresponses (latest, 0.7.9) does not pass. A fresh CI install resolves aiohttp to 3.14.x (the >=3.13.3 pin allowed <4), so every aioresponses-backed horizon test fails with: TypeError: ClientResponse.__init__() missing 1 required keyword-only argument: 'stream_writer' (34 failed on this PR's run; main's last green run predates aiohttp 3.14). Cap to <3.14 until aioresponses ships 3.14 support. The >=3.13.3 floor preserves the CVE-2025-69223/69227/69228/69229 fixes. Reproduced locally: aiohttp 3.14.1 + aioresponses 0.7.9 -> TypeError; aiohttp 3.13.5 + aioresponses 0.7.9 -> passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
main (#318) independently did the same k3d->docker pdp-tester rework and also fixed the aiohttp 3.14 / aioresponses incompatibility via a conftest.py shim that injects the required stream_writer, keeping aiohttp on 3.14.x for its June 2026 security fixes. Conflict resolution: - .github/workflows/tests.yml: kept our version of the pdp-tester job (superset of main's rework, adding timeout-minutes, pip cache, exit-status capture, and simplified post-mortem). - requirements.txt: dropped our aiohttp<3.14 cap in favor of main's conftest.py shim (main's approach avoids reintroducing the 3.14-only CVEs); kept main's cryptography bump and starlette note. - horizon/tests/conftest.py: took main's stream_writer compat shim. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
The
pdp-testerCI job fails at Start k3d cluster on every PR:AbsaOSS/k3d-action@v2.4.0defaults to k3d v5.4.6, whose GitHub release has nochecksums.txt. k3d'sinstall.shnow SHA256-verifies the downloaded binary against that file, so the download 404s. It's environmental (the job passed onmainin May) and now blocks the tester on all branches.Fix
Switch
pdp-testerto the tester's Docker runtime backend (--docker, viaaiodocker), which runs the PDP as a local container — no k3d / Helm / kubectl. This mirrors pdp-tester's ownpdp-tester-dockerCI job (the runtime the maintainers recommend for CI).pip install -e ".[docker]"thenpython -m pdp_tester --docker --local --tag next --skip-generateLOCAL_IMAGE=permitio/pdp-v2pins it to the locally builtpermitio/pdp-v2:nextimage (loaded from thebuild-pdp-imageartifact) — nothing pulled from Docker HubSTART_TIMEOUT=180(matching the old Helm setting) + adocker logspost-mortem stepVerified
Green on the branch where this originated (#318). Job logs show:
(~2m30s). Split out from #318 so it lands on
mainand applies to all PRs.🤖 Generated with Claude Code